diff --git a/CHANGELOG.md b/CHANGELOG.md index bed6722d3..f1088c4b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -113,6 +113,7 @@ - Dev: Updated `pajlada-settings` to v0.5.0. (#6797) - Dev: Updated `pajlada-serialize` to v0.2.1. (#6797) - Dev: Updated `pajlada-signals` to v0.1.1. (#6797) +- Dev: Balance IPv4 and IPv6 connection attempts. (#6804) ## 2.5.4 diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 0d2e24485..29d3cf85a 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -61,6 +61,8 @@ set(SOURCE_FILES common/websockets/WebSocketPool.cpp common/websockets/WebSocketPool.hpp + common/websockets/detail/BalancedResolverResults.cpp + common/websockets/detail/BalancedResolverResults.hpp common/websockets/detail/WebSocketConnection.cpp common/websockets/detail/WebSocketConnection.hpp common/websockets/detail/WebSocketConnectionImpl.cpp diff --git a/src/common/websockets/detail/BalancedResolverResults.cpp b/src/common/websockets/detail/BalancedResolverResults.cpp new file mode 100644 index 000000000..c770eed2f --- /dev/null +++ b/src/common/websockets/detail/BalancedResolverResults.cpp @@ -0,0 +1,88 @@ +// SPDX-FileCopyrightText: 2026 Contributors to Chatterino +// +// SPDX-License-Identifier: MIT + +#include "common/websockets/detail/BalancedResolverResults.hpp" + +namespace chatterino::ws::detail { + +BalancedResolverResults::BalancedResolverResults( + const Protocol::resolver::results_type &results) + : entries(results.begin(), results.end()) +{ +} + +BalancedResolverResults::BalancedResolverResults(std::vector entries) + : entries(std::move(entries)) +{ +} + +std::optional + BalancedResolverResults::advanceEntry() +{ + if (this->nextIsIPv6) + { + // advance v6 first + if (this->advanceIPv6() || this->advanceIPv4()) + { + return this->currentEntry(); + } + } + else + { + // advance v4 first + if (this->advanceIPv4() || this->advanceIPv6()) + { + return this->currentEntry(); + } + } + return std::nullopt; +} + +std::optional + BalancedResolverResults::currentEntry() const +{ + if (this->currentIdx < this->entries.size()) + { + return this->entries[this->currentIdx]; + } + return std::nullopt; +} + +void BalancedResolverResults::reset() +{ + this->nextIPv4Idx = 0; + this->nextIPv6Idx = 0; + this->currentIdx = std::numeric_limits::max(); + this->nextIsIPv6 = true; +} + +bool BalancedResolverResults::advanceIPv4() +{ + for (; this->nextIPv4Idx < this->entries.size(); this->nextIPv4Idx++) + { + if (this->entries[this->nextIPv4Idx].endpoint().address().is_v4()) + { + this->currentIdx = this->nextIPv4Idx++; + this->nextIsIPv6 = true; + return true; + } + } + return false; +} + +bool BalancedResolverResults::advanceIPv6() +{ + for (; this->nextIPv6Idx < this->entries.size(); this->nextIPv6Idx++) + { + if (this->entries[this->nextIPv6Idx].endpoint().address().is_v6()) + { + this->currentIdx = this->nextIPv6Idx++; + this->nextIsIPv6 = false; + return true; + } + } + return false; +} + +} // namespace chatterino::ws::detail diff --git a/src/common/websockets/detail/BalancedResolverResults.hpp b/src/common/websockets/detail/BalancedResolverResults.hpp new file mode 100644 index 000000000..8c75bf6f8 --- /dev/null +++ b/src/common/websockets/detail/BalancedResolverResults.hpp @@ -0,0 +1,62 @@ +// SPDX-FileCopyrightText: 2026 Contributors to Chatterino +// +// SPDX-License-Identifier: MIT + +#pragma once + +#include +#include + +#include +#include + +namespace chatterino::ws::detail { + +/// An iterator over resolver results ensuring IPv4 and IPv6 entries are +/// balanced when iterating where possible. +/// +/// If enough records are available, an IPv4 record is followed by an IPv6 one +/// and vice versa. The resolver starts with IPv4 entries. +class BalancedResolverResults +{ +public: + using Protocol = boost::asio::ip::tcp; + using Entry = boost::asio::ip::basic_resolver_entry; + + BalancedResolverResults() = default; + explicit BalancedResolverResults( + const Protocol::resolver::results_type &results); + explicit BalancedResolverResults(std::vector entries); + + /// Advance to the next entry and return that one. + /// + /// Once the end is reached, `std::nullopt` is returned. + std::optional advanceEntry(); + + /// Get the current entry. + /// + /// This only returns an entry after `advanceEntry` has been called. + std::optional currentEntry() const; + + /// Reset the iteration state. + void reset(); + +private: + /// Advance the IPv4 iterator. + /// Returns true if `currentIdx` now points to an IPv4 entry. + bool advanceIPv4(); + + /// Advance the IPv6 iterator. + /// Returns true if `currentIdx` now points to an IPv6 entry. + bool advanceIPv6(); + + size_t nextIPv4Idx = 0; + size_t nextIPv6Idx = 0; + bool nextIsIPv6 = true; + + size_t currentIdx = std::numeric_limits::max(); + + std::vector entries; +}; + +} // namespace chatterino::ws::detail diff --git a/src/common/websockets/detail/WebSocketConnectionImpl.cpp b/src/common/websockets/detail/WebSocketConnectionImpl.cpp index d73c7b3b5..31a82c562 100644 --- a/src/common/websockets/detail/WebSocketConnectionImpl.cpp +++ b/src/common/websockets/detail/WebSocketConnectionImpl.cpp @@ -99,23 +99,22 @@ void WebSocketConnectionHelper::onResolve( return; } - this->resolvedEndpoints = results; + this->resolvedEndpoints = BalancedResolverResults(results); - this->tryConnect(this->resolvedEndpoints.begin()); + this->tryConnect(this->resolvedEndpoints.advanceEntry()); } template void WebSocketConnectionHelper::tryConnect( - boost::asio::ip::tcp::resolver::results_type::const_iterator - endpointIterator) + std::optional entry) { - if (endpointIterator == this->resolvedEndpoints.end()) + if (!entry) { this->fail("Ran out of resolved endpoints"sv, u"connect"); return; } - const auto &endpoint = endpointIterator->endpoint(); + auto endpoint = entry->endpoint(); qCDebug(chatterinoWebsocket) << *this << "connect to" << endpoint.address().to_string(); @@ -127,16 +126,14 @@ void WebSocketConnectionHelper::tryConnect( .async_connect(endpoint, beast::bind_front_handler( &WebSocketConnectionHelper::onTcpHandshake, - this->shared_from_this(), endpointIterator)); + this->shared_from_this(), *std::move(entry))); } template void WebSocketConnectionHelper::onTcpHandshake( - boost::asio::ip::tcp::resolver::results_type::const_iterator - endpointIterator, - boost::system::error_code ec) + const BalancedResolverResults::Entry &entry, boost::system::error_code ec) { - const auto &ep = endpointIterator->endpoint(); + const auto &ep = entry.endpoint(); if (ec) { @@ -151,7 +148,7 @@ void WebSocketConnectionHelper::onTcpHandshake( << *this << "closing websocket after error" << ec.message(); } - this->tryConnect(++endpointIterator); + this->tryConnect(this->resolvedEndpoints.advanceEntry()); return; } diff --git a/src/common/websockets/detail/WebSocketConnectionImpl.hpp b/src/common/websockets/detail/WebSocketConnectionImpl.hpp index d35c91ce5..406f6f2a8 100644 --- a/src/common/websockets/detail/WebSocketConnectionImpl.hpp +++ b/src/common/websockets/detail/WebSocketConnectionImpl.hpp @@ -4,6 +4,7 @@ #pragma once +#include "common/websockets/detail/BalancedResolverResults.hpp" #include "common/websockets/detail/WebSocketConnection.hpp" #include @@ -69,12 +70,9 @@ private: /// /// If the iterator is invalid, we have run out of endpoints to try, and deem this /// connection a failure. - void tryConnect(boost::asio::ip::tcp::resolver::results_type::const_iterator - endpointIterator); - void onTcpHandshake( - boost::asio::ip::tcp::resolver::results_type::const_iterator - endpointIterator, - boost::system::error_code ec); + void tryConnect(std::optional entry); + void onTcpHandshake(const BalancedResolverResults::Entry &entry, + boost::system::error_code ec); void onWsHandshake(boost::system::error_code ec); void onReadDone(boost::system::error_code ec, size_t bytesRead); @@ -86,7 +84,7 @@ private: /// /// When we successfully resolve the host, we try to connect by /// iterating over these results. - boost::asio::ip::tcp::resolver::results_type resolvedEndpoints; + BalancedResolverResults resolvedEndpoints; }; /// A WebSocket connection over TLS (wss://). diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index ac3ce1cbb..f363f2bec 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -61,6 +61,7 @@ set(test_SOURCES ${CMAKE_CURRENT_LIST_DIR}/src/TwitchUserColor.cpp ${CMAKE_CURRENT_LIST_DIR}/src/FunctionRef.cpp ${CMAKE_CURRENT_LIST_DIR}/src/InputHighlighter.cpp + ${CMAKE_CURRENT_LIST_DIR}/src/BalancedResolverResults.cpp ${CMAKE_CURRENT_LIST_DIR}/src/lib/Snapshot.cpp ${CMAKE_CURRENT_LIST_DIR}/src/lib/Snapshot.hpp diff --git a/tests/src/BalancedResolverResults.cpp b/tests/src/BalancedResolverResults.cpp new file mode 100644 index 000000000..10f5565c0 --- /dev/null +++ b/tests/src/BalancedResolverResults.cpp @@ -0,0 +1,246 @@ +// SPDX-FileCopyrightText: 2026 Contributors to Chatterino +// +// SPDX-License-Identifier: MIT + +#include "common/websockets/detail/BalancedResolverResults.hpp" + +#include "Test.hpp" + +#include + +using namespace chatterino::ws::detail; + +namespace { + +using Entry = BalancedResolverResults::Entry; + +void runCheck(BalancedResolverResults results, + const std::vector &expectedOrder) +{ + ASSERT_FALSE(expectedOrder.empty()); + + for (size_t j = 0; j < 4; j++) + { + EXPECT_EQ(results.currentEntry(), std::nullopt); + for (size_t i = 0; i < expectedOrder.size(); i++) + { + EXPECT_EQ(results.advanceEntry()->endpoint(), + expectedOrder[i].endpoint()) + << i; + EXPECT_EQ(results.currentEntry()->endpoint(), + expectedOrder[i].endpoint()) + << i; + } + EXPECT_EQ(results.advanceEntry(), std::nullopt); + EXPECT_EQ(results.currentEntry()->endpoint(), + expectedOrder.back().endpoint()); + + results.reset(); + } +} + +Entry makeEntry(const boost::asio::ip::address &addr) +{ + return { + boost::asio::ip::tcp::endpoint{addr, 443}, + "host.example.com", + "service", + }; +} + +Entry makeV4(const char *spec) +{ + return makeEntry(boost::asio::ip::make_address_v4(spec)); +} + +Entry makeV6(const char *spec) +{ + return makeEntry(boost::asio::ip::make_address_v6(spec)); +} + +} // namespace + +TEST(BalancedResolverResults, empty) +{ + BalancedResolverResults results; + ASSERT_EQ(results.currentEntry(), std::nullopt); + ASSERT_EQ(results.advanceEntry(), std::nullopt); + ASSERT_EQ(results.currentEntry(), std::nullopt); + ASSERT_EQ(results.advanceEntry(), std::nullopt); + ASSERT_EQ(results.currentEntry(), std::nullopt); + ASSERT_EQ(results.advanceEntry(), std::nullopt); + ASSERT_EQ(results.currentEntry(), std::nullopt); + ASSERT_EQ(results.advanceEntry(), std::nullopt); + results.reset(); + ASSERT_EQ(results.currentEntry(), std::nullopt); + ASSERT_EQ(results.advanceEntry(), std::nullopt); +} + +TEST(BalancedResolverResults, singleV4) +{ + std::vector entries{makeV4("10.10.10.1")}; + runCheck(BalancedResolverResults(entries), entries); +} + +TEST(BalancedResolverResults, singleV6) +{ + std::vector entries{makeV6("1234:5678:9abc:def::1")}; + runCheck(BalancedResolverResults(entries), entries); +} + +TEST(BalancedResolverResults, singleV4AndV6) +{ + std::vector entries{ + makeV6("1234:5678:9abc:def::1"), + makeV4("10.10.10.1"), + }; + runCheck(BalancedResolverResults(entries), entries); +} + +TEST(BalancedResolverResults, doubleV4) +{ + runCheck(BalancedResolverResults({ + makeV4("10.10.10.1"), + makeV4("10.10.10.2"), + }), + { + makeV4("10.10.10.1"), + makeV4("10.10.10.2"), + }); +} + +TEST(BalancedResolverResults, doubleV6) +{ + runCheck(BalancedResolverResults({ + makeV6("1234:5678:9abc:def::1"), + makeV6("1234:5678:9abc:def::2"), + }), + { + makeV6("1234:5678:9abc:def::1"), + makeV6("1234:5678:9abc:def::2"), + }); +} + +TEST(BalancedResolverResults, doubleV4SingleV6) +{ + runCheck(BalancedResolverResults({ + makeV6("1234:5678:9abc:def::1"), + makeV4("10.10.10.1"), + makeV4("10.10.10.2"), + }), + { + makeV6("1234:5678:9abc:def::1"), + makeV4("10.10.10.1"), + makeV4("10.10.10.2"), + }); +} + +TEST(BalancedResolverResults, singleV4DoubleV6) +{ + runCheck(BalancedResolverResults({ + makeV6("1234:5678:9abc:def::1"), + makeV6("1234:5678:9abc:def::2"), + makeV4("10.10.10.1"), + }), + { + makeV6("1234:5678:9abc:def::1"), + makeV4("10.10.10.1"), + makeV6("1234:5678:9abc:def::2"), + }); +} + +TEST(BalancedResolverResults, doubleV4AndV6) +{ + runCheck(BalancedResolverResults({ + makeV4("10.10.10.1"), + makeV4("10.10.10.2"), + makeV6("1234:5678:9abc:def::1"), + makeV6("1234:5678:9abc:def::2"), + }), + { + makeV6("1234:5678:9abc:def::1"), + makeV4("10.10.10.1"), + makeV6("1234:5678:9abc:def::2"), + makeV4("10.10.10.2"), + }); +} + +TEST(BalancedResolverResults, tripleV6) +{ + runCheck(BalancedResolverResults({ + makeV6("1234:5678:9abc:def::1"), + makeV6("1234:5678:9abc:def::2"), + makeV6("1234:5678:9abc:def::3"), + }), + { + makeV6("1234:5678:9abc:def::1"), + makeV6("1234:5678:9abc:def::2"), + makeV6("1234:5678:9abc:def::3"), + }); +} + +TEST(BalancedResolverResults, tripleV4) +{ + runCheck(BalancedResolverResults({ + makeV4("10.10.10.1"), + makeV4("10.10.10.2"), + makeV4("10.10.10.3"), + }), + { + makeV4("10.10.10.1"), + makeV4("10.10.10.2"), + makeV4("10.10.10.3"), + }); +} + +TEST(BalancedResolverResults, tripleV4SingleV6) +{ + runCheck(BalancedResolverResults({ + makeV4("10.10.10.1"), + makeV4("10.10.10.2"), + makeV6("1234:5678:9abc:def::1"), + makeV4("10.10.10.3"), + }), + { + makeV6("1234:5678:9abc:def::1"), + makeV4("10.10.10.1"), + makeV4("10.10.10.2"), + makeV4("10.10.10.3"), + }); +} + +TEST(BalancedResolverResults, singleV4TripleV6) +{ + runCheck(BalancedResolverResults({ + makeV6("1234:5678:9abc:def::1"), + makeV6("1234:5678:9abc:def::2"), + makeV4("10.10.10.1"), + makeV6("1234:5678:9abc:def::3"), + }), + { + makeV6("1234:5678:9abc:def::1"), + makeV4("10.10.10.1"), + makeV6("1234:5678:9abc:def::2"), + makeV6("1234:5678:9abc:def::3"), + }); +} + +TEST(BalancedResolverResults, tripleV4AndV6) +{ + runCheck(BalancedResolverResults({ + makeV4("10.10.10.1"), + makeV4("10.10.10.2"), + makeV4("10.10.10.3"), + makeV6("1234:5678:9abc:def::1"), + makeV6("1234:5678:9abc:def::2"), + makeV6("1234:5678:9abc:def::3"), + }), + { + makeV6("1234:5678:9abc:def::1"), + makeV4("10.10.10.1"), + makeV6("1234:5678:9abc:def::2"), + makeV4("10.10.10.2"), + makeV6("1234:5678:9abc:def::3"), + makeV4("10.10.10.3"), + }); +}