diff --git a/implementations/Crafter.Network-ClientQUIC.cpp b/implementations/Crafter.Network-ClientQUIC.cpp index 323a45b..b626369 100644 --- a/implementations/Crafter.Network-ClientQUIC.cpp +++ b/implementations/Crafter.Network-ClientQUIC.cpp @@ -460,16 +460,18 @@ static HQUIC OpenClientConfiguration(const std::string& alpn, const QUICClientCr QUIC_SETTINGS settings{}; settings.IsSet.IdleTimeoutMs = 1; - settings.IdleTimeoutMs = 120'000; + settings.IdleTimeoutMs = creds.settings.idleTimeoutMs; + settings.IsSet.HandshakeIdleTimeoutMs = 1; + settings.HandshakeIdleTimeoutMs = creds.settings.handshakeIdleTimeoutMs; // 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 = 10'000; + settings.KeepAliveIntervalMs = creds.settings.keepAliveIntervalMs; settings.IsSet.DatagramReceiveEnabled = 1; - settings.DatagramReceiveEnabled = 1; + settings.DatagramReceiveEnabled = creds.settings.datagramReceiveEnabled ? 1 : 0; // 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 @@ -477,9 +479,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 = 16; + settings.PeerUnidiStreamCount = creds.settings.peerUnidiStreamCount; settings.IsSet.PeerBidiStreamCount = 1; - settings.PeerBidiStreamCount = 16; + settings.PeerBidiStreamCount = creds.settings.peerBidiStreamCount; 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 1b446c9..e3c43fe 100644 --- a/implementations/Crafter.Network-ListenerQUIC.cpp +++ b/implementations/Crafter.Network-ListenerQUIC.cpp @@ -220,13 +220,15 @@ static HQUIC OpenServerConfiguration(const std::string& alpn, QUIC_SETTINGS settings{}; settings.IsSet.IdleTimeoutMs = 1; - settings.IdleTimeoutMs = 120'000; + settings.IdleTimeoutMs = creds.settings.idleTimeoutMs; + settings.IsSet.HandshakeIdleTimeoutMs = 1; + settings.HandshakeIdleTimeoutMs = creds.settings.handshakeIdleTimeoutMs; settings.IsSet.PeerBidiStreamCount = 1; - settings.PeerBidiStreamCount = 16; + settings.PeerBidiStreamCount = creds.settings.peerBidiStreamCount; settings.IsSet.PeerUnidiStreamCount = 1; - settings.PeerUnidiStreamCount = 16; + settings.PeerUnidiStreamCount = creds.settings.peerUnidiStreamCount; settings.IsSet.DatagramReceiveEnabled = 1; - settings.DatagramReceiveEnabled = 1; + settings.DatagramReceiveEnabled = creds.settings.datagramReceiveEnabled ? 1 : 0; 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 bf35cbd..9e835d7 100644 --- a/interfaces/Crafter.Network-ClientQUIC.cppm +++ b/interfaces/Crafter.Network-ClientQUIC.cppm @@ -21,12 +21,42 @@ 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. @@ -40,6 +70,7 @@ 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 59fd6ec..304a0d8 100644 --- a/project.cpp +++ b/project.cpp @@ -133,6 +133,7 @@ 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 new file mode 100644 index 0000000..85cc4f3 --- /dev/null +++ b/tests/ShouldHonourQUICSettings/main.cpp @@ -0,0 +1,130 @@ +//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; + } +}