From 92175d7090843745b024131fa959de0dbe8bd933 Mon Sep 17 00:00:00 2001
From: "Earle F. Philhower, III" <earlephilhower@yahoo.com>
Date: Sat, 28 Nov 2020 17:22:34 -0800
Subject: [PATCH] Rewrite multipart boundary detection (#7728)
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--<boundary>`
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.
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.
Fixes #7723
---
.../ESP8266WebServer/src/ESP8266WebServer.h | 2 +-
libraries/ESP8266WebServer/src/Parsing-impl.h | 63 ++++++++-----------
2 files changed, 27 insertions(+), 38 deletions(-)
diff --git a/libraries/ESP8266WebServer/src/ESP8266WebServer.h b/libraries/ESP8266WebServer/src/ESP8266WebServer.h
index 23b5b328a..d40313a59 100644
--- a/libraries/ESP8266WebServer/src/ESP8266WebServer.h
+++ b/libraries/ESP8266WebServer/src/ESP8266WebServer.h
@@ -246,7 +246,7 @@ protected:
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 33d3efd66..cab0c04f7 100644
--- a/libraries/ESP8266WebServer/src/Parsing-impl.h
+++ b/libraries/ESP8266WebServer/src/Parsing-impl.h
@@ -347,14 +347,14 @@ void ESP8266WebServerTemplate<ServerType>::_uploadWriteByte(uint8_t b){
}
template <typename ServerType>
-uint8_t ESP8266WebServerTemplate<ServerType>::_uploadReadByte(ClientType& client){
+int ESP8266WebServerTemplate<ServerType>::_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;
}
@@ -444,45 +444,34 @@ bool ESP8266WebServerTemplate<ServerType>::_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')
+ 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();
+ }
+ 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;
- 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');
- }
- // 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);
}
+ } 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;