improve error handling, fix crash on empty /api URL

src/command.cpp

@@ -35,9 +35,7 @@ uint8_t Command::process(const char * path, const bool is_admin, const JsonObjec
     p.parse(path);
 
     if (!p.paths().size()) {
-        output.clear();
+        return message(CommandRet::ERROR, "invalid path", output);
-        output["message"] = "invalid path"; // path is empty
-        return CommandRet::ERROR;
     }
 
     // check first if it's from API, if so strip the "api/"
@@ -50,16 +48,19 @@ uint8_t Command::process(const char * path, const bool is_admin, const JsonObjec
             strncpy(new_path, path, sizeof(new_path));
             p.parse(new_path + Mqtt::base().length() + 1); // re-parse the stripped path
         } else {
-            // error
+            return message(CommandRet::ERROR, "unrecognized path", output); // error
-            output.clear();
-            output["message"] = "unrecognized path";
-            return CommandRet::ERROR;
         }
     }
 
     // Serial.println(p.path().c_str()); // dump paths, for debugging
 
-    size_t      num_paths = p.paths().size(); // re-calculate new path
+    // re-calculate new path
+    // if there is only a path (URL) and no body then error!
+    size_t num_paths = p.paths().size();
+    if (!num_paths && !input.size()) {
+        return message(CommandRet::ERROR, "missing command in path", output);
+    }
+
     std::string cmd_s;
     int8_t      id_n = -1; // default hc
 
@@ -79,11 +80,8 @@ uint8_t Command::process(const char * path, const bool is_admin, const JsonObjec
     // validate the device, make sure it exists
     uint8_t device_type = EMSdevice::device_name_2_device_type(device_s);
     if (!device_has_commands(device_type)) {
-        output.clear();
+        LOG_DEBUG(F("Command failed: unknown device '%s'"), device_s);
-        char error[100];
+        return message(CommandRet::ERROR, "unknown device", output);
-        snprintf(error, sizeof(error), "no device called %s", device_s);
-        output["message"] = error;
-        return CommandRet::NOT_FOUND;
     }
 
     // the next value on the path should be the command
@@ -117,9 +115,7 @@ uint8_t Command::process(const char * path, const bool is_admin, const JsonObjec
                 command_p = "values";
             }
         } else {
-            output.clear();
+            return message(CommandRet::NOT_FOUND, "missing or bad command", output);
-            output["message"] = "missing or bad command";
-            return CommandRet::NOT_FOUND;
         }
     }
 
@@ -154,15 +150,10 @@ uint8_t Command::process(const char * path, const bool is_admin, const JsonObjec
         char data_str[10];
         return_code = Command::call(device_type, command_p, Helpers::render_value(data_str, (float)data.as<float>(), 2), is_admin, id_n, output);
     } else if (data.isNull()) {
-        // empty
+        return_code = Command::call(device_type, command_p, "", is_admin, id_n, output); // empty
-        return_code = Command::call(device_type, command_p, "", is_admin, id_n, output);
     } else {
-        // can't process
+        return message(CommandRet::ERROR, "cannot parse command", output); // can't process
-        output.clear();
-        output["message"] = "cannot parse command";
-        return CommandRet::ERROR;
     }
-
     return return_code;
 }
 
@@ -175,10 +166,10 @@ const std::string Command::return_code_string(const uint8_t return_code) {
         return read_flash_string(F("OK"));
         break;
     case CommandRet::NOT_FOUND:
-        return read_flash_string(F("Not found"));
+        return read_flash_string(F("Not Found"));
         break;
     case CommandRet::NOT_ALLOWED:
-        return read_flash_string(F("Not authorized"));
+        return read_flash_string(F("Not Authorized"));
         break;
     case CommandRet::FAIL:
         return read_flash_string(F("Failed"));
@@ -289,18 +280,15 @@ uint8_t Command::call(const uint8_t device_type, const char * cmd, const char *
 
         // report error if call failed
         if (return_code != CommandRet::OK) {
-            output.clear();
+            return message(return_code, "callback function failed", output);
-            output["message"] = "callback function failed";
         }
 
         return return_code;
     }
 
     // we didn't find the command and its not an endpoint, report error
-    char error[100];
+    LOG_DEBUG(F("Command failed: invalid command '%s'"), cmd);
-    snprintf(error, sizeof(error), "invalid command '%s'", cmd);
+    return message(CommandRet::NOT_FOUND, "invalid command", output);
-    output["message"] = error;
-    return CommandRet::NOT_FOUND;
 }
 
 // add a command to the list, which does not return json
