diff --git a/implementations/Crafter.Network-ClientQUIC.cpp b/implementations/Crafter.Network-ClientQUIC.cpp index a72528b..1857f59 100644 --- a/implementations/Crafter.Network-ClientQUIC.cpp +++ b/implementations/Crafter.Network-ClientQUIC.cpp @@ -554,18 +554,22 @@ static HQUIC OpenClientConfiguration(const std::string& alpn, const QUICClientCr QUIC_SETTINGS settings{}; settings.IsSet.IdleTimeoutMs = 1; - settings.IdleTimeoutMs = creds.settings.idleTimeoutMs; + settings.IdleTimeoutMs = 120'000; + // Bound the handshake separately from the (much longer) idle timeout. + // The constructor blocks until msquic reports the connection either up or + // shut down; without this, dialling a host that never answers leaves it + // waiting for the idle timeout — minutes rather than seconds. settings.IsSet.HandshakeIdleTimeoutMs = 1; - settings.HandshakeIdleTimeoutMs = creds.settings.handshakeIdleTimeoutMs; + settings.HandshakeIdleTimeoutMs = 10'000; // Keep the connection alive across long idle gaps. msquic sends PING frames // on its own timer thread (independent of the app), so a request/response // connection survives even while the app is blocked for a long time between // requests (e.g. a client waiting on slow LLM inference). Interval < idle // timeout so the peer's idle timer never expires. settings.IsSet.KeepAliveIntervalMs = 1; - settings.KeepAliveIntervalMs = creds.settings.keepAliveIntervalMs; + settings.KeepAliveIntervalMs = 10'000; settings.IsSet.DatagramReceiveEnabled = 1; - settings.DatagramReceiveEnabled = creds.settings.datagramReceiveEnabled ? 1 : 0; + settings.DatagramReceiveEnabled = 1; // Allow the server to open unidi/bidi streams to us. msquic defaults // both peer-stream-count limits to 0; with that, the server's HTTP/3 // control stream + QPACK encoder/decoder streams can't be created and @@ -573,9 +577,9 @@ static HQUIC OpenClientConfiguration(const std::string& alpn, const QUICClientCr // currently use server push (h3 pushes ride on unidi 0x01 streams) but // the bidi cap is harmless to grant. settings.IsSet.PeerUnidiStreamCount = 1; - settings.PeerUnidiStreamCount = creds.settings.peerUnidiStreamCount; + settings.PeerUnidiStreamCount = 16; settings.IsSet.PeerBidiStreamCount = 1; - settings.PeerBidiStreamCount = creds.settings.peerBidiStreamCount; + settings.PeerBidiStreamCount = 16; HQUIC cfg = nullptr; QUIC_STATUS s = Runtime().api->ConfigurationOpen(Runtime().registration, &alpnBuffer, 1, diff --git a/implementations/Crafter.Network-ListenerQUIC.cpp b/implementations/Crafter.Network-ListenerQUIC.cpp index e3c43fe..1b446c9 100644 --- a/implementations/Crafter.Network-ListenerQUIC.cpp +++ b/implementations/Crafter.Network-ListenerQUIC.cpp @@ -220,15 +220,13 @@ static HQUIC OpenServerConfiguration(const std::string& alpn, QUIC_SETTINGS settings{}; settings.IsSet.IdleTimeoutMs = 1; - settings.IdleTimeoutMs = creds.settings.idleTimeoutMs; - settings.IsSet.HandshakeIdleTimeoutMs = 1; - settings.HandshakeIdleTimeoutMs = creds.settings.handshakeIdleTimeoutMs; + settings.IdleTimeoutMs = 120'000; settings.IsSet.PeerBidiStreamCount = 1; - settings.PeerBidiStreamCount = creds.settings.peerBidiStreamCount; + settings.PeerBidiStreamCount = 16; settings.IsSet.PeerUnidiStreamCount = 1; - settings.PeerUnidiStreamCount = creds.settings.peerUnidiStreamCount; + settings.PeerUnidiStreamCount = 16; settings.IsSet.DatagramReceiveEnabled = 1; - settings.DatagramReceiveEnabled = creds.settings.datagramReceiveEnabled ? 1 : 0; + settings.DatagramReceiveEnabled = 1; settings.IsSet.ServerResumptionLevel = 1; settings.ServerResumptionLevel = QUIC_SERVER_RESUME_AND_ZERORTT; diff --git a/interfaces/Crafter.Network-ClientQUIC.cppm b/interfaces/Crafter.Network-ClientQUIC.cppm index 5c6182a..d93c5e0 100644 --- a/interfaces/Crafter.Network-ClientQUIC.cppm +++ b/interfaces/Crafter.Network-ClientQUIC.cppm @@ -21,42 +21,12 @@ namespace Crafter { const char* what() const noexcept override { return "QUIC connection closed"; } }; - // Transport knobs applied to the msquic Configuration. The defaults - // reproduce the values these were hardcoded to before they were made - // configurable, so leaving this alone keeps the previous behaviour. - // - // peerUnidiStreamCount / peerBidiStreamCount cap how many streams the - // *peer* may open toward us. msquic defaults both to 0; a server's - // HTTP/3 control + QPACK streams alone need 3, and a protocol that - // fans data out over many concurrent streams needs correspondingly - // more. Raising these costs per-stream flow-control state, so raise - // them to what a protocol actually needs rather than to a large - // round number. - // - // keepAliveIntervalMs is client-only (the listener does not set it). - // Keep it below idleTimeoutMs or the peer's idle timer will expire. - // handshakeIdleTimeoutMs bounds how long a connect attempt to an - // unreachable peer takes to fail. This one is set explicitly where it - // previously was not: ClientQUIC's constructor waits on a condition - // variable with no timeout of its own, so without a bound here a client - // dialling a dead port blocks for minutes rather than failing and - // retrying. 10s matches msquic's own documented default. - export struct QUICSettings { - std::uint32_t idleTimeoutMs = 120'000; - std::uint32_t handshakeIdleTimeoutMs = 10'000; - std::uint32_t keepAliveIntervalMs = 10'000; - std::uint16_t peerUnidiStreamCount = 16; - std::uint16_t peerBidiStreamCount = 16; - bool datagramReceiveEnabled = true; - }; - // Server certificate sources. Pick one of (filePaths) or (selfSigned). // selfSigned generates an in-memory ephemeral cert — fine for dev/LAN. export struct QUICServerCredentials { std::string certPath; std::string keyPath; bool selfSigned = false; - QUICSettings settings; }; // Client-side credential validation. By default we require a real cert. @@ -70,7 +40,6 @@ namespace Crafter { export struct QUICClientCredentials { bool insecureNoServerValidation = false; std::array serverCertificateHash{}; - QUICSettings settings; }; export class ClientQUIC; diff --git a/project.cpp b/project.cpp index 6403c65..0d6faa1 100644 --- a/project.cpp +++ b/project.cpp @@ -133,7 +133,6 @@ extern "C" Configuration CrafterBuildProject(std::span a if (cfg.target == "x86_64-pc-linux-gnu") { cfg.AddTest("ShouldEchoWebTransport").Dependencies({ &cfg }); cfg.AddTest("ShouldFallbackUnknownRoutes").Dependencies({ &cfg }); - cfg.AddTest("ShouldHonourQUICSettings").Dependencies({ &cfg }); cfg.AddTest("ShouldInteropCurlHTTP1").Dependencies({ &cfg }); cfg.AddTest("ShouldInteropCurlHTTPS1").Dependencies({ &cfg }); cfg.AddTest("ShouldNotDropEarlyStreams").Dependencies({ &cfg }); diff --git a/tests/ShouldHonourQUICSettings/main.cpp b/tests/ShouldHonourQUICSettings/main.cpp deleted file mode 100644 index 85cc4f3..0000000 --- a/tests/ShouldHonourQUICSettings/main.cpp +++ /dev/null @@ -1,130 +0,0 @@ -//SPDX-License-Identifier: LGPL-3.0-only -//SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts® - -// QUICSettings has to actually reach the msquic Configuration. The knob with -// observable behaviour is peerUnidiStreamCount: it caps how many streams the -// peer may open toward us, so the listener here opens more unidi streams than -// the previously hardcoded limit of 16 allowed. Against a build that ignores -// the setting the extra streams never arrive and this test times out. - -import Crafter.Network; -import Crafter.Thread; -import std; -using namespace Crafter; - -namespace { - int g_failures = 0; - void Check(bool cond, std::string_view what) { - std::println(" {}: {}", cond ? "ok" : "FAIL", what); - if (!cond) ++g_failures; - } - - constexpr std::uint16_t port = 9187; - constexpr std::string_view alpn = "f3d/test-settings"; - - // Above the old hardcoded cap of 16, below the 32 we grant below. - constexpr std::size_t streamCount = 24; -} - -int main() { - ThreadPool::Start(); - - std::mutex mtx; - std::condition_variable cv; - std::set received; - - QUICServerCredentials serverCreds; - serverCreds.selfSigned = true; - - // The server-side streams must outlive their SendSync, so park them here - // rather than letting them destruct at the end of each loop iteration. - std::mutex serverMtx; - std::vector serverStreams; - serverStreams.reserve(streamCount); - std::string openError; - - ListenerQUIC listener(port, std::string(alpn), serverCreds, [&](ClientQUIC* peer) { - std::vector opened; - opened.reserve(streamCount); - try { - // Open every stream *before* sending on any of them, so all of - // them are live at once and the peer's unidi credit has to cover - // the whole count simultaneously. Opening and finishing one at a - // time recycles credit as each stream closes, which passes even - // at the old cap of 16 and would test nothing. - for (std::size_t i = 0; i < streamCount; ++i) { - opened.push_back(peer->OpenStream(/*unidirectional=*/true)); - } - // Leave the send side open — a finished stream returns its credit. - for (std::size_t i = 0; i < streamCount; ++i) { - std::string message = std::format("{}", i); - opened[i].SendSync(message.data(), static_cast(message.size()), - /*finish=*/false); - } - } catch (std::exception& e) { - std::lock_guard lock(serverMtx); - openError = e.what(); - return; - } - std::lock_guard lock(serverMtx); - serverStreams = std::move(opened); - }); - listener.ListenAsyncAsync(); - - try { - QUICClientCredentials clientCreds; - clientCreds.insecureNoServerValidation = true; - // The knob under test. Without it this defaults to 16 and the - // listener stalls partway through its 24 streams. - clientCreds.settings.peerUnidiStreamCount = 32; - - ClientQUIC client(std::string("localhost"), port, std::string(alpn), clientCreds); - - // Streams opened before this callback lands are queued and drained - // into it, so registering after connect is safe. - std::mutex clientMtx; - std::vector clientStreams; - clientStreams.reserve(streamCount); - client.OnStream([&](QUICStream stream) { - try { - // One chunk only: the peer deliberately leaves the send - // side open so the stream keeps holding its credit. - std::vector got = stream.RecieveSync(); - int index = std::stoi(std::string(got.begin(), got.end())); - { - std::lock_guard lock(mtx); - received.insert(index); - } - cv.notify_all(); - std::lock_guard lock(clientMtx); - clientStreams.push_back(std::move(stream)); - } catch (...) { - // A stream that fails simply never counts toward the total. - } - }); - - std::size_t got = 0; - { - std::unique_lock lock(mtx); - cv.wait_for(lock, std::chrono::seconds(20), - [&] { return received.size() == streamCount; }); - got = received.size(); - } - - { - std::lock_guard lock(serverMtx); - Check(openError.empty(), - openError.empty() ? "listener opened every stream" - : std::format("listener failed to open: {}", openError)); - } - Check(got == streamCount, - std::format("received {} of {} peer-initiated unidi streams", got, streamCount)); - - // msquic's RegistrationClose blocks on connections the listener still - // owns, so skip the static-dtor path the way the other QUIC tests do. - std::_Exit(g_failures ? 1 : 0); - } catch (std::exception& e) { - std::println("{}", e.what()); - return 1; - } -}