Proposal for fixing file webserver uploads via form upload.

The form parser has shown to have issues with files ending with "--\r\n". This commit replaces the form parser with the parser from ESP8266, which passes the test case.
espressif · Jan 23, 2024 · bd941e3 · bd941e3
1 parent cceebb5
commit bd941e3
Showing 1 changed file with 49 additions and 78 deletions.
diff --git a/libraries/WebServer/src/Parsing.cpp b/libraries/WebServer/src/Parsing.cpp
@@ -436,88 +436,59 @@ bool WebServer::_parseForm(WiFiClient& client, String boundary, uint32_t len){
             if(_currentHandler && _currentHandler->canUpload(_currentUri))
               _currentHandler->upload(*this, _currentUri, *_currentUpload);
             _currentUpload->status = UPLOAD_FILE_WRITE;
-            int argByte = _uploadReadByte(client);
-readfile:
 
-            while(argByte != 0x0D){
-                if(argByte < 0) return _parseFormUploadAborted();
-                _uploadWriteByte(argByte);
-                argByte = _uploadReadByte(client);
-            }
-
-            argByte = _uploadReadByte(client);
-            if(argByte < 0) return _parseFormUploadAborted();
-            if (argByte == 0x0A){
-              argByte = _uploadReadByte(client);
-              if(argByte < 0) return _parseFormUploadAborted();
-              if ((char)argByte != '-'){
-                //continue reading the file
-                _uploadWriteByte(0x0D);
-                _uploadWriteByte(0x0A);
-                goto readfile;
-              } else {
-                argByte = _uploadReadByte(client);
-                if(argByte < 0) return _parseFormUploadAborted();
-                if ((char)argByte != '-'){
-                  //continue reading the file
-                  _uploadWriteByte(0x0D);
-                  _uploadWriteByte(0x0A);
-                  _uploadWriteByte((uint8_t)('-'));
-                  goto readfile;
-                }
-              }
-
-              uint8_t endBuf[boundary.length()];
-              uint32_t i = 0;
-              while(i < boundary.length()){
-                argByte = _uploadReadByte(client);
-                if(argByte < 0) return _parseFormUploadAborted();
-                if ((char)argByte == 0x0D){
-                  _uploadWriteByte(0x0D);
-                  _uploadWriteByte(0x0A);
-                  _uploadWriteByte((uint8_t)('-'));
-                  _uploadWriteByte((uint8_t)('-'));
-                  uint32_t j = 0;
-                  while(j < i){
-                    _uploadWriteByte(endBuf[j++]);
-                  }
-                  goto readfile;
+            int fastBoundaryLen = 4 /* \r\n-- */ + boundary.length() + 1 /* \0 */;
+            char fastBoundary[ fastBoundaryLen ];
+            snprintf(fastBoundary, fastBoundaryLen, "\r\n--%s", boundary.c_str());
+            int boundaryPtr = 0;
+            while ( true ) {
+                int ret = _uploadReadByte(client);
+                if (ret < 0) {
+                    // Unexpected, we should have had data available per above
+                    return _parseFormUploadAborted();
                 }
-                endBuf[i++] = (uint8_t)argByte;
-              }
-
-              if (strstr((const char*)endBuf, boundary.c_str()) != NULL){
-                if(_currentHandler && _currentHandler->canUpload(_currentUri))
-                  _currentHandler->upload(*this, _currentUri, *_currentUpload);
-                _currentUpload->totalSize += _currentUpload->currentSize;
-                _currentUpload->status = UPLOAD_FILE_END;
-                if(_currentHandler && _currentHandler->canUpload(_currentUri))
-                  _currentHandler->upload(*this, _currentUri, *_currentUpload);
-                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 == "--"){
-                  log_v("Done Parsing POST");
-                  break;
+                char in = (char) ret;
+                if (in == fastBoundary[ boundaryPtr ]) {
+                    // The input matched the current expected character, advance and possibly exit this file
+                    boundaryPtr++;
+                    if (boundaryPtr == fastBoundaryLen - 1) {
+                        // We read the whole boundary line, we're done here!
+                        break;
+                    }
+                } else {
+                    // The char doesn't match what we want, so dump whatever matches we had, the read in char, and reset ptr to start
+                    for (int i = 0; i < boundaryPtr; i++) {
+                        _uploadWriteByte( fastBoundary[ i ] );
+                    }
+                    if (in == fastBoundary[ 0 ]) {
+                       // This could be the start of the real end, mark it so and don't emit/skip it
+                       boundaryPtr = 1;
+                    } else {
+                      // Not the 1st char of our pattern, so emit and ignore
+                      _uploadWriteByte( in );
+                      boundaryPtr = 0;
+                    }
                 }
-                continue;
-              } else {
-                _uploadWriteByte(0x0D);
-                _uploadWriteByte(0x0A);
-                _uploadWriteByte((uint8_t)('-'));
-                _uploadWriteByte((uint8_t)('-'));
-                uint32_t i = 0;
-                while(i < boundary.length()){
-                  _uploadWriteByte(endBuf[i++]);
-                }
-                argByte = _uploadReadByte(client);
-                goto readfile;
-              }
-            } else {
-              _uploadWriteByte(0x0D);
-              goto readfile;
             }
-            break;
+            // Found the boundary string, finish processing this file upload
+            if (_currentHandler && _currentHandler->canUpload(_currentUri))
+                _currentHandler->upload(*this, _currentUri, *_currentUpload);
+            _currentUpload->totalSize += _currentUpload->currentSize;
+            _currentUpload->status = UPLOAD_FILE_END;
+            if (_currentHandler && _currentHandler->canUpload(_currentUri))
+                _currentHandler->upload(*this, _currentUri, *_currentUpload);
+            log_v("End File: %s Type: %s Size: %d",
+                _currentUpload->filename.c_str(),
+                _currentUpload->type.c_str(),
+                (int)_currentUpload->totalSize);
+            if (!client.connected()) return _parseFormUploadAborted();
+            line = client.readStringUntil('\r');
+            client.readStringUntil('\n');
+            if (line == "--") {     // extra two dashes mean we reached the end of all form fields
+                log_v("Done Parsing POST");
+                break;
+            }
+            continue;
           }
         }
       }