Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix POST form parser edge cases #2182

Merged
merged 3 commits into from
May 30, 2024
Merged

Fix POST form parser edge cases #2182

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
81b9da5
Fix POST form parser edge cases
earlephilhower May 30, 2024
77d0ed6
Style
earlephilhower May 30, 2024
cabe966
Merge branch 'master' into postie
earlephilhower May 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
185 changes: 60 additions & 125 deletions libraries/WebServer/src/Parsing.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,49 +358,20 @@ void HTTPServer::_uploadWriteByte(uint8_t b) {

int HTTPServer::_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();
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
}
if (res < 0) {
while (!client->available() && client->connected()) {
delay(2);
}

res = client->read();
}

return res;
}

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);
String line;
int retry = 0;
Expand Down Expand Up @@ -448,10 +419,12 @@ bool HTTPServer::_parseForm(WiFiClient * client, String boundary, uint32_t len)
argType = FPSTR(mimeTable[txt].mimeType);
line = client->readStringUntil('\r');
client->readStringUntil('\n');
if (line.length() > 12 && line.substring(0, 12).equalsIgnoreCase(FPSTR(Content_Type))) {
argType = line.substring(line.indexOf(':') + 2);
//skip next line
client->readStringUntil('\r');
while (line.length() > 0) {
if (line.length() > 12 && line.substring(0, 12).equalsIgnoreCase(FPSTR(Content_Type))) {
argType = line.substring(line.indexOf(':') + 2);
}
//skip over any other headers
line = client->readStringUntil('\r');
client->readStringUntil('\n');
}
log_v("PostArg Type: %s", argType.c_str());
Expand All @@ -469,7 +442,7 @@ bool HTTPServer::_parseForm(WiFiClient * client, String boundary, uint32_t len)
}
log_v("PostArg Value: %s", argValue.c_str());

RequestArgument& arg = _postArgs[_postArgsLen++];
RequestArgument &arg = _postArgs[_postArgsLen++];
arg.key = argName;
arg.value = argValue;

Expand All @@ -493,98 +466,60 @@ bool HTTPServer::_parseForm(WiFiClient * client, String boundary, uint32_t len)
_currentHandler->upload(*this, _currentUri, *_currentUpload);
}
_currentUpload->status = UPLOAD_FILE_WRITE;
int argByte = _uploadReadByte(client);
readfile:

while (argByte != 0x0D) {
if (argByte < 0) {
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();
}
_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)('-'));
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");
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;
}
continue;
} else {
_uploadWriteByte(0x0D);
_uploadWriteByte(0x0A);
_uploadWriteByte((uint8_t)('-'));
_uploadWriteByte((uint8_t)('-'));
for (uint32_t j = 0; j < boundary.length(); j++) {
_uploadWriteByte(endBuf[j]);
// 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 All @@ -593,7 +528,7 @@ bool HTTPServer::_parseForm(WiFiClient * client, String boundary, uint32_t len)
int iarg;
int totalArgs = ((WEBSERVER_MAX_POST_ARGS - _postArgsLen) < _currentArgCount) ? (WEBSERVER_MAX_POST_ARGS - _postArgsLen) : _currentArgCount;
for (iarg = 0; iarg < totalArgs; iarg++) {
RequestArgument& arg = _postArgs[_postArgsLen++];
RequestArgument &arg = _postArgs[_postArgsLen++];
arg.key = _currentArgs[iarg].key;
arg.value = _currentArgs[iarg].value;
}
Expand All @@ -602,7 +537,7 @@ bool HTTPServer::_parseForm(WiFiClient * client, String boundary, uint32_t len)
}
_currentArgs = new RequestArgument[_postArgsLen];
for (iarg = 0; iarg < _postArgsLen; iarg++) {
RequestArgument& arg = _currentArgs[iarg];
RequestArgument &arg = _currentArgs[iarg];
arg.key = _postArgs[iarg].key;
arg.value = _postArgs[iarg].value;
}
Expand Down