From 584e95fea5783037c9501d5fc9be72fb89a36715 Mon Sep 17 00:00:00 2001 From: mikee47 Date: Mon, 11 Oct 2021 08:39:15 +0100 Subject: [PATCH 1/6] Ensure request callback is invoked on client connection error --- .../src/Network/Http/HttpClientConnection.cpp | 14 ++++++++++++++ .../src/Network/Http/HttpClientConnection.h | 1 + 2 files changed, 15 insertions(+) diff --git a/Sming/Components/Network/src/Network/Http/HttpClientConnection.cpp b/Sming/Components/Network/src/Network/Http/HttpClientConnection.cpp index d939bfb915..28d08b6042 100644 --- a/Sming/Components/Network/src/Network/Http/HttpClientConnection.cpp +++ b/Sming/Components/Network/src/Network/Http/HttpClientConnection.cpp @@ -61,6 +61,20 @@ void HttpClientConnection::reset() HttpConnection::reset(); } +void HttpClientConnection::onError(err_t err) +{ + HttpConnection::onError(err); + + auto request = waitingQueue.peek(); + + if(request != nullptr) { + auto callback = request->requestCompletedDelegate; + if(callback) { + callback(*this, false); + } + } +} + int HttpClientConnection::onMessageBegin(http_parser* parser) { incomingRequest = executionQueue.peek(); diff --git a/Sming/Components/Network/src/Network/Http/HttpClientConnection.h b/Sming/Components/Network/src/Network/Http/HttpClientConnection.h index 2720594f6a..43f590179c 100644 --- a/Sming/Components/Network/src/Network/Http/HttpClientConnection.h +++ b/Sming/Components/Network/src/Network/Http/HttpClientConnection.h @@ -68,6 +68,7 @@ class HttpClientConnection : public HttpConnection // TCP methods void onReadyToSendData(TcpConnectionEvent sourceEvent) override; + void onError(err_t err) override; void onClosed() override; From 952af718cb78259ee91e9d9a5c81ae7b18da7938 Mon Sep 17 00:00:00 2001 From: mikee47 Date: Sun, 27 Feb 2022 13:47:57 +0000 Subject: [PATCH 2/6] Add debug message to clarify websocket header failure --- Sming/Components/Network/src/Network/WebsocketClient.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/Sming/Components/Network/src/Network/WebsocketClient.cpp b/Sming/Components/Network/src/Network/WebsocketClient.cpp index fd464252eb..5b598cc4b5 100644 --- a/Sming/Components/Network/src/Network/WebsocketClient.cpp +++ b/Sming/Components/Network/src/Network/WebsocketClient.cpp @@ -87,6 +87,7 @@ bool WebsocketClient::connect(const Url& url) int WebsocketClient::verifyKey(HttpConnection& connection, HttpResponse& response) { if(!response.headers.contains(HTTP_HEADER_SEC_WEBSOCKET_ACCEPT)) { + debug_e("[WS] Websocket Accept missing from headers"); state = eWSCS_Closed; return -2; // we don't have response. } From 44956ba30cd185b3b7c4ba34e99bc3f206de68cf Mon Sep 17 00:00:00 2001 From: mikee47 Date: Sat, 22 Jun 2024 13:44:11 +0100 Subject: [PATCH 3/6] Handle failed websocket handshake If websocket upgrade fails then do NOT fail http request as it'll get retried multiple times. --- Sming/Components/Network/src/Network/WebsocketClient.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Sming/Components/Network/src/Network/WebsocketClient.cpp b/Sming/Components/Network/src/Network/WebsocketClient.cpp index 5b598cc4b5..ee9f6ef784 100644 --- a/Sming/Components/Network/src/Network/WebsocketClient.cpp +++ b/Sming/Components/Network/src/Network/WebsocketClient.cpp @@ -74,6 +74,12 @@ bool WebsocketClient::connect(const Url& url) request->headers[HTTP_HEADER_SEC_WEBSOCKET_PROTOCOL] = F("chat"); request->headers[HTTP_HEADER_SEC_WEBSOCKET_VERSION] = String(WEBSOCKET_VERSION); request->onHeadersComplete(RequestHeadersCompletedDelegate(&WebsocketClient::verifyKey, this)); + request->onRequestComplete([this](HttpConnection& client, bool successful) -> int { + if(state != eWSCS_Open && wsDisconnect) { + wsDisconnect(*this); + } + return 0; + }); if(!httpConnection->send(request)) { return false; From 079f86c85bdc2b509407f86d6b17412bbf3f8c60 Mon Sep 17 00:00:00 2001 From: mikee47 Date: Sat, 22 Jun 2024 22:09:45 +0100 Subject: [PATCH 4/6] Tracing what happens with WebsocketClient WS_URL=ws://echo.websocket.org --- .../Network/src/Network/WebsocketClient.cpp | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/Sming/Components/Network/src/Network/WebsocketClient.cpp b/Sming/Components/Network/src/Network/WebsocketClient.cpp index ee9f6ef784..ccffc69809 100644 --- a/Sming/Components/Network/src/Network/WebsocketClient.cpp +++ b/Sming/Components/Network/src/Network/WebsocketClient.cpp @@ -23,6 +23,8 @@ class WebsocketClientConnection : public HttpClientConnection // Prevent HttpClientConnection from resetting our receive delegate set by activate() call err_t onConnected(err_t err) override { +debug_i("%s CONNECTED", __PRETTY_FUNCTION__); + if(err == ERR_OK) { state = eHCS_Ready; } @@ -30,6 +32,18 @@ class WebsocketClientConnection : public HttpClientConnection // IMPORTANT: Skip HttpClientConnection implementation return HttpConnection::onConnected(err); } + + void onClosed() override + { +debug_i("%s CLOSED", __PRETTY_FUNCTION__); +HttpClientConnection::onClosed(); + } + + void onError(err_t err) override + { +debug_i("%s ERROR %d", __PRETTY_FUNCTION__, err); +HttpClientConnection::onError(err); + } }; HttpConnection* WebsocketClient::getHttpConnection() @@ -45,15 +59,18 @@ HttpConnection* WebsocketClient::getHttpConnection() bool WebsocketClient::connect(const Url& url) { +debug_i("%s CONNECT", __PRETTY_FUNCTION__); uri = url; bool useSsl = (uri.Scheme == URI_SCHEME_WEBSOCKET_SECURE); HttpConnection* httpConnection = getHttpConnection(); if(httpConnection == nullptr) { +debug_i("%s NO CONNECTION", __PRETTY_FUNCTION__); return false; } if(httpConnection->isProcessing()) { +debug_i("%s PROCESSING", __PRETTY_FUNCTION__); return false; } @@ -75,6 +92,7 @@ bool WebsocketClient::connect(const Url& url) request->headers[HTTP_HEADER_SEC_WEBSOCKET_VERSION] = String(WEBSOCKET_VERSION); request->onHeadersComplete(RequestHeadersCompletedDelegate(&WebsocketClient::verifyKey, this)); request->onRequestComplete([this](HttpConnection& client, bool successful) -> int { + debug_w("XXXXX successful %u", successful); if(state != eWSCS_Open && wsDisconnect) { wsDisconnect(*this); } @@ -82,16 +100,21 @@ bool WebsocketClient::connect(const Url& url) }); if(!httpConnection->send(request)) { +debug_i("%s SEND", __PRETTY_FUNCTION__); return false; } WebsocketConnection::setUserData(this); +debug_i("%s CONNECTING", __PRETTY_FUNCTION__); + return true; } int WebsocketClient::verifyKey(HttpConnection& connection, HttpResponse& response) { +debug_i("%s", __PRETTY_FUNCTION__); + if(!response.headers.contains(HTTP_HEADER_SEC_WEBSOCKET_ACCEPT)) { debug_e("[WS] Websocket Accept missing from headers"); state = eWSCS_Closed; From aaabb6cca8ac186b8fda3b9e1503d914787c3ec1 Mon Sep 17 00:00:00 2001 From: mikee47 Date: Sun, 23 Jun 2024 08:09:04 +0100 Subject: [PATCH 5/6] Add `Url::getQueryParameter` method --- Sming/Components/Network/src/Network/Http/HttpRequest.h | 2 +- Sming/Components/Network/src/Network/Url.h | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/Sming/Components/Network/src/Network/Http/HttpRequest.h b/Sming/Components/Network/src/Network/Http/HttpRequest.h index 5b0edd7240..6634a7984c 100644 --- a/Sming/Components/Network/src/Network/Http/HttpRequest.h +++ b/Sming/Components/Network/src/Network/Http/HttpRequest.h @@ -155,7 +155,7 @@ class HttpRequest */ String getQueryParameter(const String& name, const String& defaultValue = nullptr) const { - return static_cast(uri.Query)[name] ?: defaultValue; + return uri.getQueryParam(name, defaultValue); } /** diff --git a/Sming/Components/Network/src/Network/Url.h b/Sming/Components/Network/src/Network/Url.h index 9a2f4977d5..1b08ce1b59 100644 --- a/Sming/Components/Network/src/Network/Url.h +++ b/Sming/Components/Network/src/Network/Url.h @@ -166,6 +166,14 @@ class Url */ void debugPrintTo(Print& p) const; + /** + * @brief Get a query parameter without risk of modification + */ + const String& getQueryParam(const String& name, const String& defaultValue = nullptr) const + { + return Query[name] ?: defaultValue; + } + public: String Scheme; ///< without ":" and "//" String User; From 0b01eb767f7ef77d06bad0737362d70b271a1e06 Mon Sep 17 00:00:00 2001 From: mikee47 Date: Sun, 23 Jun 2024 08:17:08 +0100 Subject: [PATCH 6/6] Ensure Url provides default port for default scheme --- Sming/Components/Network/src/Network/Url.cpp | 10 ++++++++++ Sming/Components/Network/src/Network/Url.h | 8 +++++++- tests/HostTests/modules/Network/Url.cpp | 5 ++++- 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/Sming/Components/Network/src/Network/Url.cpp b/Sming/Components/Network/src/Network/Url.cpp index 722ec71648..2339f016d7 100644 --- a/Sming/Components/Network/src/Network/Url.cpp +++ b/Sming/Components/Network/src/Network/Url.cpp @@ -51,6 +51,16 @@ Url& Url::operator=(String urlString) return *this; } +String Url::getScheme() const +{ + if(!Scheme) { + return URI_SCHEME_DEFAULT; + } + String s = Scheme; + s.toLowerCase(); + return s; +} + int Url::getDefaultPort(const String& scheme) { #define XX(name, str, port) \ diff --git a/Sming/Components/Network/src/Network/Url.h b/Sming/Components/Network/src/Network/Url.h index 1b08ce1b59..58f54654ea 100644 --- a/Sming/Components/Network/src/Network/Url.h +++ b/Sming/Components/Network/src/Network/Url.h @@ -121,6 +121,12 @@ class Url return toString(); } + /** + * @brief Get the applicable scheme, using the default if not specified + * The returned string is always lowercase. + */ + String getScheme() const; + /** @brief Obtain the default port for a given scheme * @retval int 0 if scheme is not recognised or has no standard port defined */ @@ -132,7 +138,7 @@ class Url */ int getPort() const { - return Port ?: getDefaultPort(Scheme); + return Port ?: getDefaultPort(getScheme()); } /** @brief Get hostname+port part of URL string diff --git a/tests/HostTests/modules/Network/Url.cpp b/tests/HostTests/modules/Network/Url.cpp index 812de0fb83..a12a35d0ce 100644 --- a/tests/HostTests/modules/Network/Url.cpp +++ b/tests/HostTests/modules/Network/Url.cpp @@ -119,7 +119,10 @@ class UrlTest : public TestGroup uri.Query["key"] = "7XXUJWCWYTMXKN3L"; int sensorValue = 347; uri.Query["field1"] = String(sensorValue); - REQUIRE(uri.toString() == FS_url); + REQUIRE_EQ(uri.toString(), FS_url); + REQUIRE(!uri.Scheme); + REQUIRE_EQ(uri.Port, 0); + REQUIRE_EQ(uri.getPort(), 80); } }