Fix POST form parser edge cases (#2182)

From https://github.com/espressif/arduino-esp32/pull/9167
This commit is contained in:
Earle F. Philhower, III 2024-05-30 12:19:44 -07:00 committed by GitHub
parent a3dba8be5d
commit 2043623ce6
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -358,49 +358,20 @@ void HTTPServer::_uploadWriteByte(uint8_t b) {
int HTTPServer::_uploadReadByte(WiFiClient * client) { int HTTPServer::_uploadReadByte(WiFiClient * client) {
int res = client->read(); int res = client->read();
if (res < 0) { if (res < 0) {
// keep trying until you either read a valid byte or timeout while (!client->available() && client->connected()) {
unsigned long startMillis = millis(); delay(2);
unsigned long timeoutIntervalMillis = client->getTimeout();
bool 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
}
} }
res = client->read();
} }
return res; return res;
} }
bool HTTPServer::_parseForm(WiFiClient * client, String boundary, uint32_t len) { bool HTTPServer::_parseForm(WiFiClient * client, String boundary, uint32_t len) {
(void) len; (void)len;
log_v("Parse Form: Boundary: %s Length: %d", boundary.c_str(), len); log_v("Parse Form: Boundary: %s Length: %d", boundary.c_str(), len);
String line; String line;
int retry = 0; int retry = 0;
@ -448,10 +419,12 @@ bool HTTPServer::_parseForm(WiFiClient * client, String boundary, uint32_t len)
argType = FPSTR(mimeTable[txt].mimeType); argType = FPSTR(mimeTable[txt].mimeType);
line = client->readStringUntil('\r'); line = client->readStringUntil('\r');
client->readStringUntil('\n'); client->readStringUntil('\n');
if (line.length() > 12 && line.substring(0, 12).equalsIgnoreCase(FPSTR(Content_Type))) { while (line.length() > 0) {
argType = line.substring(line.indexOf(':') + 2); if (line.length() > 12 && line.substring(0, 12).equalsIgnoreCase(FPSTR(Content_Type))) {
//skip next line argType = line.substring(line.indexOf(':') + 2);
client->readStringUntil('\r'); }
//skip over any other headers
line = client->readStringUntil('\r');
client->readStringUntil('\n'); client->readStringUntil('\n');
} }
log_v("PostArg Type: %s", argType.c_str()); log_v("PostArg Type: %s", argType.c_str());
@ -469,7 +442,7 @@ bool HTTPServer::_parseForm(WiFiClient * client, String boundary, uint32_t len)
} }
log_v("PostArg Value: %s", argValue.c_str()); log_v("PostArg Value: %s", argValue.c_str());
RequestArgument& arg = _postArgs[_postArgsLen++]; RequestArgument &arg = _postArgs[_postArgsLen++];
arg.key = argName; arg.key = argName;
arg.value = argValue; arg.value = argValue;
@ -493,98 +466,60 @@ bool HTTPServer::_parseForm(WiFiClient * client, String boundary, uint32_t len)
_currentHandler->upload(*this, _currentUri, *_currentUpload); _currentHandler->upload(*this, _currentUri, *_currentUpload);
} }
_currentUpload->status = UPLOAD_FILE_WRITE; _currentUpload->status = UPLOAD_FILE_WRITE;
int argByte = _uploadReadByte(client);
readfile:
while (argByte != 0x0D) { int fastBoundaryLen = 4 /* \r\n-- */ + boundary.length() + 1 /* \0 */;
if (argByte < 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(); return _parseFormUploadAborted();
} }
_uploadWriteByte(argByte); char in = (char)ret;
argByte = _uploadReadByte(client); if (in == fastBoundary[boundaryPtr]) {
} // The input matched the current expected character, advance and possibly exit this file
boundaryPtr++;
argByte = _uploadReadByte(client); if (boundaryPtr == fastBoundaryLen - 1) {
if (argByte < 0) { // We read the whole boundary line, we're done here!
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)('-'));
for (uint32_t j = 0; j < i; j++) {
_uploadWriteByte(endBuf[j]);
}
goto readfile;
}
endBuf[i++] = (uint8_t)argByte;
}
if (strstr((const char*)endBuf, boundary.c_str()) != nullptr) {
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; break;
} }
continue;
} else { } else {
_uploadWriteByte(0x0D); // The char doesn't match what we want, so dump whatever matches we had, the read in char, and reset ptr to start
_uploadWriteByte(0x0A); for (int i = 0; i < boundaryPtr; i++) {
_uploadWriteByte((uint8_t)('-')); _uploadWriteByte(fastBoundary[i]);
_uploadWriteByte((uint8_t)('-')); }
for (uint32_t j = 0; j < boundary.length(); j++) { if (in == fastBoundary[0]) {
_uploadWriteByte(endBuf[j]); // 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;
} }
} }
} }
@ -593,7 +528,7 @@ readfile:
int iarg; int iarg;
int totalArgs = ((WEBSERVER_MAX_POST_ARGS - _postArgsLen) < _currentArgCount) ? (WEBSERVER_MAX_POST_ARGS - _postArgsLen) : _currentArgCount; int totalArgs = ((WEBSERVER_MAX_POST_ARGS - _postArgsLen) < _currentArgCount) ? (WEBSERVER_MAX_POST_ARGS - _postArgsLen) : _currentArgCount;
for (iarg = 0; iarg < totalArgs; iarg++) { for (iarg = 0; iarg < totalArgs; iarg++) {
RequestArgument& arg = _postArgs[_postArgsLen++]; RequestArgument &arg = _postArgs[_postArgsLen++];
arg.key = _currentArgs[iarg].key; arg.key = _currentArgs[iarg].key;
arg.value = _currentArgs[iarg].value; arg.value = _currentArgs[iarg].value;
} }
@ -602,7 +537,7 @@ readfile:
} }
_currentArgs = new RequestArgument[_postArgsLen]; _currentArgs = new RequestArgument[_postArgsLen];
for (iarg = 0; iarg < _postArgsLen; iarg++) { for (iarg = 0; iarg < _postArgsLen; iarg++) {
RequestArgument& arg = _currentArgs[iarg]; RequestArgument &arg = _currentArgs[iarg];
arg.key = _postArgs[iarg].key; arg.key = _postArgs[iarg].key;
arg.value = _postArgs[iarg].value; arg.value = _postArgs[iarg].value;
} }