Skip to content

Commit

Permalink
Fix: restart TCP connection if HTTP server sends Connection:close
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
schlimmchen committed Aug 28, 2024
1 parent b5785d0 commit 63612e9
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 4 deletions.
12 changes: 11 additions & 1 deletion include/HttpGetter.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
21 changes: 18 additions & 3 deletions src/HttpGetter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
}
}
Expand All @@ -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();
}

Expand Down

0 comments on commit 63612e9

Please sign in to comment.