Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 20 additions & 24 deletions src/runtime/api/bun/h2_frame_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand All @@ -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)
}
Expand Down
61 changes: 61 additions & 0 deletions test/js/node/http2/node-http2.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down