From 563ddc9af4e7fca7bd145e917c48925bc5108fba Mon Sep 17 00:00:00 2001 From: Tim Perry Date: Fri, 11 Sep 2026 00:03:42 +0200 Subject: [PATCH] quic: fix two small bugs in HTTP/3 stream internals Signed-off-by: Tim Perry --- src/quic/application.cc | 5 --- src/quic/http3.cc | 13 +++---- src/quic/session.cc | 6 +++ ...quic-h3-uni-stream-limit-start-failure.mjs | 38 +++++++++++++++++++ 4 files changed, 49 insertions(+), 13 deletions(-) create mode 100644 test/parallel/test-quic-h3-uni-stream-limit-start-failure.mjs diff --git a/src/quic/application.cc b/src/quic/application.cc index 1e78b7931ce1..bf3c5b9f3ced 100644 --- a/src/quic/application.cc +++ b/src/quic/application.cc @@ -338,11 +338,6 @@ class DefaultApplication final : public Session::Application { } int GetStreamData(Session::StreamData* stream_data) override { - // Reset the state of stream_data before proceeding... - stream_data->id = -1; - stream_data->count = 0; - stream_data->fin = false; - stream_data->stream.reset(); Debug(&session(), "Default application getting stream data"); DCHECK_NOT_NULL(stream_data); // If the queue is empty, there aren't any streams with data yet diff --git a/src/quic/http3.cc b/src/quic/http3.cc index 43ead1b8164e..f99aa38ba33b 100644 --- a/src/quic/http3.cc +++ b/src/quic/http3.cc @@ -235,7 +235,6 @@ class Http3ApplicationImpl final : public Session::Application { bool Start() override { if (started_) return true; - started_ = true; Debug(&session(), "Starting HTTP/3 application."); const auto params = session().remote_transport_params(); @@ -269,7 +268,7 @@ class Http3ApplicationImpl final : public Session::Application { } Debug(&session(), "Creating and binding HTTP/3 control streams"); - bool ret = + started_ = session().OpenUnidirectionalStream(&control_stream_id_) && session().OpenUnidirectionalStream(&qpack_enc_stream_id_) && session().OpenUnidirectionalStream(&qpack_dec_stream_id_) && @@ -277,7 +276,7 @@ class Http3ApplicationImpl final : public Session::Application { nghttp3_conn_bind_qpack_streams( *this, qpack_enc_stream_id_, qpack_dec_stream_id_) == 0; - if (env()->enabled_debug_list()->enabled(DebugCategory::QUIC) && ret) { + if (env()->enabled_debug_list()->enabled(DebugCategory::QUIC) && started_) { Debug(&session(), "Created and bound control stream %" PRIi64, control_stream_id_); @@ -289,7 +288,7 @@ class Http3ApplicationImpl final : public Session::Application { qpack_dec_stream_id_); } - return ret; + return started_; } void BeginShutdown() override { @@ -658,18 +657,16 @@ class Http3ApplicationImpl final : public Session::Application { offsetof(ngtcp2_vec, base) == offsetof(nghttp3_vec, base) && offsetof(ngtcp2_vec, len) == offsetof(nghttp3_vec, len), "ngtcp2_vec and nghttp3_vec must have identical layout"); - data->count = kMaxVectorCount; - ssize_t ret = 0; Debug(&session(), "HTTP/3 application getting stream data"); if (conn_ && session().max_data_left()) { // nghttp3 reports fin through an int out-param; bridge it to the bool. int fin = 0; - ret = + ssize_t ret = nghttp3_conn_writev_stream(*this, &data->id, &fin, reinterpret_cast(data->data), - data->count); + kMaxVectorCount); // A negative return value indicates an error. if (ret < 0) { return static_cast(ret); diff --git a/src/quic/session.cc b/src/quic/session.cc index 1c159d763521..5429611af1e6 100644 --- a/src/quic/session.cc +++ b/src/quic/session.cc @@ -2015,6 +2015,12 @@ void Session::SendPendingData() { } // The stream_data is the next block of data from the application stream. + // It is reused across iterations, so we reset before it's populated: + stream_data.count = 0; + stream_data.id = -1; + stream_data.fin = false; + stream_data.stream.reset(); + if (application().GetStreamData(&stream_data) < 0) { Debug(this, "Application failed to get stream data"); SetLastError(QuicError::ForNgtcp2Error(NGTCP2_ERR_INTERNAL)); diff --git a/test/parallel/test-quic-h3-uni-stream-limit-start-failure.mjs b/test/parallel/test-quic-h3-uni-stream-limit-start-failure.mjs new file mode 100644 index 000000000000..b6599ab63487 --- /dev/null +++ b/test/parallel/test-quic-h3-uni-stream-limit-start-failure.mjs @@ -0,0 +1,38 @@ +// Flags: --experimental-quic --no-warnings + +// An HTTP/3 session must not cleanly fail if the peer advertises fewer than +// the 3 unidirectional streams that HTTP/3 needs for control and QPACK. + +import { hasQuic, skip, mustNotCall } from '../common/index.mjs'; +import assert from 'node:assert'; +import * as fixtures from '../common/fixtures.mjs'; + +if (!hasQuic) { + skip('QUIC is not enabled'); +} + +const { listen, connect } = await import('node:quic'); +const { createPrivateKey } = await import('node:crypto'); + +const key = createPrivateKey(fixtures.readKey('agent1-key.pem')); +const cert = fixtures.readKey('agent1-cert.pem'); + +const serverEndpoint = await listen(async (serverSession) => { + await serverSession.closed; +}, { + sni: { '*': { keys: [key], certs: [cert] } }, + // No uni streams allowed: + transportParams: { initialMaxStreamsUni: 0 }, + onheaders: mustNotCall(), +}); + +// Expect the client to cleanly fail - not crash the process +await assert.rejects(async () => { + const clientSession = await connect(serverEndpoint.address, { + servername: 'localhost', + verifyPeer: 'manual', + }); + await clientSession.opened; +}, { code: 'ERR_QUIC_TRANSPORT_ERROR' }); + +await serverEndpoint.close();