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

…9991)

* Fix timeout in WebServer::_uploadReadByte and set timeout handleClient()

Fixes: #9990

* Set HTTP_MAX_CLOSE_WAIT equal to other HTTP_xxx_WAIT values

* ci(pre-commit): Apply automatic fixes

---------

Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com>

libraries/WebServer/src/Parsing.cpp

@@ -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;

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();
 

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;