From 0beb2fb51213aa8bf8ed9101b9f180e2b335f39f Mon Sep 17 00:00:00 2001 From: "Earle F. Philhower, III" Date: Mon, 22 Jul 2024 11:54:55 -0700 Subject: [PATCH] Fix timeout in WebServer::_uploadReadByte and handleClient() Upstream patch https://github.com/espressif/arduino-esp32/pull/9991 --- libraries/WebServer/src/HTTPServer.h | 4 +-- libraries/WebServer/src/Parsing.cpp | 39 ++++++++++++++++++--- libraries/WebServer/src/WebServerTemplate.h | 3 +- 3 files changed, 37 insertions(+), 9 deletions(-) diff --git a/libraries/WebServer/src/HTTPServer.h b/libraries/WebServer/src/HTTPServer.h index 82f9b7709..185bbcd28 100644 --- a/libraries/WebServer/src/HTTPServer.h +++ b/libraries/WebServer/src/HTTPServer.h @@ -49,7 +49,7 @@ enum HTTPAuthMethod { BASIC_AUTH, DIGEST_AUTH }; #define HTTP_MAX_DATA_AVAILABLE_WAIT 30 //ms to wait for the client to send the request when there is another client with data available #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 CONTENT_LENGTH_UNKNOWN ((size_t) -1) #define CONTENT_LENGTH_NOT_SET ((size_t) -2) @@ -73,7 +73,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; diff --git a/libraries/WebServer/src/Parsing.cpp b/libraries/WebServer/src/Parsing.cpp index f404f221c..3816acdb3 100644 --- a/libraries/WebServer/src/Parsing.cpp +++ b/libraries/WebServer/src/Parsing.cpp @@ -360,13 +360,42 @@ int HTTPServer::_uploadReadByte(WiFiClient * 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 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(); + 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; } diff --git a/libraries/WebServer/src/WebServerTemplate.h b/libraries/WebServer/src/WebServerTemplate.h index d31b3f949..38f93e30f 100644 --- a/libraries/WebServer/src/WebServerTemplate.h +++ b/libraries/WebServer/src/WebServerTemplate.h @@ -111,10 +111,9 @@ void WebServerTemplate::handleClient() { case HC_WAIT_READ: // Wait for data from client to become available if (_currentClient->available()) { + _currentClient->setTimeout(HTTP_MAX_SEND_WAIT); switch (_parseRequest(_currentClient)) { case CLIENT_REQUEST_CAN_CONTINUE: - // Because HTTP_MAX_SEND_WAIT is expressed in milliseconds, it must be divided by 1000 - _currentClient->setTimeout(HTTP_MAX_SEND_WAIT / 1000); _contentLength = CONTENT_LENGTH_NOT_SET; _handleRequest(); /* fallthrough */