From 33ac318f5844b8b4519d6d4f882ce4a24a019585 Mon Sep 17 00:00:00 2001 From: Efe Karasakal Date: Wed, 28 Oct 2026 23:05:42 +0100 Subject: [PATCH 1/2] quic: handle application-specific no-error codes Signed-off-by: Efe Karasakal --- doc/api/quic.md | 25 ++-- lib/internal/quic/quic.js | 89 ++++--------- lib/internal/quic/state.js | 14 ++ src/quic/application.h | 6 + src/quic/data.cc | 13 +- src/quic/data.h | 10 +- src/quic/http3.cc | 19 +++ src/quic/session.cc | 43 +++--- src/quic/session.h | 2 + src/quic/streams.cc | 12 +- src/quic/streams.h | 2 +- test/parallel/test-quic-h3-error-codes.mjs | 44 +++++++ ...est-quic-internal-endpoint-stats-state.mjs | 1 + ...test-quic-session-application-no-error.mjs | 122 ++++++++++++++++++ .../test-quic-stream-truncated-reads.mjs | 8 +- 15 files changed, 284 insertions(+), 126 deletions(-) create mode 100644 test/parallel/test-quic-session-application-no-error.mjs diff --git a/doc/api/quic.md b/doc/api/quic.md index 6df3f62da170..174249f39412 100644 --- a/doc/api/quic.md +++ b/doc/api/quic.md @@ -1959,10 +1959,10 @@ added: v23.8.0 * Type: {Promise} A promise that is fulfilled when the stream is fully closed. It resolves -when the stream closes cleanly (including idle timeout). It rejects with +when the stream closes without a QUIC error, including idle timeout and a +`RESET_STREAM` whose application code represents no error. It rejects with an `ERR_QUIC_APPLICATION_ERROR` or `ERR_QUIC_TRANSPORT_ERROR` when the -stream is closed due to a QUIC error (e.g., stream reset by the peer, -CONNECTION\_CLOSE with a non-zero error code). +stream is closed due to a QUIC error. ### `stream.destroy([error[, options]])` @@ -2191,7 +2191,8 @@ The callback to invoke when the peer aborts a direction of the stream by sending a `RESET_STREAM` frame (the peer abandons their writable side, so no further data will arrive on our readable side). -The callback receives a Node.js error whose `errorCode` (`bigint`) +The callback receives `undefined` when the application error code represents +no error. Otherwise, it receives a Node.js error whose `errorCode` (`bigint`) property carries the application error code from the wire frame. The stream is **not** automatically destroyed when this callback fires — @@ -2214,7 +2215,8 @@ The callback to invoke when the peer aborts a direction of the stream by sending a `STOP_SENDING` frame (the peer asks us to stop writing on our writable side). -The callback receives a Node.js error whose `errorCode` (`bigint`) +The callback receives `undefined` when the application error code represents +no error. Otherwise, it receives a Node.js error whose `errorCode` (`bigint`) property carries the application error code from the wire frame. Read/write. ### `stream.headers` @@ -3491,10 +3493,11 @@ selects how the stream's async iterator reports this: * `'ignore'` - The truncation itself is ignored: only a stream or connection error is reported, and any clean abort/cancellation or similar simply ends - the stream. A non-zero peer reset, non-zero local stop-sending or connection - error still fails, but a truncation with no error at all (an idle timeout, - a graceful close, or a plain `stopSending()`) ends the read cleanly with the - data received. This matches `stream.closed`, which rejects only on an error. + the stream. A peer reset whose application code represents an error, a + non-zero local stop-sending, or a connection error still fails. A truncation + with no error at all (an idle timeout, a graceful close, or a plain + `stopSending()`) ends the read cleanly with the data received. This matches + `stream.closed`, which rejects only on an error. #### `sessionOptions.verifyPeer` (client only) @@ -4091,7 +4094,7 @@ added: v23.8.0 --> * `this` {quic.QuicStream} -* `error` {any} +* `error` {Error|undefined} ### Callback: `OnHeadersCallback` @@ -4785,7 +4788,7 @@ added: * `stream` {quic.QuicStream} * `session` {quic.QuicSession} -* `error` {any} The QUIC error associated with the reset. +* `error` {Error|undefined} The QUIC error associated with the reset. Published when a stream receives a RESET\_STREAM frame from the peer, indicating the peer has aborted its sending direction. This is a key signal diff --git a/lib/internal/quic/quic.js b/lib/internal/quic/quic.js index 6603bd452899..a92720619c00 100644 --- a/lib/internal/quic/quic.js +++ b/lib/internal/quic/quic.js @@ -699,7 +699,7 @@ const endpointRegistry = new SafeSet(); /** * @callback OnStreamErrorCallback * @this {QuicStream} - * @param {any} error + * @param {Error|undefined} error * @returns {void} */ @@ -767,15 +767,14 @@ setCallbacks({ /** * Called when the underlying session C++ handle is closed either normally * or with an error. - * @param {number} errorType - * @param {number} code - * @param {string} [reason] - * @param {string} [errorName] Decoded TLS alert name when `code` is a - * CRYPTO_ERROR; otherwise undefined. + * @param {Array} [error] Raw `[type, code, reason, errorName]` data. */ - onSessionClose(errorType, code, reason, errorName) { - debug('session close callback', errorType, code, reason, errorName); - this[kOwner][kFinishClose](errorType, code, reason, errorName); + onSessionClose(error) { + if (error !== undefined) { + error = convertQuicError(error); + } + debug('session close callback', error); + this[kOwner][kFinishClose](error); }, /** @@ -967,20 +966,16 @@ setCallbacks({ }, onStreamClose(error) { - // Called when the stream C++ handle has been closed. The error is - // either undefined (clean close) or a raw array [type, code, reason] - // from QuicError::ToV8Value. Convert to a proper Node.js Error. + // Called when the stream C++ handle has been closed. The error is either + // undefined (clean close) or a raw [type, code, reason, errorName] array + // from the session-aware native error conversion. if (!this[kOwner]) return; if (error !== undefined) { error = convertQuicError(error); } else if (!this[kOwner].destroyed) { - // The stream is closing cleanly, but it may have been reset by the - // peer (ReceiveStreamReset) or locally (resetStream). The C++ side - // records the reset code in state.resetCode. If set, surface the - // reset as the close error so stream.closed rejects -- the reset - // was an abnormal termination even if the session closed cleanly. - const resetCode = getQuicStreamState(this[kOwner]).resetCode; - if (resetCode !== undefined && resetCode > 0n) { + const state = getQuicStreamState(this[kOwner]); + if (state.resetCodeIsError) { + const resetCode = state.resetCode; error = makeQuicError( 'ERR_QUIC_APPLICATION_ERROR', 'QUIC application error', @@ -1732,9 +1727,9 @@ class QuicStream { errorCode: inner.stopSendingCode }); } - // Non-zero reset is always an error: + // A reset carrying an application error always fails the read. const peerResetCode = inner.state.resetCode; - if (peerResetCode > 0n) { + if (inner.state.resetCodeIsError) { throw new QuicError( `The QUIC stream was reset by the peer with error code ${peerResetCode}`, { __proto__: null, @@ -3914,21 +3909,18 @@ class QuicSession { } /** - * @param {number} errorType - * @param {number} code - * @param {string} [reason] + * @param {Error} [error] */ - [kFinishClose](errorType, code, reason, errorName) { - // If code is zero, then we closed without an error. Yay! We can destroy + [kFinishClose](error) { + // If the error is undefined, then we closed without an error. Yay! We can destroy // safely without specifying an error. - if (code === 0n) { + if (error === undefined) { debug('finishing closing the session with no error'); this.destroy(); return; } - debug('finishing closing the session with an error', - errorType, code, reason, errorName); + debug('finishing closing the session with an error', error); // If the local side initiated this close with an error code (via // close({ code })), this is an intentional shutdown; not an error. @@ -3938,44 +3930,7 @@ class QuicSession { return; } - // Otherwise, errorType indicates the type of error that occurred, code indicates - // the specific error, and reason is an optional string describing the error. - // code !== 0n here (the early return above handles code === 0n). - // The errorType values map to ngtcp2_ccerr_type: - // 0 = NGTCP2_CCERR_TYPE_TRANSPORT - // 1 = NGTCP2_CCERR_TYPE_APPLICATION - // 2 = NGTCP2_CCERR_TYPE_VERSION_NEGOTIATION - // 3 = NGTCP2_CCERR_TYPE_IDLE_CLOSE - // 4 = NGTCP2_CCERR_TYPE_DROP_CONN - // 5 = NGTCP2_CCERR_TYPE_RETRY - // The DROP_CONN/RETRY cases are typically intercepted before reaching - // here (DROP_CONN tears the connection down without notifying us, RETRY - // is server-only). The default branch is a safety net so any - // unexpected value still completes the close path - without it the - // session would leak with `closed` hanging forever. - switch (errorType) { - case 0: /* Transport Error */ - this.destroy(makeQuicError('ERR_QUIC_TRANSPORT_ERROR', - 'QUIC transport error', - 'transport', code, reason, errorName)); - break; - case 1: /* Application Error */ - this.destroy(makeQuicError('ERR_QUIC_APPLICATION_ERROR', - 'QUIC application error', - 'application', code, reason, errorName)); - break; - case 2: /* Version Negotiation Error */ - this.destroy(new ERR_QUIC_VERSION_NEGOTIATION_ERROR()); - break; - case 3: /* Idle close */ - this.destroy(); - break; - default: - this.destroy(makeQuicError('ERR_QUIC_TRANSPORT_ERROR', - 'QUIC transport error', - 'transport', code, reason, errorName)); - break; - } + this.destroy(error); } [kKeylog](line) { diff --git a/lib/internal/quic/state.js b/lib/internal/quic/state.js index 0a47e2e7adc8..0617987471f0 100644 --- a/lib/internal/quic/state.js +++ b/lib/internal/quic/state.js @@ -112,6 +112,7 @@ const { IDX_STATE_STREAM_WRITE_DESIRED_SIZE, IDX_STATE_STREAM_BUDGET, IDX_STATE_STREAM_RESET_CODE, + IDX_STATE_STREAM_RESET_CODE_IS_ERROR, IDX_STATE_STREAM_SIZE, } = internalBinding('quic'); @@ -153,6 +154,7 @@ assert(IDX_STATE_STREAM_WANTS_RESET !== undefined); assert(IDX_STATE_STREAM_WANTS_STOP_SENDING !== undefined); assert(IDX_STATE_STREAM_WRITE_DESIRED_SIZE !== undefined); assert(IDX_STATE_STREAM_RESET_CODE !== undefined); +assert(IDX_STATE_STREAM_RESET_CODE_IS_ERROR !== undefined); const kEmptyObject = { __proto__: null }; @@ -873,6 +875,14 @@ class QuicStreamState { handle, this.#offset + IDX_STATE_STREAM_RESET_CODE, kIsLittleEndian); } + /** @type {boolean} */ + get resetCodeIsError() { + const handle = this.#handle; + if (handle === undefined) return undefined; + return DataViewPrototypeGetUint8( + handle, this.#offset + IDX_STATE_STREAM_RESET_CODE_IS_ERROR) !== 0; + } + /** @type {bigint} */ get writeDesiredSize() { const handle = this.#handle; @@ -924,6 +934,7 @@ class QuicStreamState { wantsStopSending, early, resetCode, + resetCodeIsError, writeDesiredSize, budget, } = this; @@ -943,6 +954,7 @@ class QuicStreamState { wantsStopSending, early, resetCode: `${resetCode}`, + resetCodeIsError, writeDesiredSize, budget, }; @@ -979,6 +991,7 @@ class QuicStreamState { wantsStopSending, early, resetCode, + resetCodeIsError, writeDesiredSize, budget, } = this; @@ -998,6 +1011,7 @@ class QuicStreamState { wantsStopSending, early, resetCode, + resetCodeIsError, writeDesiredSize, budget, }, opts)}`; diff --git a/src/quic/application.h b/src/quic/application.h index 4fd748763742..504862e512fb 100644 --- a/src/quic/application.h +++ b/src/quic/application.h @@ -60,6 +60,12 @@ class Session::Application : public MemoryRetainer { // and is not necessarily 0. virtual error_code GetNoErrorCode() const = 0; + // Returns whether a received application error code means "no error". + // Applications may override this when unknown codes have special semantics. + virtual bool IsNoErrorCode(error_code code) const { + return code == GetNoErrorCode(); + } + // The "internal error code" is the application-level error code used // to signal a non-specific failure when no more specific code has // been provided by the caller. For example, `writer.fail(reason)` on diff --git a/src/quic/data.cc b/src/quic/data.cc index 36867390b0f3..2eab0f1f340c 100644 --- a/src/quic/data.cc +++ b/src/quic/data.cc @@ -439,17 +439,6 @@ const char* QuicError::name() const { } MaybeLocal QuicError::ToV8Value(Environment* env) const { - if ((type() == Type::TRANSPORT && code() == NGTCP2_NO_ERROR) || - (type() == Type::APPLICATION && - (code() == 0 || code() == NGHTTP3_H3_NO_ERROR)) || - type() == Type::IDLE_CLOSE) { - // Application code 0 is the default no-error code for raw QUIC - // applications (DefaultApplication::GetNoErrorCode() returns 0). - // NGHTTP3_H3_NO_ERROR (0x100) is the HTTP/3 no-error code. - // Idle close is always clean — the session timed out normally. - return Undefined(env->isolate()); - } - Local type_str; if (!node::ToV8Value(env->context(), TypeName(type())).ToLocal(&type_str)) { return {}; @@ -464,7 +453,7 @@ MaybeLocal QuicError::ToV8Value(Environment* env) const { // Note that per the QUIC specification, the reason, if present, is // expected to be UTF-8 encoded. The spec uses the term "SHOULD" here, - // which means that is is entirely possible that some QUIC implementation + // which means that is entirely possible that some QUIC implementation // could choose a different encoding, in which case the conversion here // will produce garbage. That's ok though, we're going to use the default // assumption that the impl is following the guidelines. diff --git a/src/quic/data.h b/src/quic/data.h index 79d7f178a86b..fbd939643988 100644 --- a/src/quic/data.h +++ b/src/quic/data.h @@ -284,11 +284,11 @@ class QuicError final : public MemoryRetainer { std::string ToString() const; - // Returns an array containing [type, code, reason], where type is a string - // representation of the error type, code is a bigint representation of the - // error code, and reason is a string representation of the error reason, or - // undefined if the reason is the empty string. This is expected to be used - // to construct a JS error object from the information in JS. + // Returns an array containing [type, code, reason, name], where type is a + // string representation of the error type, code is a bigint representation + // of the error code, and reason and name are strings or undefined. This is a + // lossless representation; whether the code means "no error" depends on the + // negotiated application and is determined by Session. v8::MaybeLocal ToV8Value(Environment* env) const; inline v8::MaybeLocal ToV8Value(Realm* realm) const { return ToV8Value(realm->env()); diff --git a/src/quic/http3.cc b/src/quic/http3.cc index ad0d6e383fe3..b52429b8a95d 100644 --- a/src/quic/http3.cc +++ b/src/quic/http3.cc @@ -37,6 +37,9 @@ constexpr size_t kSessionTicketAppDataHeaderSize = 6; // type + version + crc constexpr size_t kSessionTicketAppDataPayloadSize = kSessionTicketAppDataSize - kSessionTicketAppDataHeaderSize; +// RFC 9297 defines H3_DATAGRAM_ERROR, but nghttp3 does not expose it. +constexpr error_code kH3DatagramError = 0x33; + inline void WriteBE32(uint8_t* buf, uint32_t val) { buf[0] = static_cast((val >> 24) & 0xff); buf[1] = static_cast((val >> 16) & 0xff); @@ -186,6 +189,22 @@ class Http3ApplicationImpl final : public Session::Application { error_code GetNoErrorCode() const override { return NGHTTP3_H3_NO_ERROR; } + bool IsNoErrorCode(error_code code) const override { + if (code == NGHTTP3_H3_NO_ERROR) return true; + if (code == kH3DatagramError) return false; + + switch (code) { +#define V(name) case NGHTTP3_##name: + HTTP3_APPLICATION_ERRORS(V) +#undef V + return false; + default: + // RFC 9114 Section 8 requires unknown HTTP/3 error codes to be + // treated as equivalent to H3_NO_ERROR. + return true; + } + } + // HTTP/3 defines H3_INTERNAL_ERROR (0x102) for non-specific failures // initiated by the implementation; this is the right code to send // on RESET_STREAM when a stream is being aborted without an diff --git a/src/quic/session.cc b/src/quic/session.cc index ba672a3bee48..371be2f25458 100644 --- a/src/quic/session.cc +++ b/src/quic/session.cc @@ -2770,6 +2770,25 @@ void Session::SetLastError(QuicError&& error) { impl_->last_error_ = std::move(error); } +bool Session::IsNoError(const QuicError& error) const { + switch (error.type()) { + case QuicError::Type::TRANSPORT: + return error.code() == NGTCP2_NO_ERROR; + case QuicError::Type::APPLICATION: + return has_application() && + application().IsNoErrorCode(error.code()); + case QuicError::Type::IDLE_CLOSE: + return true; + default: + return false; + } +} + +MaybeLocal Session::ToV8ErrorValue(const QuicError& error) const { + if (IsNoError(error)) return Undefined(env()->isolate()); + return error.ToV8Value(env()); +} + bool Session::Receive(const uint8_t* data, size_t len, const SocketAddress& local_address, @@ -4011,27 +4030,9 @@ void Session::EmitClose(const QuicError& error) { CallbackScope cb_scope(this); - Local argv[] = { - Integer::New(env()->isolate(), static_cast(error.type())), - BigInt::NewFromUnsigned(env()->isolate(), error.code()), - Undefined(env()->isolate()), - Undefined(env()->isolate()), - }; - if (error.reason().length() > 0 && - !ToV8Value(env()->context(), error.reason()).ToLocal(&argv[2])) { - return; - } - - // Attach a human-readable name for known wire codes (RFC 9000 sec. 20.1 - // names and OpenSSL TLS alert descriptions for CRYPTO_ERROR). Unknown - // codes leave the slot as undefined. See QuicError::name() for the - // matching path on stream-level errors. - if (const char* n = error.name()) { - argv[3] = BindingData::Get(env()).error_name_string(n); - } - - MakeCallback( - BindingData::Get(env()).session_close_callback(), arraysize(argv), argv); + Local err; + if (!ToV8ErrorValue(error).ToLocal(&err)) return; + MakeCallback(BindingData::Get(env()).session_close_callback(), 1, &err); // Importantly, the session instance itself should have been destroyed! CHECK(is_destroyed()); diff --git a/src/quic/session.h b/src/quic/session.h index 9834aa7ec130..6676e6926ba9 100644 --- a/src/quic/session.h +++ b/src/quic/session.h @@ -546,6 +546,8 @@ class Session final : public AsyncWrap, private SessionTicket::AppData::Source { void ExtendStreamOffset(stream_id id, size_t amount); void ExtendOffset(size_t amount); void SetLastError(QuicError&& error); + bool IsNoError(const QuicError& error) const; + v8::MaybeLocal ToV8ErrorValue(const QuicError& error) const; uint64_t max_data_left() const; // Transport operations that protocol applications (e.g. HTTP/3) invoke on diff --git a/src/quic/streams.cc b/src/quic/streams.cc index 0a2af12d366a..4d19dd04ef64 100644 --- a/src/quic/streams.cc +++ b/src/quic/streams.cc @@ -50,6 +50,7 @@ namespace quic { V(WRITE_ENDED, write_ended, uint8_t) \ V(RESET, reset, uint8_t) \ V(RESET_CODE, reset_code, uint64_t) \ + V(RESET_CODE_IS_ERROR, reset_code_is_error, uint8_t) \ V(HAS_OUTBOUND, has_outbound, uint8_t) \ V(HAS_READER, has_reader, uint8_t) \ /* Set when the stream has a block event handler */ \ @@ -1741,8 +1742,8 @@ void Stream::ReceiveStopSending(QuicError error) { void Stream::ReceiveStreamReset(uint64_t final_size, QuicError error) { // Importantly, reset stream only impacts the inbound data flow. It has no - // impact on the outbound data flow. It is essentially a signal that the peer - // has abruptly terminated the writable end of their stream with an error. + // impact on the outbound data flow. It signals that the peer abruptly + // terminated the writable end of its stream with an application code. // Any data we have received up to this point remains in the queue waiting to // be read. Debug(this, @@ -1750,6 +1751,7 @@ void Stream::ReceiveStreamReset(uint64_t final_size, QuicError error) { final_size, error); state()->reset_code = error.code(); + state()->reset_code_is_error = !session().IsNoError(error); TruncateReadable(final_size); EmitReset(error); } @@ -1863,7 +1865,7 @@ void Stream::EmitClose(const QuicError& error) { if (!env()->can_call_into_js()) return; CallbackScope cb_scope(this); Local err; - if (!error.ToV8Value(env()).ToLocal(&err)) return; + if (!session().ToV8ErrorValue(error).ToLocal(&err)) return; MakeCallback(BindingData::Get(env()).stream_close_callback(), 1, &err); } @@ -1875,7 +1877,7 @@ void Stream::EmitReset(const QuicError& error) { } CallbackScope cb_scope(this); Local err; - if (!error.ToV8Value(env()).ToLocal(&err)) return; + if (!session().ToV8ErrorValue(error).ToLocal(&err)) return; MakeCallback(BindingData::Get(env()).stream_reset_callback(), 1, &err); } @@ -1886,7 +1888,7 @@ void Stream::EmitStopSending(const QuicError& error) { } CallbackScope cb_scope(this); Local err; - if (!error.ToV8Value(env()).ToLocal(&err)) return; + if (!session().ToV8ErrorValue(error).ToLocal(&err)) return; MakeCallback(BindingData::Get(env()).stream_stop_sending_callback(), 1, &err); } diff --git a/src/quic/streams.h b/src/quic/streams.h index db6caaec088b..0264feccb482 100644 --- a/src/quic/streams.h +++ b/src/quic/streams.h @@ -356,7 +356,7 @@ class Stream final : public AsyncWrap, // stream. bool fin = false; // Indicates that this chunk of data was received in a 0RTT packet before - // the TLS handshake completed, suggesting that is is not as secure and + // the TLS handshake completed, suggesting that is not as secure and // could be replayed by an attacker. bool early = false; }; diff --git a/test/parallel/test-quic-h3-error-codes.mjs b/test/parallel/test-quic-h3-error-codes.mjs index 3a91a2e8f056..556dd6ba5bfe 100644 --- a/test/parallel/test-quic-h3-error-codes.mjs +++ b/test/parallel/test-quic-h3-error-codes.mjs @@ -120,3 +120,47 @@ const decoder = new TextDecoder(); await clientSession.close(); await serverEndpoint.close(); } + +// RESET_STREAM with H3_NO_ERROR is a clean application-level reset. It does +// not reject stream.closed, and truncatedReads: 'ignore' accepts the missing +// FIN without turning the nonzero code back into an error. +{ + const H3_NO_ERROR = 0x100n; + const reset = Promise.withResolvers(); + + const serverEndpoint = await listen(mustCall(() => {}), { + sni: { '*': { keys: [key], certs: [cert] } }, + onheaders: mustCall(function() { + this.resetStream(H3_NO_ERROR); + }), + }); + + const clientSession = await connect(serverEndpoint.address, { + servername: 'localhost', + verifyPeer: 'manual', + truncatedReads: 'ignore', + }); + await clientSession.opened; + + const stream = await clientSession.createBidirectionalStream({ + headers: { + ':method': 'GET', + ':path': '/', + ':scheme': 'https', + ':authority': 'localhost', + }, + }); + stream.onreset = mustCall((error) => { + assert.strictEqual(error, undefined); + reset.resolve(); + }); + stream.writer.endSync(); + + const body = await bytes(stream); + assert.strictEqual(body.byteLength, 0); + await reset.promise; + await stream.closed; + + await clientSession.close(); + await serverEndpoint.close(); +} diff --git a/test/parallel/test-quic-internal-endpoint-stats-state.mjs b/test/parallel/test-quic-internal-endpoint-stats-state.mjs index 2f4ce6ccc141..bc44619cdfa8 100644 --- a/test/parallel/test-quic-internal-endpoint-stats-state.mjs +++ b/test/parallel/test-quic-internal-endpoint-stats-state.mjs @@ -153,6 +153,7 @@ assert.strictEqual(streamState.finReceived, false); assert.strictEqual(streamState.readEnded, false); assert.strictEqual(streamState.writeEnded, false); assert.strictEqual(streamState.reset, false); +assert.strictEqual(streamState.resetCodeIsError, false); assert.strictEqual(streamState.hasReader, false); assert.strictEqual(streamState.wantsBlock, false); assert.strictEqual(streamState.wantsReset, false); diff --git a/test/parallel/test-quic-session-application-no-error.mjs b/test/parallel/test-quic-session-application-no-error.mjs new file mode 100644 index 000000000000..c72cfdee471d --- /dev/null +++ b/test/parallel/test-quic-session-application-no-error.mjs @@ -0,0 +1,122 @@ +// Flags: --experimental-quic --no-warnings + +// Application-level no-error codes are interpreted in the context of the +// negotiated application protocol. + +import { hasQuic, skip, mustCall, mustNotCall } from '../common/index.mjs'; +import assert from 'node:assert'; +import * as fixtures from '../common/fixtures.mjs'; +import { createPrivateKey } from 'node:crypto'; + +if (!hasQuic) { + skip('QUIC is not enabled'); +} + +const { listen, connect } = await import('node:quic'); +const key = createPrivateKey(fixtures.readKey('agent1-key.pem')); +const cert = fixtures.readKey('agent1-cert.pem'); + +// H3_NO_ERROR (0x100) and unknown HTTP/3 error codes are clean closes. +// 0x21 is one of the reserved codes defined by RFC 9114 Section 8.1. +for (const code of [0x100n, 0x21n]) { + const serverSessionReady = Promise.withResolvers(); + const endpoint = await listen(mustCall((session) => { + serverSessionReady.resolve(session); + }), { + alpn: ['h3'], + sni: { '*': { keys: [key], certs: [cert] } }, + }); + + const clientSession = await connect(endpoint.address, { + alpn: 'h3', + servername: 'localhost', + verifyPeer: 'manual', + reuseEndpoint: false, + }); + const serverSession = await serverSessionReady.promise; + serverSession.onerror = mustNotCall(); + await Promise.all([ + clientSession.opened, + serverSession.opened, + ]); + + await clientSession.close({ + code, + type: 'application', + reason: 'normal HTTP/3 close', + }); + await serverSession.closed; + await endpoint.close(); +} + +// H3_DATAGRAM_ERROR (0x33) is a known HTTP/3 application error. +{ + const serverSessionReady = Promise.withResolvers(); + const endpoint = await listen(mustCall((session) => { + serverSessionReady.resolve(session); + }), { + alpn: ['h3'], + sni: { '*': { keys: [key], certs: [cert] } }, + }); + + const clientSession = await connect(endpoint.address, { + alpn: 'h3', + servername: 'localhost', + verifyPeer: 'manual', + reuseEndpoint: false, + }); + const serverSession = await serverSessionReady.promise; + await Promise.all([ + clientSession.opened, + serverSession.opened, + ]); + + const closed = assert.rejects(serverSession.closed, { + code: 'ERR_QUIC_APPLICATION_ERROR', + errorCode: 0x33n, + reason: 'HTTP datagram failure', + }); + await clientSession.close({ + code: 0x33n, + type: 'application', + reason: 'HTTP datagram failure', + }); + await closed; + await endpoint.close(); +} + +// The same application code is not implicitly clean for another ALPN. +{ + const serverSessionReady = Promise.withResolvers(); + const endpoint = await listen(mustCall((session) => { + serverSessionReady.resolve(session); + }), { + alpn: ['quic-test'], + sni: { '*': { keys: [key], certs: [cert] } }, + }); + + const clientSession = await connect(endpoint.address, { + alpn: 'quic-test', + servername: 'localhost', + verifyPeer: 'manual', + reuseEndpoint: false, + }); + const serverSession = await serverSessionReady.promise; + await Promise.all([ + clientSession.opened, + serverSession.opened, + ]); + + const closed = assert.rejects(serverSession.closed, { + code: 'ERR_QUIC_APPLICATION_ERROR', + errorCode: 0x100n, + reason: 'application failure', + }); + await clientSession.close({ + code: 0x100n, + type: 'application', + reason: 'application failure', + }); + await closed; + await endpoint.close(); +} diff --git a/test/parallel/test-quic-stream-truncated-reads.mjs b/test/parallel/test-quic-stream-truncated-reads.mjs index e5c381a8d679..0fed12dbb6fc 100644 --- a/test/parallel/test-quic-stream-truncated-reads.mjs +++ b/test/parallel/test-quic-stream-truncated-reads.mjs @@ -3,8 +3,8 @@ // Test: a stream read that ends without a FIN is a truncation. What the read // reports is decided in this order: // -// 1. the peer reset the stream with a non-zero code: ERR_QUIC_STREAM_RESET -// carrying that code, under either policy; +// 1. the peer reset the stream with an application error code: +// ERR_QUIC_STREAM_RESET carrying that code, under either policy; // 2. the stream was already being torn down and its closed promise rejected: // that same error, under either policy; // 3. otherwise the truncation carries no error of its own and the @@ -53,8 +53,8 @@ const stall = (stream) => { stream.setBody(stallingBody(1000)); }; assert.strictEqual(threw, undefined); } -// A peer reset with a nonzero code is rule 1: an error under either policy, -// carrying the peer's code. It also rejects the closed promise on both sides. +// A peer reset whose code represents an application error is rule 1 under +// either policy, carrying the peer's code. It also rejects closed on both sides. { const serverClosed = Promise.withResolvers(); const { received, threw, closedError } = await readStream(async (stream) => { From ed96feada5cb0a001cdd9594047dc2f994fd66b0 Mon Sep 17 00:00:00 2001 From: Efe Karasakal Date: Wed, 28 Oct 2026 23:18:35 +0100 Subject: [PATCH 2/2] quic: fmt Signed-off-by: Efe Karasakal --- src/quic/http3.cc | 2 +- src/quic/session.cc | 3 +-- test/parallel/test-quic-h3-error-codes.mjs | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/quic/http3.cc b/src/quic/http3.cc index b52429b8a95d..0f9afc9a9cde 100644 --- a/src/quic/http3.cc +++ b/src/quic/http3.cc @@ -197,7 +197,7 @@ class Http3ApplicationImpl final : public Session::Application { #define V(name) case NGHTTP3_##name: HTTP3_APPLICATION_ERRORS(V) #undef V - return false; + return false; default: // RFC 9114 Section 8 requires unknown HTTP/3 error codes to be // treated as equivalent to H3_NO_ERROR. diff --git a/src/quic/session.cc b/src/quic/session.cc index 371be2f25458..5590125f3a00 100644 --- a/src/quic/session.cc +++ b/src/quic/session.cc @@ -2775,8 +2775,7 @@ bool Session::IsNoError(const QuicError& error) const { case QuicError::Type::TRANSPORT: return error.code() == NGTCP2_NO_ERROR; case QuicError::Type::APPLICATION: - return has_application() && - application().IsNoErrorCode(error.code()); + return has_application() && application().IsNoErrorCode(error.code()); case QuicError::Type::IDLE_CLOSE: return true; default: diff --git a/test/parallel/test-quic-h3-error-codes.mjs b/test/parallel/test-quic-h3-error-codes.mjs index 556dd6ba5bfe..167dbe772264 100644 --- a/test/parallel/test-quic-h3-error-codes.mjs +++ b/test/parallel/test-quic-h3-error-codes.mjs @@ -128,7 +128,7 @@ const decoder = new TextDecoder(); const H3_NO_ERROR = 0x100n; const reset = Promise.withResolvers(); - const serverEndpoint = await listen(mustCall(() => {}), { + const serverEndpoint = await listen(mustCall(), { sni: { '*': { keys: [key], certs: [cert] } }, onheaders: mustCall(function() { this.resetStream(H3_NO_ERROR);