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
68 changes: 28 additions & 40 deletions src/runtime/api/bun/h2_frame_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1119,7 +1119,10 @@ thread_local! {
// pool allocation itself.
static POOL: RefCell<Option<Box<ManuallyDrop<H2FrameParserHiveAllocator>>>> =
const { RefCell::new(None) };
static SHARED_REQUEST_BUFFER: RefCell<Box<[u8; 16384]>> = RefCell::new(Box::new([0u8; 16384]));
// Scratch for assembling one PADDED DATA payload. Moved out of the slot while in use
// (see `DirectWriterStruct::write_padded`): the write can flush the cork and re-enter
// JS, and a nested padded write must get its own buffer.
static PADDED_PAYLOAD_BUFFER: Cell<Option<Box<[u8]>>> = const { Cell::new(None) };
}
Comment thread
robobun marked this conversation as resolved.

/// One wire-order piece of a multi-frame send_data batch (see BATCH_SEGMENTS).
Expand Down Expand Up @@ -1738,19 +1741,7 @@ impl Stream {
};
let _ = data_header.write(&mut writer);
if padding != 0 {
break 'brk SHARED_REQUEST_BUFFER.with_borrow_mut(|buffer| {
// SAFETY: src/dst may overlap — use ptr::copy (memmove)
unsafe {
core::ptr::copy(
able_to_send.as_ptr(),
buffer.as_mut_ptr().add(1),
able_to_send.len(),
);
}
buffer[0] = padding;
buffer[1 + able_to_send.len()..payload_size].fill(0);
writer.write_all(&buffer[0..payload_size]).is_ok()
});
break 'brk writer.write_padded(&able_to_send, padding).is_ok();
} else {
break 'brk writer.write_all(&able_to_send).is_ok();
}
Expand Down Expand Up @@ -1802,19 +1793,7 @@ impl Stream {
};
let _ = data_header.write(&mut writer);
if padding != 0 {
break 'brk SHARED_REQUEST_BUFFER.with_borrow_mut(|buffer| {
// SAFETY: src/dst may overlap — ptr::copy is memmove; dst capacity covers payload_size
unsafe {
core::ptr::copy(
frame_slice.as_ptr(),
buffer.as_mut_ptr().add(1),
frame_slice.len(),
);
}
buffer[0] = padding;
buffer[1 + frame_slice.len()..payload_size].fill(0);
writer.write_all(&buffer[0..payload_size]).is_ok()
});
break 'brk writer.write_padded(frame_slice, padding).is_ok();
} else {
break 'brk writer.write_all(frame_slice).is_ok();
}
Expand Down Expand Up @@ -6165,6 +6144,27 @@ impl bun_io::Write for DirectWriterStruct {
}
}

impl DirectWriterStruct {
/// Writes the payload of a PADDED DATA frame: the pad length, `data`, then `padding`
/// zero bytes (RFC 9113 6.1). The caller has already written the frame header.
Comment thread
robobun marked this conversation as resolved.
Outdated
fn write_padded(&mut self, data: &[u8], padding: u8) -> bun_io::Result<()> {
let payload_size = 1 + data.len() + padding as usize;
// `write()` can flush the cork and, over a JS-backed transport, re-enter JS that
// reaches send_data()/flush_queue() again before this frame's bytes are all corked.
// Own the scratch for the duration so that nested padded write neither trips a held
// borrow nor overwrites bytes this write is still copying from.
let mut buffer = PADDED_PAYLOAD_BUFFER
.take()
.unwrap_or_else(|| vec![0u8; H2_CORK_BUFFER_SIZE].into_boxed_slice());
buffer[0] = padding;
buffer[1..=data.len()].copy_from_slice(data);
buffer[1 + data.len()..payload_size].fill(0);
let result = self.write_all(&buffer[..payload_size]);
PADDED_PAYLOAD_BUFFER.set(Some(buffer));
result
}
Comment thread
claude[bot] marked this conversation as resolved.
}

