Skip to content

Fix timeout in WebServer::_uploadReadByte and handleClient() (#9990) #9991

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.

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

Merged
merged 3 commits into from
Jul 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
38 changes: 34 additions & 4 deletions libraries/WebServer/src/Parsing.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -347,11 +347,41 @@ int WebServer::_uploadReadByte(NetworkClient &client) {
int res = client.read();

if (res < 0) {
while (!client.available() && client.connected()) {
delay(2);
}
// keep trying until you either read a valid byte or timeout
const unsigned long startMillis = millis();
const 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();
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
}
}
}

return res;
Expand Down
4 changes: 1 addition & 3 deletions libraries/WebServer/src/WebServer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -432,10 +432,8 @@ void WebServer::handleClient() {
case HC_WAIT_READ:
// Wait for data from client to become available
if (_currentClient.available()) {
_currentClient.setTimeout(HTTP_MAX_SEND_WAIT); /* / 1000 removed, WifiClient setTimeout changed to ms */
if (_parseRequest(_currentClient)) {
// because HTTP_MAX_SEND_WAIT is expressed in milliseconds,
// it must be divided by 1000
_currentClient.setTimeout(HTTP_MAX_SEND_WAIT); /* / 1000 removed, WifiClient setTimeout changed to ms */
_contentLength = CONTENT_LENGTH_NOT_SET;
_handleRequest();

Expand Down
4 changes: 2 additions & 2 deletions libraries/WebServer/src/WebServer.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ enum HTTPAuthMethod {
#define HTTP_MAX_DATA_WAIT 5000 //ms to wait for the client to send the request
#define HTTP_MAX_POST_WAIT 5000 //ms to wait for POST data to arrive
#define HTTP_MAX_SEND_WAIT 5000 //ms to wait for data chunk to be ACKed
#define HTTP_MAX_CLOSE_WAIT 2000 //ms to wait for the client to close the connection
#define HTTP_MAX_CLOSE_WAIT 5000 //ms to wait for the client to close the connection
#define HTTP_MAX_BASIC_AUTH_LEN 256 // maximum length of a basic Auth base64 encoded username:password string

#define CONTENT_LENGTH_UNKNOWN ((size_t) - 1)
Expand All @@ -88,7 +88,7 @@ typedef struct {
HTTPRawStatus status;
size_t totalSize; // content size
size_t currentSize; // size of data currently in buf
uint8_t buf[HTTP_UPLOAD_BUFLEN];
uint8_t buf[HTTP_RAW_BUFLEN];
void *data; // additional data
} HTTPRaw;

Expand Down
Loading