Skip to content

Commit

Permalink
fix(webserver): Proposal for simplifying webserver file uploads via f…
Browse files Browse the repository at this point in the history 
…orm POST. (#9167)

* 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.

* Replace _uploadReadByte() in WebServer/Parsing.cpp with ESP8266 implementation.
  • Loading branch information
@everslick
everslick authored Feb 1, 2024
1 parent 5d73dd5 commit 7c5196d
Showing 1 changed file with 55 additions and 112 deletions.
167 changes: 55 additions & 112 deletions libraries/WebServer/src/Parsing.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -309,42 +309,14 @@ void WebServer::_uploadWriteByte(uint8_t b){
_currentUpload->buf[_currentUpload->currentSize++] = b;
}

int WebServer::_uploadReadByte(WiFiClient& client){
int WebServer::_uploadReadByte(WiFiClient& client) {
int res = client.read();
if(res < 0) {
// keep trying until you either read a valid byte or timeout
unsigned long startMillis = millis();
long timeoutIntervalMillis = client.getTimeout();
boolean timedOut = false;
for(;;) {
if (!client.connected()) return -1;
// loosely modeled after blinkWithoutDelay pattern
while(!timedOut && !client.available() && client.connected()){
delay(2);
timedOut = millis() - startMillis >= timeoutIntervalMillis;
}

res = client.read();
if(res >= 0) {
return res; // exit on a valid read
}
// NOTE: it is possible to get here and have all of the following
// assertions hold true
//
// -- client.available() > 0
// -- client.connected == true
// -- res == -1
//
// a simple retry strategy overcomes this which is to say the
// assertion is not permanent, but the reason that this works
// is elusive, and possibly indicative of a more subtle underlying
// issue

timedOut = millis() - startMillis >= timeoutIntervalMillis;
if(timedOut) {
return res; // exit on a timeout
}
}
if (res < 0) {
while(!client.available() && client.connected())
delay(2);

res = client.read();
}

return res;
Expand Down Expand Up @@ -436,88 +408,59 @@ bool WebServer::_parseForm(WiFiClient& client, String boundary, uint32_t len){
if(_currentHandler && _currentHandler->canUpload(_currentUri))
_currentHandler->upload(*this, _currentUri, *_currentUpload);
_currentUpload->status = UPLOAD_FILE_WRITE;
int argByte = _uploadReadByte(client);
readfile:

while(argByte != 0x0D){
if(argByte < 0) return _parseFormUploadAborted();
_uploadWriteByte(argByte);
argByte = _uploadReadByte(client);
}

argByte = _uploadReadByte(client);
if(argByte < 0) return _parseFormUploadAborted();
if (argByte == 0x0A){
argByte = _uploadReadByte(client);
if(argByte < 0) return _parseFormUploadAborted();
if ((char)argByte != '-'){
//continue reading the file
_uploadWriteByte(0x0D);
_uploadWriteByte(0x0A);
goto readfile;
} else {
argByte = _uploadReadByte(client);
if(argByte < 0) return _parseFormUploadAborted();
if ((char)argByte != '-'){
//continue reading the file
_uploadWriteByte(0x0D);
_uploadWriteByte(0x0A);
_uploadWriteByte((uint8_t)('-'));
goto readfile;
}
}

uint8_t endBuf[boundary.length()];
uint32_t i = 0;
while(i < boundary.length()){
argByte = _uploadReadByte(client);
if(argByte < 0) return _parseFormUploadAborted();
if ((char)argByte == 0x0D){
_uploadWriteByte(0x0D);
_uploadWriteByte(0x0A);
_uploadWriteByte((uint8_t)('-'));
_uploadWriteByte((uint8_t)('-'));
uint32_t j = 0;
while(j < i){
_uploadWriteByte(endBuf[j++]);
}
goto readfile;
}
endBuf[i++] = (uint8_t)argByte;
}

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);
log_v("End File: %s Type: %s Size: %d", _currentUpload->filename.c_str(), _currentUpload->type.c_str(), _currentUpload->totalSize);
line = client.readStringUntil(0x0D);
client.readStringUntil(0x0A);
if (line == "--"){
log_v("Done Parsing POST");
break;
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();
}
continue;
} else {
_uploadWriteByte(0x0D);
_uploadWriteByte(0x0A);
_uploadWriteByte((uint8_t)('-'));
_uploadWriteByte((uint8_t)('-'));
uint32_t i = 0;
while(i < boundary.length()){
_uploadWriteByte(endBuf[i++]);
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;
}
} 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 ] );
}
if (in == fastBoundary[ 0 ]) {
// This could be the start of the real end, mark it so and don't emit/skip it
boundaryPtr = 1;
} else {
// Not the 1st char of our pattern, so emit and ignore
_uploadWriteByte( in );
boundaryPtr = 0;
}
}
argByte = _uploadReadByte(client);
goto readfile;
}
} else {
_uploadWriteByte(0x0D);
goto readfile;
}
break;
// 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);
log_v("End File: %s Type: %s Size: %d",
_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
log_v("Done Parsing POST");
break;
}
continue;
}
}
}
Expand Down

0 comments on commit 7c5196d

Please sign in to comment.