@@ -525,10 +513,8 @@ void Command::show_all(uuid::console::Shell & shell) {
     }
 }
 
-/**
+// Extract only the path component from the passed URI and normalized it
- * Extract only the path component from the passed URI and normalized it.
+// e.g. //one/two////three/// becomes /one/two/three
- * Ex. //one/two////three/// becomes /one/two/three
- */
 std::string SUrlParser::path() {
     std::string s = "/"; // set up the beginning slash
     for (std::string & f : m_folders) {

src/command.h

@@ -139,7 +139,13 @@ class Command {
   private:
     static uuid::log::Logger logger_;
 
-    static std::vector<CmdFunction> cmdfunctions_; // list of commands
+    static std::vector<CmdFunction> cmdfunctions_; // the list of commands
+
+    inline static uint8_t message(uint8_t error_code, const char * message, JsonObject & output) {
+        output.clear();
+        output["message"] = (const char *)message;
+        return error_code;
+    }
 };
 
 typedef std::unordered_map<std::string, std::string> KeyValueMap_t;

src/test/test.cpp

@@ -486,96 +486,113 @@ void Test::run_test(uuid::console::Shell & shell, const std::string & cmd) {
 
 #if defined(EMSESP_STANDALONE)
 
-        AsyncWebServerRequest request2;
+        AsyncWebServerRequest requestX;
-        request2.method(HTTP_GET);
+        DynamicJsonDocument   docX(2000);
+        JsonVariant           jsonX;
 
-        // request2.url("/api/thermostat/seltemp");
+        requestX.method(HTTP_GET);
-        // EMSESP::webAPIService.webAPIService_get(&request2);
-        // return;
 
         /*
-        request2.url("/api/thermostat/mode/auto"); 
+        requestX.url("/api"); // should fail
-        EMSESP::webAPIService.webAPIService_get(&request2);
+        EMSESP::webAPIService.webAPIService_get(&requestX);
         return;
         */
 
         /*
-        request2.url("/api/thermostat"); // check if defaults to info
+        requestX.url("/api/thermostat/seltemp");
-        EMSESP::webAPIService.webAPIService_get(&request2);
+        EMSESP::webAPIService.webAPIService_get(&requestX);
-        request2.url("/api/thermostat/info");
-        EMSESP::webAPIService.webAPIService_get(&request2);
-        request2.url("/api/thermostat/values");
-        EMSESP::webAPIService.webAPIService_get(&request2);
-        return;
-
-        request2.url("/api/thermostat/mode");
-        EMSESP::webAPIService.webAPIService_get(&request2);
         return;
         */
 
         /*
-        request2.url("/api/system"); // check if defaults to info
+        requestX.url("/api/thermostat/mode/auto"); 
-        EMSESP::webAPIService.webAPIService_get(&request2);
+        EMSESP::webAPIService.webAPIService_get(&requestX);
-        emsesp::EMSESP::logger().notice("*");
-
-        request2.url("/api/system/info");
-        EMSESP::webAPIService.webAPIService_get(&request2);
-        emsesp::EMSESP::logger().notice("*");
-
-        request2.url("/api/thermostat"); // check if defaults to values
-        EMSESP::webAPIService.webAPIService_get(&request2);
-        emsesp::EMSESP::logger().notice("*");
-
-        request2.url("/api/thermostat/info");
-        EMSESP::webAPIService.webAPIService_get(&request2);
-        emsesp::EMSESP::logger().notice("*");
-
-        request2.url("/api/thermostat/seltemp");
-        EMSESP::webAPIService.webAPIService_get(&request2);
-        return;
-
-        */
-
-        /*
-        request2.url("/api/system/restart");
-        EMSESP::webAPIService.webAPIService_get(&request2);
         return;
         */
 
         /*
-        request2.url("/api/dallassensor/fdfd");
+        requestX.url("/api/thermostat"); // check if defaults to info
-        EMSESP::webAPIService.webAPIService_get(&request2);
+        EMSESP::webAPIService.webAPIService_get(&requestX);
+        requestX.url("/api/thermostat/info");
+        EMSESP::webAPIService.webAPIService_get(&requestX);
+        requestX.url("/api/thermostat/values");
+        EMSESP::webAPIService.webAPIService_get(&requestX);
+        return;
+
+        requestX.url("/api/thermostat/mode");
+        EMSESP::webAPIService.webAPIService_get(&requestX);
+        return;
+        */
+
+        /*
+        requestX.url("/api/system"); // check if defaults to info
+        EMSESP::webAPIService.webAPIService_get(&requestX);
+        emsesp::EMSESP::logger().notice("*");
+
+        requestX.url("/api/system/info");
+        EMSESP::webAPIService.webAPIService_get(&requestX);
+        emsesp::EMSESP::logger().notice("*");
+
+        requestX.url("/api/thermostat"); // check if defaults to values
+        EMSESP::webAPIService.webAPIService_get(&requestX);
+        emsesp::EMSESP::logger().notice("*");
+
+        requestX.url("/api/thermostat/info");
+        EMSESP::webAPIService.webAPIService_get(&requestX);
+        emsesp::EMSESP::logger().notice("*");
+
+        requestX.url("/api/thermostat/seltemp");
+        EMSESP::webAPIService.webAPIService_get(&requestX);
+        return;
+        */
+
+        /*
+        requestX.url("/api/system/restart");
+        EMSESP::webAPIService.webAPIService_get(&requestX);
+        return;
+        */
+
+        /*
+        requestX.url("/api/dallassensor/xxxx");
+        EMSESP::webAPIService.webAPIService_get(&requestX);
         emsesp::EMSESP::logger().notice("****");
-        request2.url("/api/dallassensor/info");
+        requestX.url("/api/dallassensor/info");
-        EMSESP::webAPIService.webAPIService_get(&request2);
+        EMSESP::webAPIService.webAPIService_get(&requestX);
+        return;
+        */
+
+        /*
+        requestX.url("/api"); // should fail
+        EMSESP::webAPIService.webAPIService_get(&requestX);
+
+        requestX.method(HTTP_POST);
+
+        char dataX[] = "{\"device\":\"thermostat\", \"entity\":\"seltemp\",\"value\":13}";
+        deserializeJson(docX, dataX);
+        jsonX = docX.as<JsonVariant>();
+        requestX.url("/api");
+        EMSESP::webAPIService.webAPIService_post(&requestX, jsonX);
         return;
         */
 
         /*
-        AsyncWebServerRequest request2;
-        request2.method(HTTP_POST);
-        DynamicJsonDocument docX(2000);
-        JsonVariant         jsonX;
         // char                dataX[] = "{\"value\":\"0B 88 19 19 02\"}";
         char dataX[] = "{\"name\":\"temp\",\"value\":11}";
         deserializeJson(docX, dataX);
         jsonX = docX.as<JsonVariant>();
-        // request2.url("/api/system/send");
+        // requestX.url("/api/system/send");
-        request2.url("/api/thermostat");
+        requestX.url("/api/thermostat");
-        EMSESP::webAPIService.webAPIService_post(&request2, jsonX);
+        EMSESP::webAPIService.webAPIService_post(&requestX, jsonX);
         return;
         */
 
         /*
-        AsyncWebServerRequest request3;
-        request3.method(HTTP_POST);
-        DynamicJsonDocument docX(2000);
-        JsonVariant         jsonX;
         char dataX[] = "{}";
         deserializeJson(docX, dataX);
         jsonX = docX.as<JsonVariant>();
-        request3.url("/api/thermostat/mode/auto"); // should fail
+        requestX.url("/api/thermostat/mode/auto"); // should fail
-        EMSESP::webAPIService.webAPIService_post(&request3, jsonX);
+        EMSESP::webAPIService.webAPIService_post(&requestX, jsonX);
+        return;
         */
 
         // test command parse
@@ -618,26 +635,17 @@ void Test::run_test(uuid::console::Shell & shell, const std::string & cmd) {
         EMSESP::mqtt_.incoming("ems-esp/thermostat_hc1", "22");  // HA only
         EMSESP::mqtt_.incoming("ems-esp/thermostat_hc1", "off"); // HA only
         EMSESP::mqtt_.incoming("ems-esp/system/send", "11 12 13");
+        EMSESP::mqtt_.incoming("ems-esp/boiler/syspress"); // empty payload, sends reponse
+        EMSESP::mqtt_.incoming("ems-esp/thermostat/mode"); // empty payload, sends reponse
+        EMSESP::mqtt_.incoming("ems-esp/system/publish");
+        EMSESP::mqtt_.incoming("ems-esp/thermostat/seltemp"); // empty payload, sends reponse
 
         // MQTT bad tests
         EMSESP::mqtt_.incoming("ems-esp/thermostate/mode", "auto");     // unknown device
         EMSESP::mqtt_.incoming("ems-esp/thermostat/modee", "auto");     // unknown command
-        EMSESP::mqtt_.incoming("ems-esp/thermostat/mode/auto", "auto"); // invalid
+        EMSESP::mqtt_.incoming("ems-esp/thermostat/mode/auto", "auto"); // invalid, not allowed
 
-        // various MQTT checks
+        // check extended MQTT base
-        /*
-        EMSESP::mqtt_.incoming("ems-esp/thermostat/mode");         // empty payload, sends reponse
-        EMSESP::mqtt_.incoming("ems-esp/boiler/syspress"); // empty payload, sends reponse
-        EMSESP::mqtt_.incoming("ems-esp/thermostat/mode", "auto"); // set mode
-        EMSESP::mqtt_.incoming("ems-esp/thermostat/mode");         // empty payload, sends reponse
-        EMSESP::mqtt_.incoming("ems-esp/system/send", "11 12 13");
-        EMSESP::mqtt_.incoming("ems-esp/system/publish");
-        EMSESP::mqtt_.incoming("ems-esp/thermostat/seltemp"); // empty payload, sends reponse
-        EMSESP::mqtt_.incoming("ems-esp/system/send", "11 12 13");
-        return;
-        */
-
-        // check long base
         Mqtt::base("home/cellar/heating");
         EMSESP::mqtt_.incoming("home/cellar/heating/thermostat/mode"); // empty payload, sends reponse
 
@@ -683,7 +691,7 @@ void Test::run_test(uuid::console::Shell & shell, const std::string & cmd) {
         EMSESP::webAPIService.webAPIService_post(&request, json);
 
         // 3
-        char data3[] = "{\"device\":\"thermostat\", \"name\":\"temp\",\"value\":13}";
+        char data3[] = "{\"device\":\"thermostat\", \"name\":\"seltemp\",\"value\":13}";
         deserializeJson(doc, data3);
         json = doc.as<JsonVariant>();
         request.url("/api");
@@ -1135,7 +1143,7 @@ void Test::run_test(uuid::console::Shell & shell, const std::string & cmd) {
 
         EMSESP::mqtt_.incoming(thermostat_topic, "{\"cmd\":\"wwmode\",\"data\":\"auto\"}");
         EMSESP::mqtt_.incoming(thermostat_topic, "{\"cmd\":\"control\",\"data\":\"1\"}");          // RC35 only, should error
-        EMSESP::mqtt_.incoming(thermostat_topic, "{\"cmd\":\"mode\",\"data\":\"poep\",\"id\":2}"); // invalid mode
+        EMSESP::mqtt_.incoming(thermostat_topic, "{\"cmd\":\"mode\",\"data\":\"typo\",\"id\":2}"); // invalid mode
         EMSESP::mqtt_.incoming(thermostat_topic, "{\"cmd\":\"mode\",\"data\":\"auto\",\"id\":2}");
         EMSESP::mqtt_.incoming(thermostat_topic, "{\"cmd\":\"mode\",\"data\":\"auto\",\"hc\":2}");     // hc as number
         EMSESP::mqtt_.incoming(thermostat_topic, "{\"cmd\":\"temp\",\"data\":19.5,\"hc\":1}");         // data as number