nodejs/node · #66650

quic: validate stream error codes

efekrskl · merged Oct 17, 20264 files · 94 + / 51 −
doc/api/quic.md10 + / 7 −
@@ -1976,9 +1976,10 @@ changes: * `error` {any} * `options` {Object}   * `code` {bigint|number} The application error code to include in the-    `RESET_STREAM` and `STOP_SENDING` frames sent to the peer. Numbers are-    coerced to `BigInt`. When omitted, the wire code is derived from `error`-    (see below).+    `RESET_STREAM` and `STOP_SENDING` frames sent to the peer. Must be a+    non-negative 62-bit unsigned varint+    (`0n <= code <= 2n ** 62n - 1n`). Numbers are coerced to `BigInt`. When+    omitted, the wire code is derived from `error` (see below).   * `reason` {string} An optional human-readable reason string. Accepted for     symmetry with [`session.close()`][] and [`session.destroy()`][], but     **not transmitted on the wire** — neither `RESET_STREAM` nor@@ -2058,8 +2059,9 @@ not perform this derivation: they send `code` as given. added: v23.8.0 --> -* `code` {number|bigint} The application error code to send to the peer.-  **Default:** `0n`.+* `code` {number|bigint} The application error code to send to the peer. Must+  be a non-negative 62-bit unsigned varint+  (`0n <= code <= 2n ** 62n - 1n`). **Default:** `0n`.  Tells the peer that this end will not send any more data on this stream, sending a `RESET_STREAM` frame carrying `code`. The readable side is left@@ -2078,8 +2080,9 @@ remote-initiated unidirectional stream, which has no writable side to abort. added: v23.8.0 --> -* `code` {number|bigint} The application error code to send to the peer.-  **Default:** `0n`.+* `code` {number|bigint} The application error code to send to the peer. Must+  be a non-negative 62-bit unsigned varint+  (`0n <= code <= 2n ** 62n - 1n`). **Default:** `0n`.  Asks the peer to stop sending data on this stream, sending a `STOP_SENDING` frame carrying `code`. The writable side is left open, so this end can
lib/internal/quic/quic.js27 + / 32 −
@@ -1033,6 +1033,23 @@ function assertPrivateSymbol(privateSymbol) { // maximum representable code is 2**62 - 1. const kMaxQuicErrorCode = (1n << 62n) - 1n; +function validateQuicErrorCode(code, name) {+  if (typeof code !== 'bigint' && typeof code !== 'number') {+    throw new ERR_INVALID_ARG_TYPE(name, ['bigint', 'number'], code);+  }+  if (typeof code === 'number' && !NumberIsInteger(code)) {+    throw new ERR_OUT_OF_RANGE(name, 'an integer', code);+  }+  const numericCode = BigInt(code);+  if (numericCode < 0n || numericCode > kMaxQuicErrorCode) {+    throw new ERR_OUT_OF_RANGE(+      name,+      `>= 0 and <= ${kMaxQuicErrorCode}`,+      code);+  }+  return numericCode;+}+ /**  * An Error subclass that carries an explicit numeric QUIC error code.  * Use this when destroying a stream or aborting an outbound writer to@@ -1087,18 +1104,11 @@ class QuicError extends Error {     if (errorCode === undefined) {       throw new ERR_MISSING_ARGS('options.errorCode');     }-    if (typeof errorCode !== 'bigint' && typeof errorCode !== 'number') {-      throw new ERR_INVALID_ARG_TYPE('options.errorCode',-                                     ['bigint', 'number'], errorCode);-    }+    const numericCode = validateQuicErrorCode(+      errorCode,+      'options.errorCode');     validateString(code, 'options.code');     validateOneOf(type, 'options.type', ['transport', 'application']);-    const numericCode = BigInt(errorCode);-    if (numericCode < 0n || numericCode > kMaxQuicErrorCode) {-      throw new ERR_OUT_OF_RANGE('options.errorCode',-                                 `>= 0 and <= ${kMaxQuicErrorCode}`,-                                 errorCode);-    }     super(message);     this.code = code;     this.#errorCode = numericCode;@@ -2081,22 +2091,17 @@ class QuicStream {     // promise). The caller may retry with valid options.     validateObject(options, 'options');     const { code: optionCode, reason } = options;-    if (optionCode !== undefined &&-        typeof optionCode !== 'bigint' &&-        typeof optionCode !== 'number') {-      throw new ERR_INVALID_ARG_TYPE('options.code',-                                     ['bigint', 'number'], optionCode);+    let abortCode;+    if (optionCode !== undefined) {+      abortCode = validateQuicErrorCode(optionCode, 'options.code');     }     if (reason !== undefined) {       validateString(reason, 'options.reason');     }     inner.destroying = true;     // Resolve the wire error code for any RESET_STREAM / STOP_SENDING     // frames emitted below.-    let abortCode;-    if (optionCode !== undefined) {-      abortCode = BigInt(optionCode);-    } else if (error !== undefined) {+    if (abortCode === undefined && error !== undefined) {       abortCode = QuicError.isQuicError(error) ?         error.errorCode :         getQuicSessionState(inner.session).internalErrorCode;@@ -2606,7 +2611,7 @@ class QuicStream {   stopSending(code = 0n) {     assertIsQuicStream(this);     if (this.destroyed) return;-    const abortCode = BigInt(code);+    const abortCode = validateQuicErrorCode(code, 'code');     this.#inner.stopSendingCode = abortCode;     this.#handle.stopSending(abortCode);   }@@ -2621,7 +2626,7 @@ class QuicStream {   resetStream(code = 0n) {     assertIsQuicStream(this);     if (this.destroyed) return;-    this.#handle.resetStream(BigInt(code));+    this.#handle.resetStream(validateQuicErrorCode(code, 'code'));   }    /**@@ -5482,17 +5487,7 @@ function validateCloseOptions(options) {   } = options;    if (code !== undefined) {-    if (typeof code !== 'bigint' && typeof code !== 'number') {-      throw new ERR_INVALID_ARG_TYPE('options.code',-                                     ['bigint', 'number'], code);-    }-    if (typeof code === 'number' && !NumberIsInteger(code)) {-      throw new ERR_OUT_OF_RANGE('options.code', 'an integer', code);-    }-    if (code < 0 || code > kMaxQuicErrorCode) {-      throw new ERR_OUT_OF_RANGE('options.code',-                                 `>= 0 and <= ${kMaxQuicErrorCode}`, code);-    }+    validateQuicErrorCode(code, 'options.code');   }   validateOneOf(type, 'options.type', ['transport', 'application']);   if (reason !== undefined) {
src/quic/streams.cc4 + / 12 −
@@ -432,12 +432,8 @@ struct Stream::Impl {   JS_METHOD(StopSending) {     Stream* stream;     ASSIGN_OR_RETURN_UNWRAP(&stream, args.This());-    error_code code = 0;-    CHECK_IMPLIES(!args[0]->IsUndefined(), args[0]->IsBigInt());-    if (!args[0]->IsUndefined()) {-      bool unused = false;  // not used but still necessary.-      code = args[0].As<BigInt>()->Uint64Value(&unused);-    }+    CHECK(args[0]->IsBigInt());+    error_code code = args[0].As<BigInt>()->Uint64Value();      stream->SendStopSending(code);   }@@ -449,12 +445,8 @@ struct Stream::Impl {   JS_METHOD(ResetStream) {     Stream* stream;     ASSIGN_OR_RETURN_UNWRAP(&stream, args.This());-    error_code code = 0;-    CHECK_IMPLIES(!args[0]->IsUndefined(), args[0]->IsBigInt());-    if (!args[0]->IsUndefined()) {-      bool lossless = false;  // not used but still necessary.-      code = args[0].As<BigInt>()->Uint64Value(&lossless);-    }+    CHECK(args[0]->IsBigInt());+    error_code code = args[0].As<BigInt>()->Uint64Value();      stream->DoStreamReset(code);   }
test/parallel/test-quic-stream-error-code-validation.mjsadded53 + / 0 −
@@ -0,0 +1,53 @@+// Flags: --experimental-quic --no-warnings++import { hasQuic, skip, mustCall } from '../common/index.mjs';+import assert from 'node:assert';++if (!hasQuic) {+  skip('QUIC is not enabled');+}++const { listen, connect } = await import('../common/quic.mjs');++const serverEndpoint = await listen(mustCall(() => true));+const clientSession = await connect(serverEndpoint.address);+await clientSession.opened;++const stream = await clientSession.createBidirectionalStream();++const invalidTypes = ['1', true, null];+const invalidCodes = [+  -1,+  1.5,+  NaN,+  Infinity,+  2 ** 62,+  -1n,+  2n ** 62n,+];++for (const method of ['stopSending', 'resetStream']) {+  for (const code of invalidTypes) {+    assert.throws(() => stream[method](code), {+      code: 'ERR_INVALID_ARG_TYPE',+    });+  }++  for (const code of invalidCodes) {+    assert.throws(() => stream[method](code), {+      code: 'ERR_OUT_OF_RANGE',+    });+  }+}++for (const code of invalidCodes) {+  assert.throws(() => stream.destroy(new Error('test'), { code }), {+    code: 'ERR_OUT_OF_RANGE',+  });+}+assert.strictEqual(stream.destroyed, false);++stream.destroy();+await stream.closed;+await clientSession.close();+await serverEndpoint.close();