fix parseArgument #2 (#5252)

* fix parseArgument

* remove recursion

* keep variable naming style consistent

* inviting Alexa

* fix alexa fix (well, trying...)

* fix pos substrings

key_end_pos--;
dont count down here, it will cut of every key by -1 ("save" will be "sav") (substring  (end = up to, but not including, so no need to -1)

Parsing cpp L329
arg.value = urlDecode(data.substring(equal_index + 1, next_index - 1));
=> -1 is too less for substring (substring  (end = up to, but not including, so no need to -1)

* alexa invite: add workaround for malformed x-www-form-urlencoded

* when !form, alway add content in key "plain"

* fix memleak

* parse arguments: use functors

* cleaning

libraries/ESP8266WebServer/src/ESP8266WebServer.h

@@ -142,7 +142,8 @@ protected:
   void _handleRequest();
   void _finalizeResponse();
   bool _parseRequest(WiFiClient& client);
-  void _parseArguments(String data);
+  void _parseArguments(const String& data);
+  int _parseArgumentsPrivate(const String& data, std::function<void(String&,String&,const String&,int,int,int,int)> handler);
   static String _responseCodeToString(int code);
   bool _parseForm(WiFiClient& client, String boundary, uint32_t len);
   bool _parseFormUploadAborted();

libraries/ESP8266WebServer/src/Parsing.cpp

@@ -139,7 +139,7 @@ bool ESP8266WebServer::_parseRequest(WiFiClient& client) {
     String headerName;
     String headerValue;
     bool isForm = false;
-    bool isEncoded = false;
+    //bool isEncoded = false;
     uint32_t contentLength = 0;
     //parse headers
     while(1){
@@ -168,7 +168,7 @@ bool ESP8266WebServer::_parseRequest(WiFiClient& client) {
           isForm = false;
         } else if (headerValue.startsWith(F("application/x-www-form-urlencoded"))){
           isForm = false;
-          isEncoded = true;
+          //isEncoded = true;
         } else if (headerValue.startsWith(F("multipart/"))){
           boundaryStr = headerValue.substring(headerValue.indexOf('=') + 1);
           boundaryStr.replace("\"","");
@@ -181,43 +181,34 @@ bool ESP8266WebServer::_parseRequest(WiFiClient& client) {
       }
     }
 
-    if (!isForm){
+    // always parse url for key/value pairs
+    _parseArguments(searchStr);
+
+    if (!isForm) {
+      if (contentLength) {
+
+        // add key=value: plain={body} (post json or other data)
+
         size_t plainLength;
         char* plainBuf = readBytesWithTimeout(client, contentLength, plainLength, HTTP_MAX_POST_WAIT);
         if (plainLength < contentLength) {
           free(plainBuf);
           return false;
         }
-      if (contentLength > 0) {
+
-        if(isEncoded){
-          //url encoded form
-          if (searchStr != "") searchStr += '&';
-          searchStr += plainBuf;
-        }
-        _parseArguments(searchStr);
-        if(!isEncoded){
-          //plain post json or other data
         RequestArgument& arg = _currentArgs[_currentArgCount++];
         arg.key = F("plain");
         arg.value = String(plainBuf);
-        }
 
-  #ifdef DEBUG_ESP_HTTP_SERVER
-        DEBUG_OUTPUT.print("Plain: ");
-        DEBUG_OUTPUT.println(plainBuf);
-  #endif
         free(plainBuf);
-      } else {
-        // No content - but we can still have arguments in the URL.
-        _parseArguments(searchStr);
-      }
-    }
 
-    if (isForm){
+      }
-      _parseArguments(searchStr);
+    } else { // isForm is true
+
       if (!_parseForm(client, boundaryStr, contentLength)) {
         return false;
       }
+
     }
   } else {
     String headerName;
@@ -235,12 +226,12 @@ bool ESP8266WebServer::_parseRequest(WiFiClient& client) {
       headerValue = req.substring(headerDiv + 2);
       _collectHeader(headerName.c_str(),headerValue.c_str());
 
-	  #ifdef DEBUG_ESP_HTTP_SERVER
+#ifdef DEBUG_ESP_HTTP_SERVER
-	  DEBUG_OUTPUT.print("headerName: ");
+      DEBUG_OUTPUT.print(F("headerName: "));
       DEBUG_OUTPUT.println(headerName);
-	  DEBUG_OUTPUT.print("headerValue: ");
+      DEBUG_OUTPUT.print(F("headerValue: "));
       DEBUG_OUTPUT.println(headerValue);
-	  #endif
+#endif
 
       if (headerName.equalsIgnoreCase("Host")){
         _hostHeader = headerValue;
@@ -251,10 +242,16 @@ bool ESP8266WebServer::_parseRequest(WiFiClient& client) {
   client.flush();
 
 #ifdef DEBUG_ESP_HTTP_SERVER
-  DEBUG_OUTPUT.print("Request: ");
+  DEBUG_OUTPUT.print(F("Request: "));
   DEBUG_OUTPUT.println(url);
-  DEBUG_OUTPUT.print(" Arguments: ");
+  DEBUG_OUTPUT.print(F("Arguments: "));
   DEBUG_OUTPUT.println(searchStr);
+
+  DEBUG_OUTPUT.println(F("final list of key/value pairs:"));
+  for (int i = 0; i < _currentArgCount; i++)
+    DEBUG_OUTPUT.printf("  key:'%s' value:'%s'\r\n",
+      _currentArgs[i].key.c_str(),
+      _currentArgs[i].value.c_str());
 #endif
 
   return true;
@@ -270,79 +267,85 @@ bool ESP8266WebServer::_collectHeader(const char* headerName, const char* header
   return false;
 }
 
-void ESP8266WebServer::_parseArguments(String data) {
+struct storeArgHandler
+{
+  void operator() (String& key, String& value, const String& data, int equal_index, int pos, int key_end_pos, int next_index)
+  {
+    key = ESP8266WebServer::urlDecode(data.substring(pos, key_end_pos));
+    if ((equal_index != -1) && ((equal_index < next_index - 1) || (next_index == -1)))
+      value = ESP8266WebServer::urlDecode(data.substring(equal_index + 1, next_index));
+  }
+};
+
+struct nullArgHandler
+{
+  void operator() (String& key, String& value, const String& data, int equal_index, int pos, int key_end_pos, int next_index) {
+    (void)key; (void)value; (void)data; (void)equal_index; (void)pos; (void)key_end_pos; (void)next_index;
+    // do nothing
+  }
+};
+
+void ESP8266WebServer::_parseArguments(const String& data) {
+  if (_currentArgs)
+    delete[] _currentArgs;
+
+  _currentArgCount = _parseArgumentsPrivate(data, nullArgHandler());
+
+  // allocate one more, this is needed because {"plain": plainBuf} is always added
+  _currentArgs = new RequestArgument[_currentArgCount + 1];
+
+  (void)_parseArgumentsPrivate(data, storeArgHandler());
+}
+
+int ESP8266WebServer::_parseArgumentsPrivate(const String& data, std::function<void(String&,String&,const String&,int,int,int,int)> handler) {
+
 #ifdef DEBUG_ESP_HTTP_SERVER
   DEBUG_OUTPUT.print("args: ");
   DEBUG_OUTPUT.println(data);
 #endif
-  if (_currentArgs)
-    delete[] _currentArgs;
-  _currentArgs = 0;
-  if (data.length() == 0) {
-    _currentArgCount = 0;
-    _currentArgs = new RequestArgument[1];
-    return;
-  }
-  _currentArgCount = 1;
 
-  for (int i = 0; i < (int)data.length(); ) {
+  size_t pos = 0;
-    i = data.indexOf('&', i);
+  int arg_total = 0;
-    if (i == -1)
+
+  while (true) {
+
+    // skip empty expression
+    while (data[pos] == '&' || data[pos] == ';')
+      if (++pos >= data.length())
         break;
-    ++i;
+
-    ++_currentArgCount;
+    // locate separators
+    int equal_index = data.indexOf('=', pos);
+    int key_end_pos = equal_index;
+    int next_index = data.indexOf('&', pos);
+    int next_index2 = data.indexOf(';', pos);
+    if ((next_index == -1) || (next_index2 != -1 && next_index2 < next_index))
+      next_index = next_index2;
+    if ((key_end_pos == -1) || ((key_end_pos > next_index) && (next_index != -1)))
+      key_end_pos = next_index;
+    if (key_end_pos == -1)
+      key_end_pos = data.length();
+
+    // handle key/value
+    if ((int)pos < key_end_pos) {
+
+      RequestArgument& arg = _currentArgs[arg_total];
+      handler(arg.key, arg.value, data, equal_index, pos, key_end_pos, next_index);
+
+      ++arg_total;
+      pos = next_index + 1;
     }
+
+    if (next_index == -1)
+      break;
+  }
+
 #ifdef DEBUG_ESP_HTTP_SERVER
   DEBUG_OUTPUT.print("args count: ");
-  DEBUG_OUTPUT.println(_currentArgCount);
+  DEBUG_OUTPUT.println(arg_total);
-#endif
-
-  _currentArgs = new RequestArgument[_currentArgCount+1];
-  int pos = 0;
-  int iarg;
-  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
-    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
-      if (next_arg_index == -1)
-        break;
-      pos = next_arg_index + 1;
-      continue;
-    }
-    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
-    ++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
 
+  return arg_total;
 }
 
 void ESP8266WebServer::_uploadWriteByte(uint8_t b){