Skip to content

Commit

Permalink
improve error handling, fix crash on empty /api URL
Browse files Browse the repository at this point in the history
  • Loading branch information
proddy committed Nov 10, 2021
1 parent e821e8d commit 5f2a9b0
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 98 deletions.
56 changes: 21 additions & 35 deletions src/command.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
output["message"] = "invalid path"; // path is empty
return CommandRet::ERROR;
return message(CommandRet::ERROR, "invalid path", output);
}

// check first if it's from API, if so strip the "api/"
Expand All @@ -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
output.clear();
output["message"] = "unrecognized path";
return CommandRet::ERROR;
return message(CommandRet::ERROR, "unrecognized path", output); // 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

Expand All @@ -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();
char error[100];
snprintf(error, sizeof(error), "no device called %s", device_s);
output["message"] = error;
return CommandRet::NOT_FOUND;
LOG_DEBUG(F("Command failed: unknown device '%s'"), device_s);
return message(CommandRet::ERROR, "unknown device", output);
}

// the next value on the path should be the command
Expand Down Expand Up @@ -117,9 +115,7 @@ uint8_t Command::process(const char * path, const bool is_admin, const JsonObjec
command_p = "values";
}
} else {
output.clear();
output["message"] = "missing or bad command";
return CommandRet::NOT_FOUND;
return message(CommandRet::NOT_FOUND, "missing or bad command", output);
}
}

Expand Down Expand Up @@ -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);
return_code = Command::call(device_type, command_p, "", is_admin, id_n, output); // empty
} else {
// can't process
output.clear();
output["message"] = "cannot parse command";
return CommandRet::ERROR;
return message(CommandRet::ERROR, "cannot parse command", output); // can't process
}

return return_code;
}

Expand All @@ -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"));
Expand Down Expand Up @@ -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();
output["message"] = "callback function failed";
return message(return_code, "callback function failed", output);
}

return return_code;
}

// we didn't find the command and its not an endpoint, report error
char error[100];
snprintf(error, sizeof(error), "invalid command '%s'", cmd);
output["message"] = error;
return CommandRet::NOT_FOUND;
LOG_DEBUG(F("Command failed: invalid command '%s'"), cmd);
return message(CommandRet::NOT_FOUND, "invalid command", output);
}

// add a command to the list, which does not return json
Expand Down Expand Up @@ -525,10 +513,8 @@ void Command::show_all(uuid::console::Shell & shell) {
}
}

/**
* Extract only the path component from the passed URI and normalized it.
* Ex. //one/two////three/// becomes /one/two/three
*/
// Extract only the path component from the passed URI and normalized it
// e.g. //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) {
Expand Down
8 changes: 7 additions & 1 deletion src/command.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
132 changes: 70 additions & 62 deletions src/test/test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -486,96 +486,113 @@ void Test::run_test(uuid::console::Shell & shell, const std::string & cmd) {

#if defined(EMSESP_STANDALONE)

AsyncWebServerRequest request2;
request2.method(HTTP_GET);
AsyncWebServerRequest requestX;
DynamicJsonDocument docX(2000);
JsonVariant jsonX;

// request2.url("/api/thermostat/seltemp");
// EMSESP::webAPIService.webAPIService_get(&request2);
// return;
requestX.method(HTTP_GET);

/*
request2.url("/api/thermostat/mode/auto");
EMSESP::webAPIService.webAPIService_get(&request2);
requestX.url("/api"); // should fail
EMSESP::webAPIService.webAPIService_get(&requestX);
return;
*/

/*
request2.url("/api/thermostat"); // check if defaults to info
EMSESP::webAPIService.webAPIService_get(&request2);
request2.url("/api/thermostat/info");
EMSESP::webAPIService.webAPIService_get(&request2);
request2.url("/api/thermostat/values");
EMSESP::webAPIService.webAPIService_get(&request2);
requestX.url("/api/thermostat/seltemp");
EMSESP::webAPIService.webAPIService_get(&requestX);
return;
*/

/*
requestX.url("/api/thermostat/mode/auto");
EMSESP::webAPIService.webAPIService_get(&requestX);
return;
*/

/*
requestX.url("/api/thermostat"); // check if defaults to info
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;
request2.url("/api/thermostat/mode");
EMSESP::webAPIService.webAPIService_get(&request2);
requestX.url("/api/thermostat/mode");
EMSESP::webAPIService.webAPIService_get(&requestX);
return;
*/

/*
request2.url("/api/system"); // check if defaults to info
EMSESP::webAPIService.webAPIService_get(&request2);
requestX.url("/api/system"); // check if defaults to info
EMSESP::webAPIService.webAPIService_get(&requestX);
emsesp::EMSESP::logger().notice("*");
request2.url("/api/system/info");
EMSESP::webAPIService.webAPIService_get(&request2);
requestX.url("/api/system/info");
EMSESP::webAPIService.webAPIService_get(&requestX);
emsesp::EMSESP::logger().notice("*");
request2.url("/api/thermostat"); // check if defaults to values
EMSESP::webAPIService.webAPIService_get(&request2);
requestX.url("/api/thermostat"); // check if defaults to values
EMSESP::webAPIService.webAPIService_get(&requestX);
emsesp::EMSESP::logger().notice("*");
request2.url("/api/thermostat/info");
EMSESP::webAPIService.webAPIService_get(&request2);
requestX.url("/api/thermostat/info");
EMSESP::webAPIService.webAPIService_get(&requestX);
emsesp::EMSESP::logger().notice("*");
request2.url("/api/thermostat/seltemp");
EMSESP::webAPIService.webAPIService_get(&request2);
requestX.url("/api/thermostat/seltemp");
EMSESP::webAPIService.webAPIService_get(&requestX);
return;
*/

/*
request2.url("/api/system/restart");
EMSESP::webAPIService.webAPIService_get(&request2);
requestX.url("/api/system/restart");
EMSESP::webAPIService.webAPIService_get(&requestX);
return;
*/

/*
request2.url("/api/dallassensor/fdfd");
EMSESP::webAPIService.webAPIService_get(&request2);
requestX.url("/api/dallassensor/xxxx");
EMSESP::webAPIService.webAPIService_get(&requestX);
emsesp::EMSESP::logger().notice("****");
request2.url("/api/dallassensor/info");
EMSESP::webAPIService.webAPIService_get(&request2);
requestX.url("/api/dallassensor/info");
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");
request2.url("/api/thermostat");
EMSESP::webAPIService.webAPIService_post(&request2, jsonX);
// requestX.url("/api/system/send");
requestX.url("/api/thermostat");
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
EMSESP::webAPIService.webAPIService_post(&request3, jsonX);
requestX.url("/api/thermostat/mode/auto"); // should fail
EMSESP::webAPIService.webAPIService_post(&requestX, jsonX);
return;
*/

// test command parse
Expand Down Expand Up @@ -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

// various MQTT checks
/*
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;
*/
EMSESP::mqtt_.incoming("ems-esp/thermostat/mode/auto", "auto"); // invalid, not allowed

// check long base
// check extended MQTT base
Mqtt::base("home/cellar/heating");
EMSESP::mqtt_.incoming("home/cellar/heating/thermostat/mode"); // empty payload, sends reponse

Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 5f2a9b0

Please sign in to comment.