// ──────────────────────────────────────────────────────────────────────────
// H2FrameParser impl — JS host fns (part 1)
// ──────────────────────────────────────────────────────────────────────────
Expand Down Expand Up @@ -7294,19 +7294,7 @@ impl H2FrameParser {
let mut writer = self.to_writer();
let _ = data_header.write(&mut writer);
if padding != 0 {
SHARED_REQUEST_BUFFER.with_borrow_mut(|buffer| {
// SAFETY: src/dst may overlap — ptr::copy is memmove; dst capacity covers payload_size
unsafe {
core::ptr::copy(
slice.as_ptr(),
buffer.as_mut_ptr().add(1),
slice.len(),
);
}
buffer[0] = padding;
buffer[1 + slice.len()..payload_size].fill(0);
let _ = writer.write_all(&buffer[0..payload_size]);
});
let _ = writer.write_padded(slice, padding);
} else {
let _ = writer.write_all(slice);
}
Expand Down
65 changes: 65 additions & 0 deletions test/js/node/http2/node-http2.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1987,6 +1987,71 @@ it("http2 session.goaway() opaqueData survives re-entrant buffer detach over a J
expect(exitCode).toBe(0);
}, 15_000);

it("http2 padded DATA write survives a re-entrant stream write from a JS Duplex _write", async () => {
// With padding enabled, a single-frame DATA write that crosses the 16 KiB cork boundary
// flushes the cork into the JS transport mid-frame (onWrite → Duplex _write). A transport
// that writes to another padded stream from inside its _write re-enters the same padded
// send path while the outer frame is still being assembled. That must neither abort the
// process nor clobber the outer frame's bytes: every payload byte of all three writes has
// to reach the transport (same totals as node).
const fixture = `
const http2 = require("node:http2");
const { Duplex } = require("node:stream");
// fill: 13000 -> 9+13256 corked; outer: 12000 -> 9+12256, crosses the cork boundary;
// side: 3000 -> 9+3256, written from inside the transport _write during that flush.
const EXPECTED_WIRE = 9 + 13256 + 9 + 12256 + 9 + 3256;
let side, armed = false, reentered = false, total = 0, onComplete;
const counts = { 0x41: 0, 0x42: 0, 0x43: 0 };
const duplex = new Duplex({
read() {},
write(chunk, enc, cb) {
if (armed) {
for (const b of chunk) if (b in counts) counts[b]++;
total += chunk.length;
if (!reentered) {
reentered = true;
side.write(Buffer.alloc(3000, 0x42));
}
if (total >= EXPECTED_WIRE && onComplete) onComplete();
}
cb();
},
});
const session = http2.connect("http://127.0.0.1:1", {
createConnection: () => duplex,
paddingStrategy: http2.constants.PADDING_STRATEGY_MAX,
});
session.on("error", () => {});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
await new Promise(r => session.once("connect", r));
const frame = (type, flags) => Buffer.from([0, 0, 0, type, flags, 0, 0, 0, 0]);
// server preface by hand: empty SETTINGS, then the ACK for the client's SETTINGS
duplex.push(Buffer.concat([frame(4, 0), frame(4, 1)]));
await new Promise(r => setImmediate(r));
side = session.request({ ":method": "POST", ":path": "/side" }, { endStream: false });
const outer = session.request({ ":method": "POST", ":path": "/outer" }, { endStream: false });
const fill = session.request({ ":method": "POST", ":path": "/fill" }, { endStream: false });
for (const s of [side, outer, fill]) s.on("error", () => {});
await new Promise(r => setImmediate(r)); // HEADERS leave the cork at the end of the tick
const complete = new Promise(r => (onComplete = r));
fill.write(Buffer.alloc(13000, 0x43));
armed = true;
outer.write(Buffer.alloc(12000, 0x41));
if (total < EXPECTED_WIRE) await complete;
console.log(JSON.stringify({ reentered, total, A: counts[0x41], B: counts[0x42], C: counts[0x43] }));
process.exit(0);
`;
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({ reentered: true, total: 28795, A: 12000, B: 3000, C: 13000 });
Comment thread
coderabbitai[bot] marked this conversation as resolved.
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