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
27 changes: 24 additions & 3 deletions src/jsc/rare_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,9 @@ pub struct RareData {

pub(crate) temp_pipe_read_buffer: Option<Box<PipeReadBuffer>>,

/// `node:http2` PADDED DATA scratch; see [`Self::take_h2_padded_frame_buffer`].
h2_padded_frame_buffer: Option<Box<H2PaddedFrameBuffer>>,

// There is intentionally no `aws_signature_cache` field — storage lives in
// `bun_s3_signing::credentials::AWS_SIGNATURE_CACHE` (process static; it
// was always reached via the main-thread VM, so it was a singleton in
Expand Down Expand Up @@ -326,6 +329,7 @@ impl Default for RareData {
memory_pressure_watcher: None,
listening_sockets_for_watch_mode: Mutex::new(Vec::new()),
temp_pipe_read_buffer: None,
h2_padded_frame_buffer: None,
s3_default_client: Strong::empty(),
node_quic_callbacks: Strong::empty(),
default_csrf_secret: Box::default(),
Expand Down Expand Up @@ -382,6 +386,9 @@ impl PathBuf {
// remains a stable path for existing callers.
pub use bun_event_loop::PipeReadBuffer;

/// One max-size HTTP/2 PADDED DATA frame payload (pad-length byte + data + padding).
pub type H2PaddedFrameBuffer = [u8; 16384];

// ──────────────────────────────────────────────────────────────────────────
// ProxyEnvStorage
// ──────────────────────────────────────────────────────────────────────────
Expand Down Expand Up @@ -661,6 +668,20 @@ impl RareData {
.get_or_insert_with(bun_core::boxed_zeroed::<PipeReadBuffer>)
}

/// Take the padded-frame scratch out of its slot (lazily allocated). By value rather
/// than borrowed: the socket write it feeds can re-enter JS and reach this path
/// again, and that nested caller then finds the slot empty and allocates its own.
Comment thread
robobun marked this conversation as resolved.
pub fn take_h2_padded_frame_buffer(&mut self) -> Box<H2PaddedFrameBuffer> {
self.h2_padded_frame_buffer
.take()
.unwrap_or_else(bun_core::boxed_zeroed::<H2PaddedFrameBuffer>)
}

/// Hand a taken buffer back; the slot keeps the first one returned.
pub fn put_back_h2_padded_frame_buffer(&mut self, buffer: Box<H2PaddedFrameBuffer>) {
self.h2_padded_frame_buffer.get_or_insert(buffer);
}

pub fn boring_engine(&mut self) -> *mut boring::ENGINE {
// The raw `ENGINE_new()` result is cached without a null check:
// `EVP_DigestInit_ex` tolerates a NULL engine, so OOM here degrades to
Expand Down Expand Up @@ -1043,9 +1064,9 @@ fn get_tls_default_ciphers_from_js(

impl Drop for RareData {
fn drop(&mut self) {
// temp_pipe_read_buffer / spawn_sync_event_loop_ / s3_default_client /
// default_csrf_secret / cleanup_hooks / cron_jobs / path_buf /
// tls_default_ciphers:
// temp_pipe_read_buffer / h2_padded_frame_buffer / spawn_sync_event_loop_ /
// s3_default_client / default_csrf_secret / cleanup_hooks / cron_jobs /
// path_buf / tls_default_ciphers:
Comment thread
robobun marked this conversation as resolved.
// all dropped automatically via field Drop.

if let Some(engine) = self.boring_ssl_engine.take() {
Expand Down
74 changes: 30 additions & 44 deletions src/runtime/api/bun/h2_frame_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1093,10 +1093,9 @@ type H2FrameParserHiveAllocator = HiveArrayFallback<H2FrameParser, 256>;
const H2_CORK_BUFFER_SIZE: usize = 16384;

thread_local! {
// Boxed so only a pointer lives in static TLS — these two buffers are 32 KB
// combined and would otherwise dominate PT_TLS MemSiz on every thread
// (see test/js/bun/binary/tls-segment-size). Lazily allocated on first
// HTTP/2 access; threads that never touch h2 pay nothing.
// Boxed so only a pointer lives in static TLS — a 16 KB buffer would otherwise
// dominate PT_TLS MemSiz on every thread (see test/js/bun/binary/tls-segment-size).
// Lazily allocated on first HTTP/2 access; threads that never touch h2 pay nothing.
Comment thread
robobun marked this conversation as resolved.
static CORK_BUFFER: RefCell<Box<[u8; H2_CORK_BUFFER_SIZE]>> =
RefCell::new(Box::new([0u8; H2_CORK_BUFFER_SIZE]));
static CORK_OFFSET: Cell<u16> = const { Cell::new(0) };
Expand All @@ -1119,7 +1118,6 @@ 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]));
}
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 +1736,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 +1788,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 +6139,30 @@ impl bun_io::Write for DirectWriterStruct {
}
}

impl DirectWriterStruct {
/// The payload of a PADDED DATA frame (RFC 9113 6.1); the caller wrote the frame header.
fn write_padded(&mut self, data: &[u8], padding: u8) -> bun_io::Result<()> {
let payload_size = 1 + data.len() + padding as usize;
// Taken by value so a re-entrant call gets its own; see take_h2_padded_frame_buffer.
let global = self.writer.global();
let mut buffer = global
.bun_vm()
.as_mut()
.rare_data()
.take_h2_padded_frame_buffer();
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]);
global
.bun_vm()
.as_mut()
.rare_data()
.put_back_h2_padded_frame_buffer(buffer);
result
}
Comment thread
claude[bot] marked this conversation as resolved.
}

// ──────────────────────────────────────────────────────────────────────────
// H2FrameParser impl — JS host fns (part 1)
// ──────────────────────────────────────────────────────────────────────────
Expand Down Expand Up @@ -7294,19 +7292,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
72 changes: 72 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,78 @@ 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 fail = what => err => {
console.error(what, err);
process.exit(1);
};
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;
// The first chunk carrying outer's payload is the mid-frame cork flush.
if (!reentered && chunk.includes(0x41)) {
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", fail("session error"));
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", fail("stream error"));
// The three HEADERS frames leave the cork in this tick's deferred auto-flush, which runs
// before the loop reaches the next immediate.
await new Promise(r => setImmediate(r));
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