Fix URL parameter decoding in web server (#3313)

* Make HTTP server test data easier to examine

* Add HTTP server parameter tests containing & and =

* Fix URL parameter decoding in web server

The parameters string needs to be first split on & and =, and URL
decoding on parts done after that. Otherwise URL encoded & and = within
parameter names and values cause incorrect splitting.

Author: scop

libraries/ESP8266WebServer/src/Parsing.cpp

@@ -184,13 +184,9 @@ bool ESP8266WebServer::_parseRequest(WiFiClient& client) {
       	return false;
       }
       if (contentLength > 0) {
-        if (searchStr != "") searchStr += '&';
         if(isEncoded){
           //url encoded form
-          String decoded = urlDecode(plainBuf);
-          size_t decodedLen = decoded.length();
-          memcpy(plainBuf, decoded.c_str(), decodedLen);
-          plainBuf[decodedLen] = 0;
+          if (searchStr != "") searchStr += '&';
           searchStr += plainBuf;
         }
         _parseArguments(searchStr);
@@ -321,7 +317,7 @@ void ESP8266WebServer::_parseArguments(String data) {
       continue;
     }
     RequestArgument& arg = _currentArgs[iarg];
-    arg.key = data.substring(pos, equal_sign_index);
+    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 ");

tests/device/test_http_server/test_http_server.ino

@@ -36,16 +36,16 @@ TEST_CASE("HTTP GET Parameters", "[HTTPServer]")
             siteData = "";
             for (uint8_t i=0; i<server.args(); i++){
                 if(i > 0)
-                    siteData += "&";
-                siteData += server.argName(i) + "=" + server.arg(i);
+                    siteData += "\n";
+                siteData += server.argName(i) + " = " + server.arg(i);
             }
             siteHits++;
             server.send(200, "text/plain", siteData);
         });
         uint32_t startTime = millis();
         while(siteHits == 0 && (millis() - startTime) < 10000)
             server.handleClient();
-        REQUIRE(siteHits > 0 && siteData.equals("var1=val with spaces&var+=some%"));
+        REQUIRE(siteHits > 0 && siteData.equals("var1 = val with spaces\nva=r+ = so&me%"));
     }
 }
 
@@ -57,16 +57,16 @@ TEST_CASE("HTTP POST Parameters", "[HTTPServer]")
             siteData = "";
             for (uint8_t i=0; i<server.args(); i++){
                 if(i > 0)
-                    siteData += "&";
-                siteData += server.argName(i) + "=" + server.arg(i);
+                    siteData += "\n";
+                siteData += server.argName(i) + " = " + server.arg(i);
             }
             siteHits++;
             server.send(200, "text/plain", siteData);
         });
         uint32_t startTime = millis();
         while(siteHits == 0 && (millis() - startTime) < 10000)
             server.handleClient();
-        REQUIRE(siteHits > 0 && siteData.equals("var2=val with spaces"));
+        REQUIRE(siteHits > 0 && siteData.equals("var2 = val with spaces"));
     }
 }
 
@@ -78,16 +78,16 @@ TEST_CASE("HTTP GET+POST Parameters", "[HTTPServer]")
             siteData = "";
             for (uint8_t i=0; i<server.args(); i++){
                 if(i > 0)
-                    siteData += "&";
-                siteData += server.argName(i) + "=" + server.arg(i);
+                    siteData += "\n";
+                siteData += server.argName(i) + " = " + server.arg(i);
             }
             siteHits++;
             server.send(200, "text/plain", siteData);
         });
         uint32_t startTime = millis();
         while(siteHits == 0 && (millis() - startTime) < 10000)
             server.handleClient();
-        REQUIRE(siteHits > 0 && siteData.equals("var3=val with spaces&var+=some%"));
+        REQUIRE(siteHits > 0 && siteData.equals("var3 = val with spaces\nva&r+ = so=me%"));
     }
 }
 
@@ -98,8 +98,8 @@ TEST_CASE("HTTP Upload", "[HTTPServer]")
         server.on("/upload", HTTP_POST, [](){
             for (uint8_t i=0; i<server.args(); i++){
                 if(i > 0)
-                    siteData += "&";
-                siteData += server.argName(i) + "=" + server.arg(i);
+                    siteData += "\n";
+                siteData += server.argName(i) + " = " + server.arg(i);
             }
             siteHits++;
             server.send(200, "text/plain", siteData);
@@ -110,13 +110,13 @@ TEST_CASE("HTTP Upload", "[HTTPServer]")
             } else if(upload.status == UPLOAD_FILE_END){
               siteData.concat(":");
               siteData.concat(String(upload.totalSize));
-              siteData.concat("&");
+              siteData.concat("\n");
             }
         });
         uint32_t startTime = millis();
         while(siteHits == 0 && (millis() - startTime) < 10000)
             server.handleClient();
-        REQUIRE(siteHits > 0 && siteData.equals("test.txt:16&var4=val with spaces"));
+        REQUIRE(siteHits > 0 && siteData.equals("test.txt:16\nvar4 = val with spaces"));
     }
 }
 

tests/device/test_http_server/test_http_server.py

@@ -24,7 +24,7 @@ def http_test(res, url, get=None, post=None):
 @setup('HTTP GET Parameters')
 def setup_http_get_params(e):
     def testRun():
-        return http_test('var1=val with spaces&var+=some%', 'http://etd.local/get', {'var1' : 'val with spaces', 'var+' : 'some%'})
+        return http_test('var1 = val with spaces\nva=r+ = so&me%', 'http://etd.local/get', {'var1' : 'val with spaces', 'va=r+' : 'so&me%'})
     Thread(target=testRun).start()
 
 @teardown('HTTP GET Parameters')
@@ -34,7 +34,7 @@ def teardown_http_get_params(e):
 @setup('HTTP POST Parameters')
 def setup_http_post_params(e):
     def testRun():
-        return http_test('var2=val with spaces', 'http://etd.local/post', None, {'var2' : 'val with spaces'})
+        return http_test('var2 = val with spaces', 'http://etd.local/post', None, {'var2' : 'val with spaces'})
     Thread(target=testRun).start()
 
 @teardown('HTTP POST Parameters')
@@ -44,7 +44,7 @@ def teardown_http_post_params(e):
 @setup('HTTP GET+POST Parameters')
 def setup_http_getpost_params(e):
     def testRun():
-        return http_test('var3=val with spaces&var+=some%', 'http://etd.local/get_and_post', {'var3' : 'val with spaces'}, {'var+' : 'some%'})
+        return http_test('var3 = val with spaces\nva&r+ = so=me%', 'http://etd.local/get_and_post', {'var3' : 'val with spaces'}, {'va&r+' : 'so=me%'})
     Thread(target=testRun).start()
 
 @teardown('HTTP GET+POST Parameters')
@@ -63,7 +63,7 @@ def testRun():
             response = urllib2.urlopen(request, None, 2).read()
         except:
             return 1
-        if response != 'test.txt:16&var4=val with spaces':
+        if response != 'test.txt:16\nvar4 = val with spaces':
             return 1
         return 0
     Thread(target=testRun).start()