#37568 node:http2: coerce outbound header values before taking a stream id

Repro

A header value whose toString() makes another request on the same session. The raw server below only records the stream id of each HEADERS frame it receives, in wire order:

const http2 = require("node:http2"), net = require("node:net");
function frame(t, fl, sid, p = Buffer.alloc(0)) { const h = Buffer.alloc(9); h.writeUIntBE(p.length, 0, 3); h.writeUInt8(t, 3); h.writeUInt8(fl, 4); h.writeUInt32BE(sid, 5); return Buffer.concat([h, p]); }
const server = net.createServer(sock => {
  let buf = Buffer.alloc(0), preface = false; const ids = [];
  sock.on("data", d => {
    buf = Buffer.concat([buf, d]);
    if (!preface) { if (buf.length < 24) return; buf = buf.subarray(24); preface = true; sock.write(frame(4, 0, 0)); }
    while (buf.length >= 9) {
      const len = buf.readUIntBE(0, 3); if (buf.length < 9 + len) break;
      const type = buf.readUInt8(3), sid = buf.readUInt32BE(5) & 0x7fffffff; buf = buf.subarray(9 + len);
      if (type === 1) ids.push(sid);
      if (ids.length === 2) { console.log(JSON.stringify(ids)); process.exit(0); }
    }
  });
});
server.listen(0, "127.0.0.1", () => {
  const client = http2.connect(`http://127.0.0.1:${server.address().port}`);
  client.on("connect", () => {
    let once = false;
    const value = { toString() { if (!once) { once = true; client.request({ ":path": "/inner" }).end(); } return "outer"; } };
    client.request({ ":path": "/outer", "x-probe": value }).end();
  });
});

