From b813a806b59c3cfe8fd17cea8bcdfebe9e0f0943 Mon Sep 17 00:00:00 2001 From: "Earle F. Philhower, III" Date: Sat, 28 Nov 2020 10:53:20 -0800 Subject: [PATCH 1/2] Rewrite multipart boundary detection Use a simpler, cleaner implementation of multipart form detection as defined in https://tools.ietf.org/html/rfc7578 . Implements a simple state machine that detects the \r\n-- stream in input a character at a time, instead of buffering and comparing in chunks which can miss things due to alignment issues and which also had a problem with replacing characters in a binary stream. Fixes #7723 --- libraries/ESP8266WebServer/src/Parsing-impl.h | 58 ++++++++----------- 1 file changed, 23 insertions(+), 35 deletions(-) diff --git a/libraries/ESP8266WebServer/src/Parsing-impl.h b/libraries/ESP8266WebServer/src/Parsing-impl.h index 33d3efd669..af49f81f5e 100644 --- a/libraries/ESP8266WebServer/src/Parsing-impl.h +++ b/libraries/ESP8266WebServer/src/Parsing-impl.h @@ -444,45 +444,33 @@ bool ESP8266WebServerTemplate::_parseForm(ClientType& client, const _currentHandler->upload(*this, _currentUri, *_currentUpload); _currentUpload->status = UPLOAD_FILE_WRITE; - int bLen = boundary.length(); - uint8_t boundBuf[2 + bLen + 1]; // "--" + boundary + null terminator - boundBuf[2 + bLen] = '\0'; - uint8_t argByte; - bool first = true; - while (1) { - //attempt to fill up boundary buffer with length of boundary string - int i; - for (i = 0; i < 2 + bLen; i++) { - if (!client.connected()) return _parseFormUploadAborted(); - argByte = _uploadReadByte(client); - if (argByte == '\r') - break; - boundBuf[i] = argByte; - } - if ((strncmp((const char*)boundBuf, "--", 2) == 0) && (strcmp((const char*)(boundBuf + 2), boundary.c_str()) == 0)) - break; //found the boundary, done parsing this file - if (first) first = false; //only add newline characters after the first line - else { - _uploadWriteByte('\r'); - _uploadWriteByte('\n'); + 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 ) { + if ( !client.connected() ) { + // Unexpected disconnection, abort! + return _parseFormUploadAborted(); } - // current line does not contain boundary, upload all bytes in boundary buffer - for (int j = 0; j < i; j++) - _uploadWriteByte(boundBuf[j]); - // the initial pass (filling up the boundary buffer) did not reach the end of the line. Upload the rest of the line now - if (i >= 2 + bLen) { - if (!client.connected()) return _parseFormUploadAborted(); - argByte = _uploadReadByte(client); - while (argByte != '\r') { - if (!client.connected()) return _parseFormUploadAborted(); - _uploadWriteByte(argByte); - argByte = _uploadReadByte(client); + char in = _uploadReadByte(client); + 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 ] ); } + _uploadWriteByte( in ); + boundaryPtr = 0; } - if (!client.connected()) return _parseFormUploadAborted(); - _uploadReadByte(client); // '\n' } - //Found the boundary string, finish processing this file upload + // Found the boundary string, finish processing this file upload if (_currentHandler && _currentHandler->canUpload(_currentUri)) _currentHandler->upload(*this, _currentUri, *_currentUpload); _currentUpload->totalSize += _currentUpload->currentSize; From 730421e874fdc0d6a90e0339754ec3f8d7057eca Mon Sep 17 00:00:00 2001 From: "Earle F. Philhower, III" Date: Sat, 28 Nov 2020 15:43:03 -0800 Subject: [PATCH 2/2] Use uploadReadByte return code to abort when needed Adjust the private _uploadReadByte function to return -1 on error (like a read()), and the main file upload handler to use that return value instead of duplicating logic. --- libraries/ESP8266WebServer/src/ESP8266WebServer.h | 2 +- libraries/ESP8266WebServer/src/Parsing-impl.h | 11 ++++++----- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/libraries/ESP8266WebServer/src/ESP8266WebServer.h b/libraries/ESP8266WebServer/src/ESP8266WebServer.h index 23b5b328aa..d40313a59d 100644 --- a/libraries/ESP8266WebServer/src/ESP8266WebServer.h +++ b/libraries/ESP8266WebServer/src/ESP8266WebServer.h @@ -246,7 +246,7 @@ class ESP8266WebServerTemplate bool _parseForm(ClientType& client, const String& boundary, uint32_t len); bool _parseFormUploadAborted(); void _uploadWriteByte(uint8_t b); - uint8_t _uploadReadByte(ClientType& client); + int _uploadReadByte(ClientType& client); void _prepareHeader(String& response, int code, const char* content_type, size_t contentLength); bool _collectHeader(const char* headerName, const char* headerValue); diff --git a/libraries/ESP8266WebServer/src/Parsing-impl.h b/libraries/ESP8266WebServer/src/Parsing-impl.h index af49f81f5e..cab0c04f75 100644 --- a/libraries/ESP8266WebServer/src/Parsing-impl.h +++ b/libraries/ESP8266WebServer/src/Parsing-impl.h @@ -347,14 +347,14 @@ void ESP8266WebServerTemplate::_uploadWriteByte(uint8_t b){ } template -uint8_t ESP8266WebServerTemplate::_uploadReadByte(ClientType& client){ +int ESP8266WebServerTemplate::_uploadReadByte(ClientType& client){ int res = client.read(); if(res == -1){ while(!client.available() && client.connected()) yield(); res = client.read(); } - return (uint8_t)res; + return res; } @@ -449,11 +449,12 @@ bool ESP8266WebServerTemplate::_parseForm(ClientType& client, const snprintf(fastBoundary, fastBoundaryLen, "\r\n--%s", boundary.c_str()); int boundaryPtr = 0; while ( true ) { - if ( !client.connected() ) { - // Unexpected disconnection, abort! + int ret = _uploadReadByte(client); + if (ret < 0) { + // Unexpected, we should have had data available per above return _parseFormUploadAborted(); } - char in = _uploadReadByte(client); + char in = (char) ret; if (in == fastBoundary[ boundaryPtr ]) { // The input matched the current expected character, advance and possibly exit this file boundaryPtr++;