Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit bd941e3

Browse files
authoredJan 23, 2024
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.
1 parent cceebb5 commit bd941e3

File tree

1 file changed

+49
-78
lines changed

1 file changed

+49
-78
lines changed
 

‎libraries/WebServer/src/Parsing.cpp

Lines changed: 49 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -436,88 +436,59 @@ bool WebServer::_parseForm(WiFiClient& client, String boundary, uint32_t len){
436436
if(_currentHandler && _currentHandler->canUpload(_currentUri))
437437
_currentHandler->upload(*this, _currentUri, *_currentUpload);
438438
_currentUpload->status = UPLOAD_FILE_WRITE;
439-
int argByte = _uploadReadByte(client);
440-
readfile:
441439

442-
while(argByte != 0x0D){
443-
if(argByte < 0) return _parseFormUploadAborted();
444-
_uploadWriteByte(argByte);
445-
argByte = _uploadReadByte(client);
446-
}
447-
448-
argByte = _uploadReadByte(client);
449-
if(argByte < 0) return _parseFormUploadAborted();
450-
if (argByte == 0x0A){
451-
argByte = _uploadReadByte(client);
452-
if(argByte < 0) return _parseFormUploadAborted();
453-
if ((char)argByte != '-'){
454-
//continue reading the file
455-
_uploadWriteByte(0x0D);
456-
_uploadWriteByte(0x0A);
457-
goto readfile;
458-
} else {
459-
argByte = _uploadReadByte(client);
460-
if(argByte < 0) return _parseFormUploadAborted();
461-
if ((char)argByte != '-'){
462-
//continue reading the file
463-
_uploadWriteByte(0x0D);
464-
_uploadWriteByte(0x0A);
465-
_uploadWriteByte((uint8_t)('-'));
466-
goto readfile;
467-
}
468-
}
469-
470-
uint8_t endBuf[boundary.length()];
471-
uint32_t i = 0;
472-
while(i < boundary.length()){
473-
argByte = _uploadReadByte(client);
474-
if(argByte < 0) return _parseFormUploadAborted();
475-
if ((char)argByte == 0x0D){
476-
_uploadWriteByte(0x0D);
477-
_uploadWriteByte(0x0A);
478-
_uploadWriteByte((uint8_t)('-'));
479-
_uploadWriteByte((uint8_t)('-'));
480-
uint32_t j = 0;
481-
while(j < i){
482-
_uploadWriteByte(endBuf[j++]);
483-
}
484-
goto readfile;
440+
int fastBoundaryLen = 4 /* \r\n-- */ + boundary.length() + 1 /* \0 */;
441+
char fastBoundary[ fastBoundaryLen ];
442+
snprintf(fastBoundary, fastBoundaryLen, "\r\n--%s", boundary.c_str());
443+
int boundaryPtr = 0;
444+
while ( true ) {
445+
int ret = _uploadReadByte(client);
446+
if (ret < 0) {
447+
// Unexpected, we should have had data available per above
448+
return _parseFormUploadAborted();
485449
}
486-
endBuf[i++] = (uint8_t)argByte;
487-
}
488-
489-
if (strstr((const char*)endBuf, boundary.c_str()) != NULL){
490-
if(_currentHandler && _currentHandler->canUpload(_currentUri))
491-
_currentHandler->upload(*this, _currentUri, *_currentUpload);
492-
_currentUpload->totalSize += _currentUpload->currentSize;
493-
_currentUpload->status = UPLOAD_FILE_END;
494-
if(_currentHandler && _currentHandler->canUpload(_currentUri))
495-
_currentHandler->upload(*this, _currentUri, *_currentUpload);
496-
log_v("End File: %s Type: %s Size: %d", _currentUpload->filename.c_str(), _currentUpload->type.c_str(), _currentUpload->totalSize);
497-
line = client.readStringUntil(0x0D);
498-
client.readStringUntil(0x0A);
499-
if (line == "--"){
500-
log_v("Done Parsing POST");
501-
break;
450+
char in = (char) ret;
451+
if (in == fastBoundary[ boundaryPtr ]) {
452+
// The input matched the current expected character, advance and possibly exit this file
453+
boundaryPtr++;
454+
if (boundaryPtr == fastBoundaryLen - 1) {
455+
// We read the whole boundary line, we're done here!
456+
break;
457+
}
458+
} else {
459+
// The char doesn't match what we want, so dump whatever matches we had, the read in char, and reset ptr to start
460+
for (int i = 0; i < boundaryPtr; i++) {
461+
_uploadWriteByte( fastBoundary[ i ] );
462+
}
463+
if (in == fastBoundary[ 0 ]) {
464+
// This could be the start of the real end, mark it so and don't emit/skip it
465+
boundaryPtr = 1;
466+
} else {
467+
// Not the 1st char of our pattern, so emit and ignore
468+
_uploadWriteByte( in );
469+
boundaryPtr = 0;
470+
}
502471
}
503-
continue;
504-
} else {
505-
_uploadWriteByte(0x0D);
506-
_uploadWriteByte(0x0A);
507-
_uploadWriteByte((uint8_t)('-'));
508-
_uploadWriteByte((uint8_t)('-'));
509-
uint32_t i = 0;
510-
while(i < boundary.length()){
511-
_uploadWriteByte(endBuf[i++]);
512-
}
513-
argByte = _uploadReadByte(client);
514-
goto readfile;
515-
}
516-
} else {
517-
_uploadWriteByte(0x0D);
518-
goto readfile;
519472
}
520-
break;
473+
// Found the boundary string, finish processing this file upload
474+
if (_currentHandler && _currentHandler->canUpload(_currentUri))
475+
_currentHandler->upload(*this, _currentUri, *_currentUpload);
476+
_currentUpload->totalSize += _currentUpload->currentSize;
477+
_currentUpload->status = UPLOAD_FILE_END;
478+
if (_currentHandler && _currentHandler->canUpload(_currentUri))
479+
_currentHandler->upload(*this, _currentUri, *_currentUpload);
480+
log_v("End File: %s Type: %s Size: %d",
481+
_currentUpload->filename.c_str(),
482+
_currentUpload->type.c_str(),
483+
(int)_currentUpload->totalSize);
484+
if (!client.connected()) return _parseFormUploadAborted();
485+
line = client.readStringUntil('\r');
486+
client.readStringUntil('\n');
487+
if (line == "--") { // extra two dashes mean we reached the end of all form fields
488+
log_v("Done Parsing POST");
489+
break;
490+
}
491+
continue;
521492
}
522493
}
523494
}

0 commit comments

Comments
 (0)
Please sign in to comment.