From 45fc796cced522492e95e58e7a944668fc5954af Mon Sep 17 00:00:00 2001 From: TD-er Date: Fri, 5 Jul 2024 21:55:22 +0200 Subject: [PATCH 1/3] Fix timeout in WebServer::_uploadReadByte and set timeout handleClient() Fixes: #9990 --- libraries/WebServer/src/Parsing.cpp | 38 +++++++++++++++++++++++---- libraries/WebServer/src/WebServer.cpp | 4 +-- 2 files changed, 34 insertions(+), 8 deletions(-) diff --git a/libraries/WebServer/src/Parsing.cpp b/libraries/WebServer/src/Parsing.cpp index 200244e6848..453cae1e6e0 100644 --- a/libraries/WebServer/src/Parsing.cpp +++ b/libraries/WebServer/src/Parsing.cpp @@ -345,13 +345,41 @@ void WebServer::_uploadWriteByte(uint8_t b) { 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; diff --git a/libraries/WebServer/src/WebServer.cpp b/libraries/WebServer/src/WebServer.cpp index 92623b79c01..3996d3bdb0e 100644 --- a/libraries/WebServer/src/WebServer.cpp +++ b/libraries/WebServer/src/WebServer.cpp @@ -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(); From 6e8d674455b0b05d870025564a159c3379dd256b Mon Sep 17 00:00:00 2001 From: TD-er Date: Sun, 7 Jul 2024 13:57:43 +0200 Subject: [PATCH 2/3] Set HTTP_MAX_CLOSE_WAIT equal to other HTTP_xxx_WAIT values --- libraries/WebServer/src/WebServer.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/WebServer/src/WebServer.h b/libraries/WebServer/src/WebServer.h index c43dd4542ea..a107e223b34 100644 --- a/libraries/WebServer/src/WebServer.h +++ b/libraries/WebServer/src/WebServer.h @@ -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) @@ -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; From b523d24db5936bacf2b63beaafe4c771d6a60e00 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci-lite[bot]" <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com> Date: Wed, 10 Jul 2024 07:24:23 +0000 Subject: [PATCH 3/3] ci(pre-commit): Apply automatic fixes --- libraries/WebServer/src/Parsing.cpp | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/libraries/WebServer/src/Parsing.cpp b/libraries/WebServer/src/Parsing.cpp index 453cae1e6e0..53f0962fe7e 100644 --- a/libraries/WebServer/src/Parsing.cpp +++ b/libraries/WebServer/src/Parsing.cpp @@ -345,23 +345,25 @@ void WebServer::_uploadWriteByte(uint8_t b) { int WebServer::_uploadReadByte(NetworkClient &client) { int res = client.read(); - + if (res < 0) { // 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; + for (;;) { + if (!client.connected()) { + return -1; + } // loosely modeled after blinkWithoutDelay pattern - while(!timedOut && !client.available() && client.connected()){ + 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 + 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 @@ -377,7 +379,7 @@ int WebServer::_uploadReadByte(NetworkClient &client) { timedOut = (millis() - startMillis) >= timeoutIntervalMillis; if (timedOut) { - return res; // exit on a timeout + return res; // exit on a timeout } } }