From d2b01781c92080ea3d07687438f3a2cdde6106ea Mon Sep 17 00:00:00 2001 From: "Earle F. Philhower, III" Date: Thu, 17 Jun 2021 13:02:14 -0700 Subject: [PATCH 1/2] WiFiClientSecure owns context, fix stopAllExcept Fixes #8079 Because WiFiClientSecure inherits WiFiClient, and WiFiClientSecureCtx also inherits WiFiClient, they both end up in the list of TCP connections that are used for WiFiClient::stopAllExcept(). This would cause the underlying SSL connection to be closed whenever you attempted to stopAllExcept(WiFiClientSecure) Fix by adding a "_owner" pointer in the WiFiClient object which points to nullptr (default case) or to the associated upper-layer connection. When stopping all connections except one, only look at the uppermost connections. --- libraries/ESP8266WiFi/src/WiFiClient.cpp | 15 +++++++++++++-- libraries/ESP8266WiFi/src/WiFiClient.h | 1 + .../ESP8266WiFi/src/WiFiClientSecureBearSSL.h | 4 ++-- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/libraries/ESP8266WiFi/src/WiFiClient.cpp b/libraries/ESP8266WiFi/src/WiFiClient.cpp index 39f57b0637..db59b8321f 100644 --- a/libraries/ESP8266WiFi/src/WiFiClient.cpp +++ b/libraries/ESP8266WiFi/src/WiFiClient.cpp @@ -77,14 +77,14 @@ WiFiClient* SList::_s_first = 0; WiFiClient::WiFiClient() -: _client(0) +: _client(0), _owner(0) { _timeout = 5000; WiFiClient::_add(this); } WiFiClient::WiFiClient(ClientContext* client) -: _client(client) +: _client(client), _owner(0) { _timeout = 5000; _client->ref(); @@ -106,6 +106,7 @@ WiFiClient::WiFiClient(const WiFiClient& other) _client = other._client; _timeout = other._timeout; _localPort = other._localPort; + _owner = other._owner; if (_client) _client->ref(); WiFiClient::_add(this); @@ -118,6 +119,7 @@ WiFiClient& WiFiClient::operator=(const WiFiClient& other) _client = other._client; _timeout = other._timeout; _localPort = other._localPort; + _owner = other._owner; if (_client) _client->ref(); return *this; @@ -382,7 +384,16 @@ void WiFiClient::stopAll() void WiFiClient::stopAllExcept(WiFiClient* except) { + // Stop all will look at the highest-level wrapper connections only + // Find the "except" top-level connection + while (except->_owner) { + except = except->_owner; + } for (WiFiClient* it = _s_first; it; it = it->_next) { + // Find the top-level owner of the current list entry + while (it->_owner) { + it = it->_owner; + } if (it != except) { it->stop(); } diff --git a/libraries/ESP8266WiFi/src/WiFiClient.h b/libraries/ESP8266WiFi/src/WiFiClient.h index e178d3a483..5ada8b1453 100644 --- a/libraries/ESP8266WiFi/src/WiFiClient.h +++ b/libraries/ESP8266WiFi/src/WiFiClient.h @@ -144,6 +144,7 @@ class WiFiClient : public Client, public SList { void _err(int8_t err); ClientContext* _client; + WiFiClient* _owner; static uint16_t _localPort; }; diff --git a/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.h b/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.h index 8ed5edcd9d..977d9b2628 100644 --- a/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.h +++ b/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.h @@ -232,8 +232,8 @@ class WiFiClientSecure : public WiFiClient { public: - WiFiClientSecure():_ctx(new WiFiClientSecureCtx()) { } - WiFiClientSecure(const WiFiClientSecure &rhs): WiFiClient(), _ctx(rhs._ctx) { } + WiFiClientSecure():_ctx(new WiFiClientSecureCtx()) { _ctx->_owner = this; } + WiFiClientSecure(const WiFiClientSecure &rhs): WiFiClient(), _ctx(rhs._ctx) { if (_ctx) _ctx->_owner = this; } ~WiFiClientSecure() override { _ctx = nullptr; } WiFiClientSecure& operator=(const WiFiClientSecure&) = default; // The shared-ptrs handle themselves automatically From 415e888efa5c5fd495aa373e2dd08ac05c1ec8a6 Mon Sep 17 00:00:00 2001 From: "Earle F. Philhower, III" Date: Thu, 17 Jun 2021 13:40:57 -0700 Subject: [PATCH 2/2] Reverse logic, use lowest-level Ctx for match Because there may be multiple copies of the WiFiClientSecure, but only a single WiFiClientCtx, use the Ctx to check for relatedness on connections. This will avoid closing SSL connections that were assignment copies. --- libraries/ESP8266WiFi/src/WiFiClient.cpp | 26 +++++++++---------- libraries/ESP8266WiFi/src/WiFiClient.h | 2 +- .../ESP8266WiFi/src/WiFiClientSecureBearSSL.h | 4 +-- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/libraries/ESP8266WiFi/src/WiFiClient.cpp b/libraries/ESP8266WiFi/src/WiFiClient.cpp index db59b8321f..decc7d7ac0 100644 --- a/libraries/ESP8266WiFi/src/WiFiClient.cpp +++ b/libraries/ESP8266WiFi/src/WiFiClient.cpp @@ -77,14 +77,14 @@ WiFiClient* SList::_s_first = 0; WiFiClient::WiFiClient() -: _client(0), _owner(0) +: _client(0), _owned(0) { _timeout = 5000; WiFiClient::_add(this); } WiFiClient::WiFiClient(ClientContext* client) -: _client(client), _owner(0) +: _client(client), _owned(0) { _timeout = 5000; _client->ref(); @@ -106,7 +106,7 @@ WiFiClient::WiFiClient(const WiFiClient& other) _client = other._client; _timeout = other._timeout; _localPort = other._localPort; - _owner = other._owner; + _owned = other._owned; if (_client) _client->ref(); WiFiClient::_add(this); @@ -119,7 +119,7 @@ WiFiClient& WiFiClient::operator=(const WiFiClient& other) _client = other._client; _timeout = other._timeout; _localPort = other._localPort; - _owner = other._owner; + _owned = other._owned; if (_client) _client->ref(); return *this; @@ -384,18 +384,18 @@ void WiFiClient::stopAll() void WiFiClient::stopAllExcept(WiFiClient* except) { - // Stop all will look at the highest-level wrapper connections only - // Find the "except" top-level connection - while (except->_owner) { - except = except->_owner; + // Stop all will look at the lowest-level wrapper connections only + while (except->_owned) { + except = except->_owned; } for (WiFiClient* it = _s_first; it; it = it->_next) { - // Find the top-level owner of the current list entry - while (it->_owner) { - it = it->_owner; + WiFiClient* conn = it; + // Find the lowest-level owner of the current list entry + while (conn->_owned) { + conn = conn->_owned; } - if (it != except) { - it->stop(); + if (conn != except) { + conn->stop(); } } } diff --git a/libraries/ESP8266WiFi/src/WiFiClient.h b/libraries/ESP8266WiFi/src/WiFiClient.h index 5ada8b1453..038e8032df 100644 --- a/libraries/ESP8266WiFi/src/WiFiClient.h +++ b/libraries/ESP8266WiFi/src/WiFiClient.h @@ -144,7 +144,7 @@ class WiFiClient : public Client, public SList { void _err(int8_t err); ClientContext* _client; - WiFiClient* _owner; + WiFiClient* _owned; static uint16_t _localPort; }; diff --git a/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.h b/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.h index 977d9b2628..166c29c603 100644 --- a/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.h +++ b/libraries/ESP8266WiFi/src/WiFiClientSecureBearSSL.h @@ -232,8 +232,8 @@ class WiFiClientSecure : public WiFiClient { public: - WiFiClientSecure():_ctx(new WiFiClientSecureCtx()) { _ctx->_owner = this; } - WiFiClientSecure(const WiFiClientSecure &rhs): WiFiClient(), _ctx(rhs._ctx) { if (_ctx) _ctx->_owner = this; } + WiFiClientSecure():_ctx(new WiFiClientSecureCtx()) { _owned = _ctx.get(); } + WiFiClientSecure(const WiFiClientSecure &rhs): WiFiClient(), _ctx(rhs._ctx) { if (_ctx) _owned = _ctx.get(); } ~WiFiClientSecure() override { _ctx = nullptr; } WiFiClientSecure& operator=(const WiFiClientSecure&) = default; // The shared-ptrs handle themselves automatically