Skip to content

Commit 92175d7

Browse files
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
1 parent 04b0c27 commit 92175d7

File tree

2 files changed

+27
-38
lines changed

2 files changed

+27
-38
lines changed

Diff for: libraries/ESP8266WebServer/src/ESP8266WebServer.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ class ESP8266WebServerTemplate
246246
bool _parseForm(ClientType& client, const String& boundary, uint32_t len);
247247
bool _parseFormUploadAborted();
248248
void _uploadWriteByte(uint8_t b);
249-
uint8_t _uploadReadByte(ClientType& client);
249+
int _uploadReadByte(ClientType& client);
250250
void _prepareHeader(String& response, int code, const char* content_type, size_t contentLength);
251251
bool _collectHeader(const char* headerName, const char* headerValue);
252252

Diff for: libraries/ESP8266WebServer/src/Parsing-impl.h

+26-37
Original file line numberDiff line numberDiff line change
@@ -347,14 +347,14 @@ void ESP8266WebServerTemplate<ServerType>::_uploadWriteByte(uint8_t b){
347347
}
348348

349349
template <typename ServerType>
350-
uint8_t ESP8266WebServerTemplate<ServerType>::_uploadReadByte(ClientType& client){
350+
int ESP8266WebServerTemplate<ServerType>::_uploadReadByte(ClientType& client){
351351
int res = client.read();
352352
if(res == -1){
353353
while(!client.available() && client.connected())
354354
yield();
355355
res = client.read();
356356
}
357-
return (uint8_t)res;
357+
return res;
358358
}
359359

360360

@@ -444,45 +444,34 @@ bool ESP8266WebServerTemplate<ServerType>::_parseForm(ClientType& client, const
444444
_currentHandler->upload(*this, _currentUri, *_currentUpload);
445445
_currentUpload->status = UPLOAD_FILE_WRITE;
446446

447-
int bLen = boundary.length();
448-
uint8_t boundBuf[2 + bLen + 1]; // "--" + boundary + null terminator
449-
boundBuf[2 + bLen] = '\0';
450-
uint8_t argByte;
451-
bool first = true;
452-
while (1) {
453-
//attempt to fill up boundary buffer with length of boundary string
454-
int i;
455-
for (i = 0; i < 2 + bLen; i++) {
456-
if (!client.connected()) return _parseFormUploadAborted();
457-
argByte = _uploadReadByte(client);
458-
if (argByte == '\r')
459-
break;
460-
boundBuf[i] = argByte;
461-
}
462-
if ((strncmp((const char*)boundBuf, "--", 2) == 0) && (strcmp((const char*)(boundBuf + 2), boundary.c_str()) == 0))
463-
break; //found the boundary, done parsing this file
464-
if (first) first = false; //only add newline characters after the first line
465-
else {
466-
_uploadWriteByte('\r');
467-
_uploadWriteByte('\n');
447+
int fastBoundaryLen = 4 /* \r\n-- */ + boundary.length() + 1 /* \0 */;
448+
char fastBoundary[ fastBoundaryLen ];
449+
snprintf(fastBoundary, fastBoundaryLen, "\r\n--%s", boundary.c_str());
450+
int boundaryPtr = 0;
451+
while ( true ) {
452+
int ret = _uploadReadByte(client);
453+
if (ret < 0) {
454+
// Unexpected, we should have had data available per above
455+
return _parseFormUploadAborted();
468456
}
469-
// current line does not contain boundary, upload all bytes in boundary buffer
470-
for (int j = 0; j < i; j++)
471-
_uploadWriteByte(boundBuf[j]);
472-
// the initial pass (filling up the boundary buffer) did not reach the end of the line. Upload the rest of the line now
473-
if (i >= 2 + bLen) {
474-
if (!client.connected()) return _parseFormUploadAborted();
475-
argByte = _uploadReadByte(client);
476-
while (argByte != '\r') {
477-
if (!client.connected()) return _parseFormUploadAborted();
478-
_uploadWriteByte(argByte);
479-
argByte = _uploadReadByte(client);
457+
char in = (char) ret;
458+
if (in == fastBoundary[ boundaryPtr ]) {
459+
// The input matched the current expected character, advance and possibly exit this file
460+
boundaryPtr++;
461+
if (boundaryPtr == fastBoundaryLen - 1) {
462+
// We read the whole boundary line, we're done here!
463+
break;
464+
}
465+
} else {
466+
// The char doesn't match what we want, so dump whatever matches we had, the read in char, and reset ptr to start
467+
for (int i = 0; i < boundaryPtr; i++) {
468+
_uploadWriteByte( fastBoundary[ i ] );
480469
}
470+
_uploadWriteByte( in );
471+
boundaryPtr = 0;
481472
}
482-
if (!client.connected()) return _parseFormUploadAborted();
483-
_uploadReadByte(client); // '\n'
484473
}
485-
//Found the boundary string, finish processing this file upload
474+
// Found the boundary string, finish processing this file upload
486475
if (_currentHandler && _currentHandler->canUpload(_currentUri))
487476
_currentHandler->upload(*this, _currentUri, *_currentUpload);
488477
_currentUpload->totalSize += _currentUpload->currentSize;

0 commit comments

Comments
 (0)