node:http2: make setNextStreamID ignore the ids nghttp2 rejects

src/runtime/api/bun/h2_frame_parser.rs

@@ -8271,23 +8271,23 @@ 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`.
+    /// First id of this side's parity above `last_stream_id`, which never exceeds `MAX_STREAM_ID`.
     fn get_next_stream_id(&self) -> u32 {
         let stream_id = self.last_stream_id.get();
         if self.is_server.get() {
             if stream_id.is_multiple_of(2) {
-                stream_id.saturating_add(2)
+                stream_id + 2
             } else {
-                stream_id.saturating_add(1)
+                stream_id + 1
             }
         } else if stream_id.is_multiple_of(2) {
-            stream_id.saturating_add(1)
+            stream_id + 1
         } else {
-            stream_id.saturating_add(2)
+            stream_id + 2
         }
     }
 
+    /// Node's setNextStreamID is `nghttp2_session_set_next_stream_id`: invalid ids are ignored.
     #[bun_jsc::host_fn(method)]
     pub(crate) fn set_next_stream_id(
         this: &Self,
@@ -8298,22 +8298,16 @@ impl H2FrameParser {
         debug_assert!(args_list.len() >= 1);
         let stream_id_arg = args_list[0];
         debug_assert!(stream_id_arg.is_number());
-        // 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 {
-                next_stream_id.saturating_sub(1)
-            }
-        } else if next_stream_id.is_multiple_of(2) {
-            next_stream_id.saturating_sub(1)
-        } else {
-            next_stream_id.saturating_sub(2)
-        };
-        this.last_stream_id.set(last_stream_id);
+        let local_parity = u32::from(!this.is_server.get());
+        if next_stream_id % 2 != local_parity
+            || next_stream_id > MAX_STREAM_ID
+            || next_stream_id <= this.get_next_stream_id()
+        {
+            return Ok(JSValue::UNDEFINED);
+        }
+        // Above the next id with this side's parity, so this still moves `last_stream_id` forward.
+        this.last_stream_id.set(next_stream_id - 2);
         Ok(JSValue::UNDEFINED)
     }
 

test/js/node/http2/node-http2.test.js

@@ -2671,8 +2671,9 @@ 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.
+  // underflow (node ends up on stream 1 too). 2 ** 32 - 1 passes the JS range check as well; on a
+  // server it is ignored as an odd id (node ignores it too), where it used to be stored and
+  // overflow the next id computation. The 31-bit bound itself is covered by the next test.
   const fixture = `
     const http2 = require("node:http2");
     const result = { client: {}, server: {} };
@@ -2724,12 +2725,153 @@ it("http2 setNextStreamID at the edges of the id space does not overflow", async
   expect(stderr).toBe("");
   expect(JSON.parse(stdout.trim())).toEqual({
     client: { 0.5: 1, 1: 1 },
-    server: { 0: 2, 2: 2, 4294967295: 4294967295 },
+    server: { 0: 2, 2: 2, 4294967295: 2 },
     response: { streamId: 1, status: 200 },
   });
   expect(exitCode).toBe(0);
 });
 
