From c289c75da1293e5bce3d654027dfcc11bbbdf7d7 Mon Sep 17 00:00:00 2001 From: Matthew Date: Sun, 23 Aug 2020 12:04:17 -0400 Subject: [PATCH 1/3] fixed bug in parsing POST file uploads --- libraries/ESP8266WebServer/src/Parsing-impl.h | 114 ++++++++---------- 1 file changed, 51 insertions(+), 63 deletions(-) diff --git a/libraries/ESP8266WebServer/src/Parsing-impl.h b/libraries/ESP8266WebServer/src/Parsing-impl.h index 4bb6db8878..9a32273aa3 100644 --- a/libraries/ESP8266WebServer/src/Parsing-impl.h +++ b/libraries/ESP8266WebServer/src/Parsing-impl.h @@ -355,6 +355,7 @@ uint8_t ESP8266WebServerTemplate::_uploadReadByte(ClientType& client return (uint8_t)res; } + template bool ESP8266WebServerTemplate::_parseForm(ClientType& client, const String& boundary, uint32_t len){ (void) len; @@ -440,74 +441,61 @@ bool ESP8266WebServerTemplate::_parseForm(ClientType& client, const if(_currentHandler && _currentHandler->canUpload(_currentUri)) _currentHandler->upload(*this, _currentUri, *_currentUpload); _currentUpload->status = UPLOAD_FILE_WRITE; - uint8_t argByte = _uploadReadByte(client); -readfile: - while(argByte != 0x0D){ - if (!client.connected()) return _parseFormUploadAborted(); - _uploadWriteByte(argByte); - argByte = _uploadReadByte(client); - } - argByte = _uploadReadByte(client); - if (!client.connected()) return _parseFormUploadAborted(); - if (argByte == 0x0A){ - argByte = _uploadReadByte(client); - if (!client.connected()) return _parseFormUploadAborted(); - if ((char)argByte != '-'){ - //continue reading the file - _uploadWriteByte(0x0D); - _uploadWriteByte(0x0A); - goto readfile; - } else { - argByte = _uploadReadByte(client); - if (!client.connected()) return _parseFormUploadAborted(); - if ((char)argByte != '-'){ - //continue reading the file - _uploadWriteByte(0x0D); - _uploadWriteByte(0x0A); - _uploadWriteByte((uint8_t)('-')); - goto readfile; + 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; } - } - - uint8_t endBuf[boundary.length()]; - client.readBytes(endBuf, boundary.length()); - - 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); - DBGWS("End File: %s Type: %s Size: %d\n", - _currentUpload->filename.c_str(), - _currentUpload->type.c_str(), - (int)_currentUpload->totalSize); - line = client.readStringUntil(0x0D); - client.readStringUntil(0x0A); - if (line == "--"){ - DBGWS("Done Parsing POST\n"); - break; + if (String((const char*)boundBuf).startsWith("--" + boundary)) //found the boundary, done parsing this file + break; + if (first) first = false; //only add newline characters after the first line + else { + _uploadWriteByte('\r'); + _uploadWriteByte('\n'); } - continue; - } else { - _uploadWriteByte(0x0D); - _uploadWriteByte(0x0A); - _uploadWriteByte((uint8_t)('-')); - _uploadWriteByte((uint8_t)('-')); - uint32_t i = 0; - while(i < boundary.length()){ - _uploadWriteByte(endBuf[i++]); + // 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) { + argByte = _uploadReadByte(client); + while (argByte != '\r') { + if (!client.connected()) return _parseFormUploadAborted(); + _uploadWriteByte(argByte); + argByte = _uploadReadByte(client); + } } - argByte = _uploadReadByte(client); - goto readfile; - } - } else { - _uploadWriteByte(0x0D); - goto readfile; + _uploadReadByte(client); // '\n' + } + //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); + DBGWS("End File: %s Type: %s Size: %d\n", + _currentUpload->filename.c_str(), + _currentUpload->type.c_str(), + (int)_currentUpload->totalSize); + line = client.readStringUntil('\r'); + client.readStringUntil('\n'); + if (line == "--") { // extra two dashes mean we reached the end of all form fields + DBGWS("Done Parsing POST\n"); + break; } - break; + continue; } } } From ea9a6fbbc654f709bf9bda2385843a85442c1170 Mon Sep 17 00:00:00 2001 From: Matthew Foran <46829130+mjforanVT@users.noreply.github.com> Date: Sun, 23 Aug 2020 12:49:11 -0400 Subject: [PATCH 2/3] Eliminated temporary String --- libraries/ESP8266WebServer/src/Parsing-impl.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/ESP8266WebServer/src/Parsing-impl.h b/libraries/ESP8266WebServer/src/Parsing-impl.h index 9a32273aa3..f7d5523319 100644 --- a/libraries/ESP8266WebServer/src/Parsing-impl.h +++ b/libraries/ESP8266WebServer/src/Parsing-impl.h @@ -457,8 +457,8 @@ bool ESP8266WebServerTemplate::_parseForm(ClientType& client, const break; boundBuf[i] = argByte; } - if (String((const char*)boundBuf).startsWith("--" + boundary)) //found the boundary, done parsing this file - break; + 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'); From fbf058c80607e12fb5ae0b2affeb9ba53ff0f4c5 Mon Sep 17 00:00:00 2001 From: Matthew Date: Sun, 30 Aug 2020 10:20:35 -0400 Subject: [PATCH 3/3] extra checks to make sure client is connected before reading --- libraries/ESP8266WebServer/src/Parsing-impl.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libraries/ESP8266WebServer/src/Parsing-impl.h b/libraries/ESP8266WebServer/src/Parsing-impl.h index f7d5523319..de10de6b1f 100644 --- a/libraries/ESP8266WebServer/src/Parsing-impl.h +++ b/libraries/ESP8266WebServer/src/Parsing-impl.h @@ -469,6 +469,7 @@ bool ESP8266WebServerTemplate::_parseForm(ClientType& client, const _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(); @@ -476,6 +477,7 @@ bool ESP8266WebServerTemplate::_parseForm(ClientType& client, const argByte = _uploadReadByte(client); } } + if (!client.connected()) return _parseFormUploadAborted(); _uploadReadByte(client); // '\n' } //Found the boundary string, finish processing this file upload @@ -489,6 +491,7 @@ bool ESP8266WebServerTemplate::_parseForm(ClientType& client, const _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