RFC 9113 §5.1.1 requires every stream an endpoint opens or reserves to have a higher id than the ones before it, so nghttp2 (node's server, curl, ...) ignores the HEADERS on stream 1 and the outer request never gets a response. #37563 makes bun's server do the same for inbound HEADERS, after which this hangs against bun servers too.

Cause

ClientHttp2Session.request() calls parser.getNextStream() and then passes the header object to the native request(), which coerces each value to a string while it encodes the block (to_js_string in h2_frame_parser.rs). Any user code that runs there, such as a toString(), runs after the outer request has taken its id but before its frame is written. A nested request() takes the next id and writes its HEADERS first. ServerHttp2Stream.pushStream() has the identical structure (getNextStream() for the promised id, then the native pushPromise() coerces).

The nested block is also encoded in the middle of the outer block's encoding, so the shared HPACK encoder table is updated in an order that differs from the order the blocks reach the peer; in the repro the nested block references the :authority entry the outer block inserted but has not sent yet, which is the COMPRESSION_ERROR in the table above. A throwing toString() leaves the same half-encoded state behind.

Node cannot hit any of this because buildNgHeaderString stringifies every value in JS inside request() / pushStream() itself, and nghttp2 only assigns the id when it serializes the frame. #31323 already moved request()'s options reads ahead of the encode for the same reason (a getter re-entering request() reordered the blocks); header values were one of the two remaining places user code could run between the id and the frame. The other is diagnostics_channel: request() published http2.client.stream.created and pushStream() published http2.server.stream.created / .start after taking the id but before the native call, so a subscriber making another request / push from its handler produced the same [3,1] / [4,2] wire order (only the id half; each block is encoded whole there, so HPACK stays consistent). Node publishes these after the frame is queued.

Fix

src/js/node/http2.ts: toWireHeaders() builds the block handed to native with every object value (the only kind whose coercion runs user code) turned into a string up front, for the object form, array values (copied element-wise, in both forms, so they still reach the encoder as arrays; this is what lets #37796, which teaches the raw-form encoder about arrays, compose with this change in either merge order), and the raw [name, value, ...] form. request() calls it before the pending-queue check and getNextStream(), and the pending queue stores the coerced form, so a request made while still connecting is coerced when it is made too (node does the same). pushStream() calls it before reserving the promised id. A request that gets queued (made before connect, or behind SETTINGS_MAX_CONCURRENT_STREAMS) queues a copy of the block when nothing in it needed coercing, because in that case the block would otherwise be the very object the returned stream exposes as sentHeaders until the queue drains, and a value put there in the meantime would have been coerced by the encoder at flush time, after the id was taken. Node sends the list it built inside request() in that situation as well, so a later edit of sentHeaders has no effect on the wire in either runtime now.

The channel publishes move to after the frame is written: http2.client.stream.created is published at the end of request() on both branches (node's position as well: a connected request() now publishes start then created like node, and a request made by a subscriber while the session is still connecting is queued behind the one being published instead of ahead of it), and the server push created / start pair is published once pushPromise() has returned, still in that order (the ported test-diagnostics-channel-http2-server-stream-created-start-timing.js checks it). A push whose header block fails native validation still publishes created before its silent teardown, since that is the only handle user code can get on that stream (the existing pushStream failure reports only via callback test relies on it), but no longer publishes start for a stream that was never announced; node publishes neither.

Details that keep the rest of the behavior unchanged:

• Primitives are passed through untouched, so the native validator still produces the same errors for undefined / null / symbol values, and numbers still stringify natively.
• Arrays are always copied, so the native walk only ever reads a fresh array of primitives (nothing left for an accessor or a proxy to run from).
• The caller's object is never mutated: it still backs sentHeaders (node keeps the uncoerced values there as well; the test checks identity) and the diagnostics channel payloads.
• When nothing needs coercing (all values primitive, no arrays), the same object is passed through as before, so the common path adds one Object.keys walk over the block and does not copy it. Otherwise the block is copied before any coercion runs and the copy is what gets coerced, so the encoder never sees anything a coercion did to the object it was read from.
• A toString() that destroys the session instead of making a request now leaves request() returning a pending stream, which is what node returns there too (before, the stream had already taken an id); pushStream() behaves as before in that case.

A throwing coercion now propagates out of request() / pushStream() synchronously, which is what node does (verified against v26.3.0 for both); before, a pending request() reported it as a later stream 'error' and pushStream() routed it to the callback, and in both cases the session was left with a desynced HPACK table. The existing request() propagates a throwing header-value toString() test in node-http2.test.js still passes.

The encode side of the same problem for respond() / additionalHeaders() / sendTrailers() (which have a fixed stream id, so only the HPACK half applies) is what #33470 addresses natively by collecting the block before encoding; this PR is the id-allocation half that a native change cannot cover, since the id is taken in JS before native is entered.

The other way to close this would be to stop taking the id in JS at all and have the native call allocate it as it writes the frame, the way nghttp2 does. That means creating the stream object without an id and attaching it afterwards (the push path creates the pushed stream from inside the id allocation today), and it would land in the middle of the native request() that #33470 is restructuring, for no gain over this once the only things left between getNextStream() and the write are the stream construction and bookkeeping in this module: header coercion, the channel publishes and the queued block were the inputs user code could reach, and all three are handled here.

Verification

Twelve tests in test/js/node/http2/h2-conformance.test.ts (request()/pushStream() run no user code between taking a stream id and writing its frame): HEADERS wire order plus the ids each stream ends up with for the object, array-element, raw-array and raw-array-with-an-array-slot forms, the same for a request() made before the session connected, the same for a request made from a http2.client.stream.created subscriber on a connected and on a still-connecting session (it must come second both times: [1,3] with the outer request on 1) and for a push made from a http2.server.stream.created subscriber ([2,4] with the outer push on 2), a request queued before connect whose sentHeaders is given a re-entering value before the flush (it must never be coerced, and exactly one HEADERS frame goes out), an end-to-end run against an http2.createServer() checking that both requests are answered with their own headers (and that sentHeaders keeps the original value), PUSH_PROMISE promised-id order observed by a raw client, and the synchronous throw plus the session staying usable afterwards. The expected values were checked against node v26.3.0 with the same scenarios.

I also ran a script sending about 45 value shapes through request() (numbers, bigint, Date, Buffer and Uint8Array values and elements, boxed strings, Symbol.toPrimitive / valueOf-only objects, functions, nested / holey / empty arrays, Array subclasses, proxies, undefined / null / symbol values and elements, and the raw array form with numeric, object, null and symbol slots) against an http2.createServer() and compared what the server received or what was thrown on the unfixed and fixed builds. The only difference is a Proxy wrapping an array, which used to be sent as one joined field and is now sent as one field per element, which is what node does (Array.isArray sees through proxies).

All twelve fail on the unfixed build ([3,1] / [4,2] / Session closed with error code 9 / returned) and pass with the fix. bun bd test test/js/node/http2/ (476 tests), the 256 upstream test-http2-*.js files and the 18 test-diagnostics-channel-http2-*.js files all pass with the fix.

[review] gate passed · iteration 1 · 2 files touched

ASAN without fix: 12 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" "test/js/node/http2/h2-conformance.test.ts"
bun test v1.4.0 (f7554f227)

test/js/node/http2/h2-conformance.test.ts:
(pass) connection preface & SETTINGS handshake (checklist §1) > server sends a SETTINGS frame first (§1.4) [385.60ms]
(pass) connection preface & SETTINGS handshake (checklist §1) > server ACKs the client's SETTINGS frame (§3.5) [108.46ms]
(pass) connection preface & SETTINGS handshake (checklist §1) > a SETTINGS frame with a non-zero stream id is a PROTOCOL_ERROR (§3.5) [84.93ms]
(pass) connection preface & SETTINGS handshake (checklist §1) > a SETTINGS frame whose length is not a multiple of 6 is a FRAME_SIZE_ERROR (§3.5) [42.89ms]
(pass) connection preface & SETTINGS handshake (checklist §1) > a SETTINGS ACK that carries a payload is a FRAME_SIZE_ERROR (§3.5) [38.18ms]
(pass) PING (checklist §3.7) > server replies to PING with a PING ACK echoing the payload [38.84ms]
(pass) PING (checklist §3.7) > a PING with length != 8 is a FRAME_SIZE_ERROR [40.23ms]
(pass) PING (checklist §3.7) > a PING on a non-zero st
... (truncated)

release without fix: 12 FAILED
bun test v1.4.0-canary.1 (da3851e57)

test/js/node/http2/h2-conformance.test.ts:
(pass) connection preface & SETTINGS handshake (checklist §1) > server sends a SETTINGS frame first (§1.4) [6.98ms]
(pass) connection preface & SETTINGS handshake (checklist §1) > server ACKs the client's SETTINGS frame (§3.5) [1.77ms]
(pass) connection preface & SETTINGS handshake (checklist §1) > a SETTINGS frame with a non-zero stream id is a PROTOCOL_ERROR (§3.5) [1.58ms]
(pass) connection preface & SETTINGS handshake (checklist §1) > a SETTINGS frame whose length is not a multiple of 6 is a FRAME_SIZE_ERROR (§3.5) [0.85ms]
(pass) connection preface & SETTINGS handshake (checklist §1) > a SETTINGS ACK that carries a payload is a FRAME_SIZE_ERROR (§3.5) [0.76ms]
(pass) PING (checklist §3.7) > server replies to PING with a PING ACK echoing the payload [0.69ms]
(pass) PING (checklist §3.7) > a PING with length != 8 is a FRAME_SIZE_ERROR [0.76ms]
(pass) PING (checklist §3.7) > a PING on a non-zero stream id is a PROTOCOL_ERROR [0.66ms]
(pass) WINDOW_UPDATE (checklist §6) > a connection-level WINDOW_UPDATE with a 0 increment is a PROTOCOL_ERROR [0.63ms]
(pass) WINDOW_UPDATE
... (truncated)

ASAN with fix: all passed
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" "test/js/node/http2/h2-conformance.test.ts"
bun test v1.4.0 (f7554f227)

test/js/node/http2/h2-conformance.test.ts:
(pass) connection preface & SETTINGS handshake (checklist §1) > server sends a SETTINGS frame first (§1.4) [388.97ms]
(pass) connection preface & SETTINGS handshake (checklist §1) > server ACKs the client's SETTINGS frame (§3.5) [102.44ms]
(pass) connection preface & SETTINGS handshake (checklist §1) > a SETTINGS frame with a non-zero stream id is a PROTOCOL_ERROR (§3.5) [77.83ms]
(pass) connection preface & SETTINGS handshake (checklist §1) > a SETTINGS frame whose length is not a multiple of 6 is a FRAME_SIZE_ERROR (§3.5) [42.27ms]
(pass) connection preface & SETTINGS handshake (checklist §1) > a SETTINGS ACK that carries a payload is a FRAME_SIZE_ERROR (§3.5) [36.09ms]
(pass) PING (checklist §3.7) > server replies to PING with a PING ACK echoing the payload [32.43ms]
(pass) PING (checklist §3.7) > a PING with length != 8 is a FRAME_SIZE_ERROR [40.08ms]
(pass) PING (checklist §3.7) > a PING on a non-zero st
... (truncated)

release with fix: all passed
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped)
  target       linux-x64-gnu
  build type   Release
  build dir    ./build/release
  revision     f7554f227c
  features     baseline

22 deps, 107 codegen, 1176 objects in 1050ms

ninja: Entering directory `/workspace/bun/build/release'
[1/1238] install /workspace/bun
bun install v1.4.0-canary.1 (da3851e57)

Checked 107 installs across 153 packages (no changes) [15.00ms]
[2/1238] gen ErrorCode+*.h
[3/1238] install /workspace/bun/packages/bun-error
bun install v1.4.0-canary.1 (da3851e57)

Checked 1 install across 2 packages (no changes) [5.00ms]
[4/1238] gen bindgenv2
[5/1238] install /workspace/bun/src/node-fallbacks
bun install v1.4.0-canary.1 (da3851e57)

Checked 129 installs across 147 packages (no changes) [14.00ms]
[6/1238] fetch libjpeg-turbo
[libjpeg-turbo] up to date
[7/1238] fetch tinycc
[tinycc] up to date
[8/1237] gen .bind.ts → GeneratedBindings.cpp
[9/1237] fetch zlib
[zlib] up to date
[10/1237] gen ProcessBindingConstants.lut.h
Generating /workspace/bun/build/release/codegen/ProcessBindingConstants.lut.h from /workspace/bun/src/jsc/bindings/ProcessBindingConstants.cp
... (truncated)

src/js/node/http2.ts                      |  87 +++++++--
 test/js/node/http2/h2-conformance.test.ts | 312 ++++++++++++++++++++++++++++++
 2 files changed, 382 insertions(+), 17 deletions(-)

gate history · 2 passed · 0 rejected · iteration 1

file                                       reads  edits  tests
src/js/node/http2.ts                          28     36      0
test/js/node/http2/h2-conformance.test.ts     11     11      0