+it("http2 setNextStreamID ignores the ids nghttp2 rejects instead of rounding them or moving backwards", async () => {
+  // Expected values come from node v26.3.0, where setNextStreamID ends up in
+  // nghttp2_session_set_next_stream_id: an id of the peer's parity, an id that is not above the
+  // current next id, or one that does not fit in 31 bits leaves the session untouched. The server
+  // side is checked while handling the client's first stream (id 1), where node reports 2 as the
+  // server's next id too. The "equal to next" rows also read lastProcStreamID: it is the one value
+  // that tells an ignored call apart from one that re-stores the current position.
+  const readState = ({ nextStreamID, lastProcStreamID }) => [nextStreamID, lastProcStreamID];
+  const server = http2.createServer();
+  const serverSide = Promise.withResolvers();
+  const pushed = Promise.withResolvers();
+  const failure = Promise.withResolvers();
+  server.on("sessionError", failure.reject);
+  server.on("stream", (stream, headers) => {
+    if (headers[":path"] !== "/first") {
+      stream.respond();
+      stream.end();
+      return;
+    }
+    try {
+      const { session } = stream;
+      // ServerHttp2Session does not expose setNextStreamID; call the native setter it would wrap.
+      const parser = session[Symbol.for("::bunhttp2native::")];
+      const set = id => {
+        parser.setNextStreamID(id);
+        return session.state.nextStreamID;
+      };
+      const seen = { "initial [next, lastProc]": readState(session.state) };
+      parser.setNextStreamID(2);
+      seen["set 2 (equal to next) [next, lastProc]"] = readState(session.state);
+      seen["set 100"] = set(100);
+      seen["set 101 (odd)"] = set(101);
+      seen["set 105 (odd)"] = set(105);
+      seen["set 50 (below next)"] = set(50);
+      seen["set 100 (equal to next)"] = set(100);
+      stream.pushStream({ ":path": "/pushed" }, (err, pushedStream) => {
+        if (err) return failure.reject(err);
+        pushedStream.respond();
+        pushedStream.end();
+      });
+      seen["after pushStream"] = session.state.nextStreamID;
+      seen["set 50 after pushStream"] = set(50);
+      seen["set 104 (lowest id above next)"] = set(104);
+      seen["set 2 ** 31 (not 31-bit)"] = set(2 ** 31);
+      seen["set 2 ** 32 - 2 (not 31-bit)"] = set(2 ** 32 - 2);
+      seen["set 2 ** 31 - 2 (highest even)"] = set(2 ** 31 - 2);
+      serverSide.resolve(seen);
+    } catch (err) {
+      failure.reject(err);
+    }
+    stream.respond();
+    stream.end();
+  });
+  await new Promise(resolve => server.listen(0, "127.0.0.1", resolve));
+  const client = http2.connect(`http://127.0.0.1:${server.address().port}`);
+  client.on("error", failure.reject);
+  client.once("stream", (pushedStream, pushHeaders) => {
+    pushedStream.resume();
+    pushed.resolve({ id: pushedStream.id, path: pushHeaders[":path"] });
+  });
+  const set = id => {
+    client.setNextStreamID(id);
+    return client.state.nextStreamID;
+  };
+  const request = path =>
+    new Promise((resolve, reject) => {
+      const req = client.request({ ":path": path });
+      let status;
+      req.on("error", reject);
+      req.on("response", responseHeaders => (status = responseHeaders[":status"]));
+      req.on("close", () => resolve({ id: req.id, status }));
+      req.resume();
+      req.end();
+    });
+  const run = async () => {
+    await new Promise(resolve => client.once("connect", resolve));
+    const seen = { "initial [next, lastProc]": readState(client.state) };
+    client.setNextStreamID(1);
+    seen["set 1 (equal to next) [next, lastProc]"] = readState(client.state);
+    seen["request /first"] = await request("/first");
+    seen["set 5 (lowest id above next)"] = set(5);
+    seen["request /second"] = await request("/second");
+    seen["set 11"] = set(11);
+    seen["set 12 (even)"] = set(12);
+    seen["set 14 (even)"] = set(14);
+    seen["set 7 (below next)"] = set(7);
+    seen["set 11 (equal to next)"] = set(11);
+    seen["request /third"] = await request("/third");
+    seen["after /third"] = client.state.nextStreamID;
+    seen["set 7 after /third"] = set(7);
+    seen["request /fourth"] = await request("/fourth");
+    seen["set 2 ** 31 + 1 (not 31-bit)"] = set(2 ** 31 + 1);
+    seen["set 2 ** 32 - 1 (not 31-bit)"] = set(2 ** 32 - 1);
+    seen["set 2 ** 31 - 1 (highest odd)"] = set(2 ** 31 - 1);
+    return { client: seen, server: await serverSide.promise, pushed: await pushed.promise };
+  };
+
+  try {
+    expect(await Promise.race([run(), failure.promise])).toEqual({
+      client: {
+        "initial [next, lastProc]": [1, 0],
+        "set 1 (equal to next) [next, lastProc]": [1, 0],
+        "request /first": { id: 1, status: 200 },
+        "set 5 (lowest id above next)": 5,
+        "request /second": { id: 5, status: 200 },
+        "set 11": 11,
+        "set 12 (even)": 11,
+        "set 14 (even)": 11,
+        "set 7 (below next)": 11,
+        "set 11 (equal to next)": 11,
+        "request /third": { id: 11, status: 200 },
+        "after /third": 13,
+        "set 7 after /third": 13,
+        "request /fourth": { id: 13, status: 200 },
+        "set 2 ** 31 + 1 (not 31-bit)": 15,
+        "set 2 ** 32 - 1 (not 31-bit)": 15,
+        "set 2 ** 31 - 1 (highest odd)": 2 ** 31 - 1,
+      },
+      server: {
+        "initial [next, lastProc]": [2, 1],
+        "set 2 (equal to next) [next, lastProc]": [2, 1],
+        "set 100": 100,
+        "set 101 (odd)": 100,
+        "set 105 (odd)": 100,
+        "set 50 (below next)": 100,
+        "set 100 (equal to next)": 100,
+        "after pushStream": 102,
+        "set 50 after pushStream": 102,
+        "set 104 (lowest id above next)": 104,
+        "set 2 ** 31 (not 31-bit)": 104,
+        "set 2 ** 32 - 2 (not 31-bit)": 104,
+        "set 2 ** 31 - 2 (highest even)": 2 ** 31 - 2,
+      },
+      pushed: { id: 100, path: "/pushed" },
+    });
+  } finally {
+    client.close();
+    server.close();
+  }
+});
+
 it("http2 request.destroy() with error", async () => {
   const server = http2.createServer();