From 63612e92767ba9a7b88f918808e7f178b22c9db6 Mon Sep 17 00:00:00 2001
From: Bernhard Kirchen <schlimmchen@posteo.net>
Date: Wed, 21 Aug 2024 21:16:16 +0200
Subject: [PATCH] Fix: restart TCP connection if HTTP server sends
 Connection:close

we previously performed a whole new GET request when doing digest
authentication. it seemed beneficial to reuse the TCP connection to
perform the second GET request, which includes the authentication
tokens. however, if the server sends "Connection: close" we must not
requse the TCP connection for another HTTP request.

this broke authentication against Shelly devices (at least those with
original firmware).

now we explicitly set "Connection: keep-alive" in our request, and reuse
the TCP connection only if te server replies with "Connection:
keep-alive" as well.
---
 include/HttpGetter.h | 12 +++++++++++-
 src/HttpGetter.cpp   | 21 ++++++++++++++++++---
 2 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/include/HttpGetter.h b/include/HttpGetter.h
index 11ece109..41b354f4 100644
--- a/include/HttpGetter.h
+++ b/include/HttpGetter.h
@@ -9,7 +9,17 @@
 #include <HTTPClient.h>
 #include <WiFiClient.h>
 
-using up_http_client_t = std::unique_ptr<HTTPClient>;
+class HttpGetterClient : public HTTPClient {
+public:
+    void restartTCP() {
+        // keeps the NetworkClient, and closes the TCP connections (as we
+        // effectively do not support keep-alive with HTTP 1.0).
+        HTTPClient::disconnect(true);
+        HTTPClient::connect();
+    }
+};
+
+using up_http_client_t = std::unique_ptr<HttpGetterClient>;
 using sp_wifi_client_t = std::shared_ptr<WiFiClient>;
 
 class HttpRequestResult {
diff --git a/src/HttpGetter.cpp b/src/HttpGetter.cpp
index 8b2e158e..1bd1d9b3 100644
--- a/src/HttpGetter.cpp
+++ b/src/HttpGetter.cpp
@@ -100,7 +100,7 @@ HttpRequestResult HttpGetter::performGetRequest()
         }
     }
 
-    auto upTmpHttpClient = std::make_unique<HTTPClient>();
+    auto upTmpHttpClient = std::make_unique<HttpGetterClient>();
 
     // use HTTP1.0 to avoid problems with chunked transfer encoding when the
     // stream is later used to read the server's response.
@@ -135,8 +135,13 @@ HttpRequestResult HttpGetter::performGetRequest()
             break;
         }
         case Auth_t::Digest: {
-            const char *headers[1] = {"WWW-Authenticate"};
-            upTmpHttpClient->collectHeaders(headers, 1);
+            // send "Connection: keep-alive" (despite using HTTP/1.0, where
+            // "Connection: close" is the default) so there is a chance to
+            // reuse the TCP connection when performing the second GET request.
+            upTmpHttpClient->setReuse(true);
+
+            const char *headers[2] = {"WWW-Authenticate", "Connection"};
+            upTmpHttpClient->collectHeaders(headers, 2);
             break;
         }
     }
@@ -152,6 +157,16 @@ HttpRequestResult HttpGetter::performGetRequest()
         String authReq = upTmpHttpClient->header("WWW-Authenticate");
         String authorization = getAuthDigest(authReq, 1);
         upTmpHttpClient->addHeader("Authorization", authorization);
+
+        // use a new TCP connection if the server sent "Connection: close".
+        bool restart = true;
+        if (upTmpHttpClient->hasHeader("Connection")) {
+            String connection = upTmpHttpClient->header("Connection");
+            connection.toLowerCase();
+            restart = connection.indexOf("keep-alive") == -1;
+        }
+        if (restart) { upTmpHttpClient->restartTCP(); }
+
         httpCode = upTmpHttpClient->GET();
     }