From 29d59876b4eb0c62567c97e2731fa7c8c0e3f2fd Mon Sep 17 00:00:00 2001 From: Neal MIller Date: Tue, 22 Jan 2019 03:30:10 -0600 Subject: [PATCH] Webserver library - fix logging (#2355) (#2359) * Webserver fix logging (#1) * Change logging to use esp32-hal-log.h fixes #2355 * adjust log parameter output positions, reduce lines The DEBUG_ESP method used less lines than I originally set `log_v` to use when displaying the details of the received params ("@" and "=" indexes, and File info on a single line) --- libraries/WebServer/src/Parsing.cpp | 148 +++++--------------------- libraries/WebServer/src/WebServer.cpp | 35 ++---- 2 files changed, 35 insertions(+), 148 deletions(-) diff --git a/libraries/WebServer/src/Parsing.cpp b/libraries/WebServer/src/Parsing.cpp index 40300aeaa60..e55e6873c8f 100644 --- a/libraries/WebServer/src/Parsing.cpp +++ b/libraries/WebServer/src/Parsing.cpp @@ -20,18 +20,12 @@ */ #include +#include #include "WiFiServer.h" #include "WiFiClient.h" #include "WebServer.h" #include "detail/mimetable.h" -//#define DEBUG_ESP_HTTP_SERVER -#ifdef DEBUG_ESP_PORT -#define DEBUG_OUTPUT DEBUG_ESP_PORT -#else -#define DEBUG_OUTPUT Serial -#endif - #ifndef WEBSERVER_MAX_POST_ARGS #define WEBSERVER_MAX_POST_ARGS 32 #endif @@ -85,10 +79,7 @@ bool WebServer::_parseRequest(WiFiClient& client) { int addr_start = req.indexOf(' '); int addr_end = req.indexOf(' ', addr_start + 1); if (addr_start == -1 || addr_end == -1) { -#ifdef DEBUG_ESP_HTTP_SERVER - DEBUG_OUTPUT.print("Invalid request: "); - DEBUG_OUTPUT.println(req); -#endif + log_e("Invalid request: %s", req.c_str()); return false; } @@ -119,14 +110,7 @@ bool WebServer::_parseRequest(WiFiClient& client) { } _currentMethod = method; -#ifdef DEBUG_ESP_HTTP_SERVER - DEBUG_OUTPUT.print("method: "); - DEBUG_OUTPUT.print(methodStr); - DEBUG_OUTPUT.print(" url: "); - DEBUG_OUTPUT.print(url); - DEBUG_OUTPUT.print(" search: "); - DEBUG_OUTPUT.println(searchStr); -#endif + log_v("method: %s url: %s search: %s", methodStr.c_str(), url.c_str(), searchStr.c_str()); //attach handler RequestHandler* handler; @@ -159,12 +143,8 @@ bool WebServer::_parseRequest(WiFiClient& client) { headerValue.trim(); _collectHeader(headerName.c_str(),headerValue.c_str()); - #ifdef DEBUG_ESP_HTTP_SERVER - DEBUG_OUTPUT.print("headerName: "); - DEBUG_OUTPUT.println(headerName); - DEBUG_OUTPUT.print("headerValue: "); - DEBUG_OUTPUT.println(headerValue); - #endif + log_v("headerName: %s", headerName.c_str()); + log_v("headerValue: %s", headerValue.c_str()); if (headerName.equalsIgnoreCase(FPSTR(Content_Type))){ using namespace mime; @@ -206,10 +186,7 @@ bool WebServer::_parseRequest(WiFiClient& client) { arg.value = String(plainBuf); } - #ifdef DEBUG_ESP_HTTP_SERVER - DEBUG_OUTPUT.print("Plain: "); - DEBUG_OUTPUT.println(plainBuf); - #endif + log_v("Plain: %s", plainBuf); free(plainBuf); } else { // No content - but we can still have arguments in the URL. @@ -239,12 +216,8 @@ bool WebServer::_parseRequest(WiFiClient& client) { headerValue = req.substring(headerDiv + 2); _collectHeader(headerName.c_str(),headerValue.c_str()); - #ifdef DEBUG_ESP_HTTP_SERVER - DEBUG_OUTPUT.print("headerName: "); - DEBUG_OUTPUT.println(headerName); - DEBUG_OUTPUT.print("headerValue: "); - DEBUG_OUTPUT.println(headerValue); - #endif + log_v("headerName: %s", headerName.c_str()); + log_v("headerValue: %s", headerValue.c_str()); if (headerName.equalsIgnoreCase("Host")){ _hostHeader = headerValue; @@ -254,12 +227,8 @@ bool WebServer::_parseRequest(WiFiClient& client) { } client.flush(); -#ifdef DEBUG_ESP_HTTP_SERVER - DEBUG_OUTPUT.print("Request: "); - DEBUG_OUTPUT.println(url); - DEBUG_OUTPUT.print(" Arguments: "); - DEBUG_OUTPUT.println(searchStr); -#endif + log_v("Request: %s", url.c_str()); + log_v(" Arguments: %s", searchStr.c_str()); return true; } @@ -275,10 +244,7 @@ bool WebServer::_collectHeader(const char* headerName, const char* headerValue) } void WebServer::_parseArguments(String data) { -#ifdef DEBUG_ESP_HTTP_SERVER - DEBUG_OUTPUT.print("args: "); - DEBUG_OUTPUT.println(data); -#endif + log_v("args: %s", data.c_str()); if (_currentArgs) delete[] _currentArgs; _currentArgs = 0; @@ -296,10 +262,7 @@ void WebServer::_parseArguments(String data) { ++i; ++_currentArgCount; } -#ifdef DEBUG_ESP_HTTP_SERVER - DEBUG_OUTPUT.print("args count: "); - DEBUG_OUTPUT.println(_currentArgCount); -#endif + log_v("args count: %d", _currentArgCount); _currentArgs = new RequestArgument[_currentArgCount+1]; int pos = 0; @@ -307,19 +270,9 @@ void WebServer::_parseArguments(String data) { for (iarg = 0; iarg < _currentArgCount;) { int equal_sign_index = data.indexOf('=', pos); int next_arg_index = data.indexOf('&', pos); -#ifdef DEBUG_ESP_HTTP_SERVER - DEBUG_OUTPUT.print("pos "); - DEBUG_OUTPUT.print(pos); - DEBUG_OUTPUT.print("=@ "); - DEBUG_OUTPUT.print(equal_sign_index); - DEBUG_OUTPUT.print(" &@ "); - DEBUG_OUTPUT.println(next_arg_index); -#endif + log_v("pos %d =@%d &@%d", pos, equal_sign_index, next_arg_index); if ((equal_sign_index == -1) || ((equal_sign_index > next_arg_index) && (next_arg_index != -1))) { -#ifdef DEBUG_ESP_HTTP_SERVER - DEBUG_OUTPUT.print("arg missing value: "); - DEBUG_OUTPUT.println(iarg); -#endif + log_e("arg missing value: %d", iarg); if (next_arg_index == -1) break; pos = next_arg_index + 1; @@ -328,24 +281,14 @@ void WebServer::_parseArguments(String data) { RequestArgument& arg = _currentArgs[iarg]; arg.key = urlDecode(data.substring(pos, equal_sign_index)); arg.value = urlDecode(data.substring(equal_sign_index + 1, next_arg_index)); -#ifdef DEBUG_ESP_HTTP_SERVER - DEBUG_OUTPUT.print("arg "); - DEBUG_OUTPUT.print(iarg); - DEBUG_OUTPUT.print(" key: "); - DEBUG_OUTPUT.print(arg.key); - DEBUG_OUTPUT.print(" value: "); - DEBUG_OUTPUT.println(arg.value); -#endif + log_v("arg %d key: %s value: %s", iarg, arg.key.c_str(), arg.value.c_str()); ++iarg; if (next_arg_index == -1) break; pos = next_arg_index + 1; } _currentArgCount = iarg; -#ifdef DEBUG_ESP_HTTP_SERVER - DEBUG_OUTPUT.print("args count: "); - DEBUG_OUTPUT.println(_currentArgCount); -#endif + log_v("args count: %d", _currentArgCount); } @@ -371,12 +314,7 @@ int WebServer::_uploadReadByte(WiFiClient& client){ bool WebServer::_parseForm(WiFiClient& client, String boundary, uint32_t len){ (void) len; -#ifdef DEBUG_ESP_HTTP_SERVER - DEBUG_OUTPUT.print("Parse Form: Boundary: "); - DEBUG_OUTPUT.print(boundary); - DEBUG_OUTPUT.print(" Length: "); - DEBUG_OUTPUT.println(len); -#endif + log_v("Parse Form: Boundary: %s Length: %d", boundary.c_str(), len); String line; int retry = 0; do { @@ -410,18 +348,12 @@ bool WebServer::_parseForm(WiFiClient& client, String boundary, uint32_t len){ argFilename = argName.substring(nameStart+2, argName.length() - 1); argName = argName.substring(0, argName.indexOf('"')); argIsFile = true; -#ifdef DEBUG_ESP_HTTP_SERVER - DEBUG_OUTPUT.print("PostArg FileName: "); - DEBUG_OUTPUT.println(argFilename); -#endif + log_v("PostArg FileName: %s",argFilename.c_str()); //use GET to set the filename if uploading using blob - if (argFilename == F("blob") && hasArg(FPSTR(filename))) + if (argFilename == F("blob") && hasArg(FPSTR(filename))) argFilename = arg(FPSTR(filename)); } -#ifdef DEBUG_ESP_HTTP_SERVER - DEBUG_OUTPUT.print("PostArg Name: "); - DEBUG_OUTPUT.println(argName); -#endif + log_v("PostArg Name: %s", argName.c_str()); using namespace mime; argType = FPSTR(mimeTable[txt].mimeType); line = client.readStringUntil('\r'); @@ -432,10 +364,7 @@ bool WebServer::_parseForm(WiFiClient& client, String boundary, uint32_t len){ client.readStringUntil('\r'); client.readStringUntil('\n'); } -#ifdef DEBUG_ESP_HTTP_SERVER - DEBUG_OUTPUT.print("PostArg Type: "); - DEBUG_OUTPUT.println(argType); -#endif + log_v("PostArg Type: %s", argType.c_str()); if (!argIsFile){ while(1){ line = client.readStringUntil('\r'); @@ -444,20 +373,14 @@ bool WebServer::_parseForm(WiFiClient& client, String boundary, uint32_t len){ if (argValue.length() > 0) argValue += "\n"; argValue += line; } -#ifdef DEBUG_ESP_HTTP_SERVER - DEBUG_OUTPUT.print("PostArg Value: "); - DEBUG_OUTPUT.println(argValue); - DEBUG_OUTPUT.println(); -#endif + log_v("PostArg Value: %s", argValue.c_str()); RequestArgument& arg = _postArgs[_postArgsLen++]; arg.key = argName; arg.value = argValue; if (line == ("--"+boundary+"--")){ -#ifdef DEBUG_ESP_HTTP_SERVER - DEBUG_OUTPUT.println("Done Parsing POST"); -#endif + log_v("Done Parsing POST"); break; } } else { @@ -468,12 +391,7 @@ bool WebServer::_parseForm(WiFiClient& client, String boundary, uint32_t len){ _currentUpload->type = argType; _currentUpload->totalSize = 0; _currentUpload->currentSize = 0; -#ifdef DEBUG_ESP_HTTP_SERVER - DEBUG_OUTPUT.print("Start File: "); - DEBUG_OUTPUT.print(_currentUpload->filename); - DEBUG_OUTPUT.print(" Type: "); - DEBUG_OUTPUT.println(_currentUpload->type); -#endif + log_v("Start File: %s Type: %s", _currentUpload->filename.c_str(), _currentUpload->type.c_str()); if(_currentHandler && _currentHandler->canUpload(_currentUri)) _currentHandler->upload(*this, _currentUri, *_currentUpload); _currentUpload->status = UPLOAD_FILE_WRITE; @@ -518,20 +436,11 @@ bool WebServer::_parseForm(WiFiClient& client, String boundary, uint32_t len){ _currentUpload->status = UPLOAD_FILE_END; if(_currentHandler && _currentHandler->canUpload(_currentUri)) _currentHandler->upload(*this, _currentUri, *_currentUpload); -#ifdef DEBUG_ESP_HTTP_SERVER - DEBUG_OUTPUT.print("End File: "); - DEBUG_OUTPUT.print(_currentUpload->filename); - DEBUG_OUTPUT.print(" Type: "); - DEBUG_OUTPUT.print(_currentUpload->type); - DEBUG_OUTPUT.print(" Size: "); - DEBUG_OUTPUT.println(_currentUpload->totalSize); -#endif + log_v("End File: %s Type: %s Size: %d", _currentUpload->filename.c_str(), _currentUpload->type.c_str(), _currentUpload->totalSize); line = client.readStringUntil(0x0D); client.readStringUntil(0x0A); if (line == "--"){ -#ifdef DEBUG_ESP_HTTP_SERVER - DEBUG_OUTPUT.println("Done Parsing POST"); -#endif + log_v("Done Parsing POST"); break; } continue; @@ -579,10 +488,7 @@ bool WebServer::_parseForm(WiFiClient& client, String boundary, uint32_t len){ } return true; } -#ifdef DEBUG_ESP_HTTP_SERVER - DEBUG_OUTPUT.print("Error: line: "); - DEBUG_OUTPUT.println(line); -#endif + log_e("Error: line: %s", line.c_str()); return false; } diff --git a/libraries/WebServer/src/WebServer.cpp b/libraries/WebServer/src/WebServer.cpp index 819e338bbf5..2057b230eea 100644 --- a/libraries/WebServer/src/WebServer.cpp +++ b/libraries/WebServer/src/WebServer.cpp @@ -22,6 +22,7 @@ #include +#include #include #include "WiFiServer.h" #include "WiFiClient.h" @@ -30,12 +31,6 @@ #include "detail/RequestHandlersImpl.h" #include "mbedtls/md5.h" -//#define DEBUG_ESP_HTTP_SERVER -#ifdef DEBUG_ESP_PORT -#define DEBUG_OUTPUT DEBUG_ESP_PORT -#else -#define DEBUG_OUTPUT Serial -#endif static const char AUTHORIZATION_HEADER[] = "Authorization"; static const char qop_auth[] = "qop=auth"; @@ -163,9 +158,7 @@ bool WebServer::authenticate(const char * username, const char * password){ delete[] encoded; } else if(authReq.startsWith(F("Digest"))) { authReq = authReq.substring(7); - #ifdef DEBUG_ESP_HTTP_SERVER - DEBUG_OUTPUT.println(authReq); - #endif + log_v("%s", authReq.c_str()); String _username = _extractParam(authReq,F("username=\"")); if(!_username.length() || _username != String(username)) { authReq = ""; @@ -193,9 +186,7 @@ bool WebServer::authenticate(const char * username, const char * password){ _cnonce = _extractParam(authReq, F("cnonce=\"")); } String _H1 = md5str(String(username) + ':' + _realm + ':' + String(password)); - #ifdef DEBUG_ESP_HTTP_SERVER - DEBUG_OUTPUT.println("Hash of user:realm:pass=" + _H1); - #endif + log_v("Hash of user:realm:pass=%s", _H1.c_str()); String _H2 = ""; if(_currentMethod == HTTP_GET){ _H2 = md5str(String(F("GET:")) + _uri); @@ -208,18 +199,14 @@ bool WebServer::authenticate(const char * username, const char * password){ }else{ _H2 = md5str(String(F("GET:")) + _uri); } - #ifdef DEBUG_ESP_HTTP_SERVER - DEBUG_OUTPUT.println("Hash of GET:uri=" + _H2); - #endif + log_v("Hash of GET:uri=%s", _H2.c_str()); String _responsecheck = ""; if(authReq.indexOf(FPSTR(qop_auth)) != -1) { _responsecheck = md5str(_H1 + ':' + _nonce + ':' + _nc + ':' + _cnonce + F(":auth:") + _H2); } else { _responsecheck = md5str(_H1 + ':' + _nonce + ':' + _H2); } - #ifdef DEBUG_ESP_HTTP_SERVER - DEBUG_OUTPUT.println("The Proper response=" +_responsecheck); - #endif + log_v("The Proper response=%s", _responsecheck.c_str()); if(_response == _responsecheck){ authReq = ""; return true; @@ -294,9 +281,7 @@ void WebServer::handleClient() { return; } -#ifdef DEBUG_ESP_HTTP_SERVER - DEBUG_OUTPUT.println("New client"); -#endif + log_v("New client"); _currentClient = client; _currentStatus = HC_WAIT_READ; @@ -614,17 +599,13 @@ void WebServer::onNotFound(THandlerFunction fn) { void WebServer::_handleRequest() { bool handled = false; if (!_currentHandler){ -#ifdef DEBUG_ESP_HTTP_SERVER - DEBUG_OUTPUT.println("request handler not found"); -#endif + log_e("request handler not found"); } else { handled = _currentHandler->handle(*this, _currentMethod, _currentUri); -#ifdef DEBUG_ESP_HTTP_SERVER if (!handled) { - DEBUG_OUTPUT.println("request handler failed to handle request"); + log_e("request handler failed to handle request"); } -#endif } if (!handled && _notFoundHandler) { _notFoundHandler();