diff --git a/src/runtime/api/bun/h2_frame_parser.rs b/src/runtime/api/bun/h2_frame_parser.rs index afc5337db482..8c91b7394e35 100644 --- a/src/runtime/api/bun/h2_frame_parser.rs +++ b/src/runtime/api/bun/h2_frame_parser.rs @@ -8271,24 +8271,21 @@ impl H2FrameParser { Ok(JSValue::js_number(result as f64)) } + /// `set_next_stream_id` can park `last_stream_id` anywhere in the u32 range, so the step + /// saturates; callers that open the stream reject anything above `MAX_STREAM_ID`. fn get_next_stream_id(&self) -> u32 { - let mut stream_id: u32 = self.last_stream_id.get(); + let stream_id = self.last_stream_id.get(); if self.is_server.get() { if stream_id.is_multiple_of(2) { - stream_id += 2; + stream_id.saturating_add(2) } else { - stream_id += 1; + stream_id.saturating_add(1) } + } else if stream_id.is_multiple_of(2) { + stream_id.saturating_add(1) } else { - if stream_id.is_multiple_of(2) { - stream_id += 1; - } else if stream_id == 0 { - stream_id = 1; - } else { - stream_id += 2; - } + stream_id.saturating_add(2) } - stream_id } #[bun_jsc::host_fn(method)] @@ -8301,22 +8298,21 @@ impl H2FrameParser { debug_assert!(args_list.len() >= 1); let stream_id_arg = args_list[0]; debug_assert!(stream_id_arg.is_number()); - let mut last_stream_id = stream_id_arg.to_u32(); - if this.is_server.get() { - if last_stream_id.is_multiple_of(2) { - last_stream_id -= 2; + // Store the id `get_next_stream_id` steps from. A fractional id passes the JS layer's + // `id <= 0` check and truncates to 0 here; 0 (and 1 on a client) has no predecessor, + // so the subtraction saturates to the initial state instead of wrapping. + let next_stream_id = stream_id_arg.to_u32(); + let last_stream_id = if this.is_server.get() { + if next_stream_id.is_multiple_of(2) { + next_stream_id.saturating_sub(2) } else { - last_stream_id -= 1; + next_stream_id.saturating_sub(1) } + } else if next_stream_id.is_multiple_of(2) { + next_stream_id.saturating_sub(1) } else { - if last_stream_id.is_multiple_of(2) { - last_stream_id -= 1; - } else if last_stream_id == 1 { - last_stream_id = 0; - } else { - last_stream_id -= 2; - } - } + next_stream_id.saturating_sub(2) + }; this.last_stream_id.set(last_stream_id); Ok(JSValue::UNDEFINED) } diff --git a/test/js/node/http2/node-http2.test.js b/test/js/node/http2/node-http2.test.js index aa4900ab329e..72a35f650fa4 100644 --- a/test/js/node/http2/node-http2.test.js +++ b/test/js/node/http2/node-http2.test.js @@ -2539,6 +2539,67 @@ it("http2 client.setNextStreamID validates input", async () => { }); }); +it("http2 setNextStreamID at the edges of the id space does not overflow", async () => { + // 0.5 passes the JS range check (> 0) and reaches the native setter as 0, which used to + // underflow (node ends up on stream 1 too). A server parked at 2 ** 32 - 1 used to overflow + // when the next id was computed; it has to read back as an unusable id, not wrap to a low one. + const fixture = ` + const http2 = require("node:http2"); + const result = { client: {}, server: {} }; + const fail = what => err => { + console.error(what, err); + process.exit(1); + }; + const server = http2.createServer(); + server.on("sessionError", fail("server session error")); + server.on("stream", stream => { + // ServerHttp2Session does not expose setNextStreamID; call the native setter it would wrap. + const parser = stream.session[Symbol.for("::bunhttp2native::")]; + for (const id of [0, 2, 2 ** 32 - 1]) { + parser.setNextStreamID(id); + result.server[id] = stream.session.state.nextStreamID; + } + stream.respond({ ":status": 200 }); + stream.end(); + }); + server.listen(0, "127.0.0.1", () => { + const client = http2.connect("http://127.0.0.1:" + server.address().port); + client.on("error", fail("client session error")); + client.on("connect", () => { + for (const id of [0.5, 1]) { + client.setNextStreamID(id); + result.client[id] = client.state.nextStreamID; + } + const req = client.request({ ":path": "/" }); + req.on("error", fail("request error")); + req.on("response", headers => { + result.response = { streamId: req.id, status: headers[":status"] }; + }); + req.resume(); + req.on("close", () => { + console.log(JSON.stringify(result)); + process.exit(0); + }); + req.end(); + }); + }); + `; + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", fixture], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stderr).toBe(""); + expect(JSON.parse(stdout.trim())).toEqual({ + client: { 0.5: 1, 1: 1 }, + server: { 0: 2, 2: 2, 4294967295: 4294967295 }, + response: { streamId: 1, status: 200 }, + }); + expect(exitCode).toBe(0); +}); + it("http2 request.destroy() with error", async () => { const server = http2.createServer();