bugfix2/ESP8266HTTPClient (#6476)

* Because of git problems, start from a new fork and create a new PR. This was PR #6457

* Style update to pass Travis

* Update ReuseConnectionV2.ino

* fix + enforce testing http code

per @earlephilhower review

* Close connection before ::connecting on HTTP/1.0

HTTPClient never actually closes the TCP connection on its own. It will leave the TCP connection open unless you explicitly do a getString which makes a StreamString and stuffs it with the HTTP server response, at which point the HTTP server itself will close the connection.

If you check the HTTP error code and find failure, unless you do a getString and throw it away, it won't disconnect.  Even in HTTP/1.0 or in cases when you haven't enabled _reuse.

Change the logic in ::connect to only reuse the connection when it is specifically allowed.  Otherwise, fall back to re-connection.

* Adjust example per request

Do single URL get in each loop, avoid infinite for loop at end.

* Fix astyle

* Clean up final pass notice

* Fix example syntax error

Editing code in a web textbox without running it is a painful process.


Co-authored-by: Earle F. Philhower, III <[email protected]>

Author: Jeroen88

libraries/ESP8266HTTPClient/examples/ReuseConnectionV2/ReuseConnectionV2.ino

@@ -0,0 +1,82 @@
+/**
+   reuseConnectionV2.ino
+
+    Created on: 22.11.2015
+
+   This example reuses the http connection and also restores the connection if the connection is lost
+*/
+
+
+#include <ESP8266WiFi.h>
+#include <ESP8266WiFiMulti.h>
+#include <ESP8266HTTPClient.h>
+
+#ifndef STASSID
+#define STASSID "your-ssid"
+#define STAPSK  "your-password"
+#endif
+
+ESP8266WiFiMulti WiFiMulti;
+
+HTTPClient http;
+WiFiClient client;
+
+void setup() {
+
+  Serial.begin(115200);
+  // Serial.setDebugOutput(true);
+
+  Serial.println();
+  Serial.println();
+  Serial.println("Connecting to WiFi...");
+
+  WiFi.mode(WIFI_STA);
+  WiFiMulti.addAP(STASSID, STAPSK);
+
+  // wait for WiFi connection
+  while ((WiFiMulti.run() != WL_CONNECTED)) {
+    Serial.write('.');
+    delay(500);
+  }
+  Serial.println(" connected to WiFi");
+
+  // allow reuse (if server supports it)
+  http.setReuse(true);
+
+
+  http.begin(client, "http://jigsaw.w3.org/HTTP/connection.html");
+  //http.begin(client, "jigsaw.w3.org", 80, "/HTTP/connection.html");
+}
+
+int pass = 0;
+
+void loop() {
+  // First 10 loop()s, retrieve the URL
+  if (pass < 10) {
+    pass++;
+    Serial.printf("Reuse connection example, GET url for the %d time\n", pass);
+    int httpCode = http.GET();
+    if (httpCode > 0) {
+      Serial.printf("[HTTP] GET... code: %d\n", httpCode);
+
+      // file found at server
+      if (httpCode == HTTP_CODE_OK) {
+        http.writeToStream(&Serial);
+      }
+    } else {
+      Serial.printf("[HTTP] GET... failed, error: %s\n", http.errorToString(httpCode).c_str());
+      // Something went wrong with the connection, try to reconnect
+      http.end();
+      http.begin(client, "http://jigsaw.w3.org/HTTP/connection.html");
+      //http.begin(client, "jigsaw.w3.org", 80, "/HTTP/connection.html");
+    }
+
+    if (pass == 10) {
+      http.end();
+      Serial.println("Done testing");
+    } else {
+      Serial.println("\n\n\nWait 5 second...\n");
+      delay(5000);
+    }
+  }
+}

libraries/ESP8266HTTPClient/src/ESP8266HTTPClient.cpp

@@ -457,6 +457,10 @@ void HTTPClient::disconnect(bool preserveClient)
 #endif
         }
     } else {
+        if (!preserveClient && _client) { // Also destroy _client if not connected()
+            _client = nullptr;
+        }
+
         DEBUG_HTTPCLIENT("[HTTP-Client][end] tcp is closed\n");
     }
 }
@@ -970,7 +974,9 @@ int HTTPClient::writeToStream(Stream * stream)
         return returnError(HTTPC_ERROR_NO_STREAM);
     }
 
-    if(!connected()) {
+    // Only return error if not connected and no data available, because otherwise ::getString() will return an error instead of an empty
+    // string when the server returned a http code 204 (no content)
+    if(!connected() && _transferEncoding != HTTPC_TE_IDENTITY && _size > 0) {
         return returnError(HTTPC_ERROR_NOT_CONNECTED);
     }
 
@@ -979,11 +985,13 @@ int HTTPClient::writeToStream(Stream * stream)
     int ret = 0;
 
     if(_transferEncoding == HTTPC_TE_IDENTITY) {
-        ret = writeToStreamDataBlock(stream, len);
+        if(len > 0) {
+            ret = writeToStreamDataBlock(stream, len);
 
-        // have we an error?
-        if(ret < 0) {
-            return returnError(ret);
+            // have we an error?
+            if(ret < 0) {
+                return returnError(ret);
+            }
         }
     } else if(_transferEncoding == HTTPC_TE_CHUNKED) {
         int size = 0;
@@ -1198,12 +1206,8 @@ bool HTTPClient::hasHeader(const char* name)
  */
 bool HTTPClient::connect(void)
 {
-    if(connected()) {
-        if(_reuse) {
-            DEBUG_HTTPCLIENT("[HTTP-Client] connect: already connected, reusing connection\n");
-        } else {
-            DEBUG_HTTPCLIENT("[HTTP-Client] connect: already connected, try reuse!\n");
-        }
+    if(_reuse && _canReuse && connected()) {
+        DEBUG_HTTPCLIENT("[HTTP-Client] connect: already connected, reusing connection\n");
         while(_client->available() > 0) {
             _client->read();
         }
@@ -1334,22 +1338,21 @@ int HTTPClient::handleHeaderResponse()
     while(connected()) {
         size_t len = _client->available();
         if(len > 0) {
+            int headerSeparator = -1;
             String headerLine = _client->readStringUntil('\n');
 
             lastDataTime = millis();
 
             DEBUG_HTTPCLIENT("[HTTP-Client][handleHeaderResponse] RX: '%s'\n", headerLine.c_str());
 
             if (headerLine.startsWith(F("HTTP/1."))) {
-                if (_canReuse) {
-                    _canReuse = (headerLine[sizeof "HTTP/1." - 1] != '0');
-                }
-                _returnCode = headerLine.substring(9, headerLine.indexOf(' ', 9)).toInt();
-                continue;
-            }
 
-            int headerSeparator = headerLine.indexOf(':');
-            if (headerSeparator > 0) {
+                constexpr auto httpVersionIdx = sizeof "HTTP/1." - 1;
+                _canReuse = _canReuse && (headerLine[httpVersionIdx] != '0');
+                _returnCode = headerLine.substring(httpVersionIdx + 2, headerLine.indexOf(' ', httpVersionIdx + 2)).toInt();
+                _canReuse = _canReuse && (_returnCode > 0) && (_returnCode < 500);
+
+            } else if ((headerSeparator = headerLine.indexOf(':')) > 0) {
                 String headerName = headerLine.substring(0, headerSeparator);
                 String headerValue = headerLine.substring(headerSeparator + 1);
                 headerValue.trim();