From 157ce57996de890cee98b0dcf7eae3c82d10ef46 Mon Sep 17 00:00:00 2001 From: Jeroen88 Date: Fri, 15 May 2020 23:12:49 +0200 Subject: [PATCH] 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 --- .../ReuseConnectionV2/ReuseConnectionV2.ino | 82 +++++++++++++++++++ .../src/ESP8266HTTPClient.cpp | 41 +++++----- 2 files changed, 104 insertions(+), 19 deletions(-) create mode 100644 libraries/ESP8266HTTPClient/examples/ReuseConnectionV2/ReuseConnectionV2.ino diff --git a/libraries/ESP8266HTTPClient/examples/ReuseConnectionV2/ReuseConnectionV2.ino b/libraries/ESP8266HTTPClient/examples/ReuseConnectionV2/ReuseConnectionV2.ino new file mode 100644 index 0000000000..42a89beced --- /dev/null +++ b/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 +#include +#include + +#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); + } + } +} diff --git a/libraries/ESP8266HTTPClient/src/ESP8266HTTPClient.cpp b/libraries/ESP8266HTTPClient/src/ESP8266HTTPClient.cpp index 5212dcf3ee..85d27d5c6c 100644 --- a/libraries/ESP8266HTTPClient/src/ESP8266HTTPClient.cpp +++ b/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,6 +1338,7 @@ int HTTPClient::handleHeaderResponse() while(connected()) { size_t len = _client->available(); if(len > 0) { + int headerSeparator = -1; String headerLine = _client->readStringUntil('\n'); lastDataTime = millis(); @@ -1341,15 +1346,13 @@ int HTTPClient::handleHeaderResponse() 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();