Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
8 changes: 6 additions & 2 deletions src/runtime/api/bun/h2_frame_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6603,11 +6603,15 @@
if callframe.arguments_count() >= 3 {
if !opaque_data_arg.is_empty_or_undefined_or_null() {
if let Some(array_buffer) = opaque_data_arg.as_array_buffer(global_object) {
let slice = array_buffer.byte_slice();
// send_go_away's write() can re-enter JS on JS-backed sockets (cork flush →
// onWrite → Duplex write), and that JS can detach/transfer this buffer. Own
// the bytes before any JS can run so the GOAWAY payload is what the caller
// passed, not whatever later occupies that heap block.
Comment thread
robobun marked this conversation as resolved.
Resolved
let copied = array_buffer.byte_slice().to_vec();
this.send_go_away(
0,
ErrorCode(error_code as u32),
slice,
&copied,

Check warning on line 6614 in src/runtime/api/bun/h2_frame_parser.rs

View check run for this annotation

Claude / Claude Code Review

Same borrowed-slice-across-JS-reentry bug remains in write_stream/send_data

The same borrowed-slice-across-JS-reentry pattern this PR fixes for `goaway()` remains in `write_stream()` → `send_data()`'s single-frame branch (h2_frame_parser.rs:7314): `StringOrBuffer::from_js_with_encoding` borrows the caller's Buffer, and `writer.write_all(slice)` goes through the identical cork-flush → `onWrite` → user Duplex `_write()` loop over a JS-backed socket. Per REVIEW.md ("fix the whole class… if a site is intentionally excluded, say so in the PR"), either gate a defensive copy o
Comment thread
robobun marked this conversation as resolved.
Resolved
last_stream_id,
false,
);
Expand Down
59 changes: 59 additions & 0 deletions test/js/node/http2/node-http2.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1926,6 +1926,65 @@ it("http2 session.goaway() sends custom data", async done => {
});
});

it("http2 session.goaway() opaqueData survives re-entrant buffer detach over a JS Duplex", async () => {
// The cork-flush path re-enters JS (onWrite → Duplex _write) when the transport is a JS
// stream. If that JS detaches the opaqueData ArrayBuffer mid-write, the GOAWAY payload must
// still be the bytes the caller passed, not whatever later occupies that heap block.
const fixture = `
const http2 = require("node:http2");
const { Duplex } = require("node:stream");
const N = 256 * 1024;
let opaque = new Uint8Array(N).fill(0x41);
const spray = [], received = [];
let armed = false, fired = false;
const duplex = new Duplex({
read() {},
write(chunk, enc, cb) {
received.push(Buffer.from(chunk));
if (armed && !fired && chunk.length >= 16384) {
fired = true;
opaque.buffer.transfer(0);
opaque = null;
Bun.gc(true);
for (let i = 0; i < 64; i++) spray.push(new Uint8Array(N).fill(0x5a));
}
cb();
},
});
const session = http2.connect("http://127.0.0.1:1", { createConnection: () => duplex });
session.on("error", () => {});
session.once("connect", () => {
setImmediate(() => {
received.length = 0;
armed = true;
session.goaway(0, 0, opaque);
setImmediate(() => {
const all = Buffer.concat(received);
let off = 0;
while (off + 9 <= all.length && all[off + 3] !== 7) off += 9 + all.readUIntBE(off, 3);
const len = all.readUIntBE(off, 3);
const payload = all.subarray(off + 17, off + 9 + len);
let ok = 0;
for (const b of payload) if (b === 0x41) ok++;
console.log(JSON.stringify({ fired, declared: payload.length, ok }));
process.exit(0);
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
});
});
`;
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("");
const result = JSON.parse(stdout.trim());
expect(result).toEqual({ fired: true, declared: 256 * 1024, ok: 256 * 1024 });
expect(exitCode).toBe(0);
});

it("http2 server sends protocol-error GOAWAY on stream 0", async () => {
// RFC 9113 section 6.8: GOAWAY frames MUST be sent with a stream identifier
// of 0 in the frame header; the last processed stream id lives in the
Expand Down