diff --git a/src/http/lshpack.rs b/src/http/lshpack.rs index 2d42a58cdd9a..4adaba7a3494 100644 --- a/src/http/lshpack.rs +++ b/src/http/lshpack.rs @@ -29,11 +29,9 @@ pub struct HPACK { self_: *mut c_void, } -pub struct DecodeResult { - // TODO(port): lifetime โ€” name/value point into an FFI thread_local shared buffer, - // valid only until the next decode/encode call. Consider `DecodeResult<'a>`. - pub name: &'static [u8], - pub value: &'static [u8], +pub struct DecodeResult<'a> { + pub name: &'a [u8], + pub value: &'a [u8], pub never_index: bool, pub well_know: u16, /// offset of the next header position in src @@ -69,8 +67,8 @@ impl HPACK { // TODO(port): wrap in an owning newtype with Drop instead of returning a raw *mut HPACK } - /// DecodeResult name and value uses a thread_local shared buffer and should be copy/cloned before the next decode/encode call - pub fn decode(&mut self, src: &[u8]) -> Result { + /// DecodeResult name and value borrow this handle's decode buffer and stay valid until the next decode/encode call on it + pub fn decode<'a>(&'a mut self, src: &[u8]) -> Result, HpackError> { let mut header = lshpack_header::default(); // SAFETY: genuine FFI โ€” only the `(src.as_ptr(), src.len())` pair carries // an obligation here (in-bounds read), discharged by `src: &[u8]`. The @@ -85,9 +83,10 @@ impl HPACK { } // SAFETY: lshpack_wrapper_decode writes name/value as offsets into the - // thread_local `shared_header_buffer` (set via lsxpack_header_prepare_decode), - // so both pointers are provably non-null after a successful decode. Use bare - // `from_raw_parts` to avoid a dead null-branch on the per-HTTP/2-header path. + // wrapper-owned staging buffer (set via lsxpack_header_prepare_decode), + // so both pointers are provably non-null after a successful decode and + // stay valid for the `&'a mut self` borrow returned to the caller. Use + // bare `from_raw_parts` to avoid a dead null-branch on the per-HTTP/2-header path. let (name, value) = unsafe { ( core::slice::from_raw_parts(header.name, header.name_len), diff --git a/src/jsc/bindings/c-bindings.cpp b/src/jsc/bindings/c-bindings.cpp index 13d71e7be485..f0e287ffe1a6 100644 --- a/src/jsc/bindings/c-bindings.cpp +++ b/src/jsc/bindings/c-bindings.cpp @@ -337,21 +337,6 @@ extern "C" void on_before_reload_process_linux() #define LSHPACK_MAX_HEADER_SIZE 65536 -// Lazily heap-allocated so it doesn't land in the .tls section on Windows -// (PE has no TLS BSS; a static thread_local char[65536] ships as 64 KB of -// zeros in bun.exe and is copied into every thread's TLS block at creation). -// unique_ptr so the allocation is released when a Worker thread exits โ€” -// thread_local destructors run via __cxa_thread_atexit and are unaffected -// by -fno-c++-static-destructors. -static char* shared_header_buffer_get() -{ - static thread_local std::unique_ptr buffer; - if (!buffer) [[unlikely]] { - buffer.reset(new char[LSHPACK_MAX_HEADER_SIZE]); - } - return buffer.get(); -} - extern "C" { typedef void* (*lshpack_wrapper_alloc)(size_t size); typedef void (*lshpack_wrapper_free)(void*); @@ -359,6 +344,7 @@ typedef struct { struct lshpack_enc enc; struct lshpack_dec dec; lshpack_wrapper_free free; + char buffer[LSHPACK_MAX_HEADER_SIZE]; } lshpack_wrapper; typedef struct { @@ -393,12 +379,11 @@ size_t lshpack_wrapper_encode(lshpack_wrapper* self, if (name_len + val_len > LSHPACK_MAX_HEADER_SIZE) return 0; - char* shared_header_buffer = shared_header_buffer_get(); lsxpack_header_t hdr; memset(&hdr, 0, sizeof(lsxpack_header_t)); - memcpy(&shared_header_buffer[0], name, name_len); - memcpy(&shared_header_buffer[name_len], val, val_len); - lsxpack_header_set_offset2(&hdr, &shared_header_buffer[0], 0, name_len, name_len, val_len); + memcpy(&self->buffer[0], name, name_len); + memcpy(&self->buffer[name_len], val, val_len); + lsxpack_header_set_offset2(&hdr, &self->buffer[0], 0, name_len, name_len, val_len); if (never_index) { hdr.indexed_type = 2; } @@ -415,7 +400,7 @@ size_t lshpack_wrapper_decode(lshpack_wrapper* self, { lsxpack_header_t hdr; memset(&hdr, 0, sizeof(lsxpack_header_t)); - lsxpack_header_prepare_decode(&hdr, shared_header_buffer_get(), 0, LSHPACK_MAX_HEADER_SIZE); + lsxpack_header_prepare_decode(&hdr, self->buffer, 0, LSHPACK_MAX_HEADER_SIZE); const unsigned char* s = src; diff --git a/src/runtime/api/bun/h2_frame_parser.rs b/src/runtime/api/bun/h2_frame_parser.rs index fa0f9ae741c7..432a87923085 100644 --- a/src/runtime/api/bun/h2_frame_parser.rs +++ b/src/runtime/api/bun/h2_frame_parser.rs @@ -2159,7 +2159,13 @@ impl AbortListener for SignalRef { } } -type HeaderValue = lshpack::DecodeResult; +pub(crate) struct HeaderValue { + name_len: usize, + never_index: bool, + well_know: u16, + /// offset of the next header position in src + next: usize, +} // PORT NOTE: `lshpack::HpackError` does not yet impl `From` for `bun_core::Error` // (see TODO in lshpack.rs). Map variants 1:1 to interned error names so Zig @@ -2215,12 +2221,27 @@ impl H2FrameParser { } } - pub(crate) fn decode(&self, src_buffer: &[u8]) -> Result { + /// Decodes one header from `src_buffer` into `scratch` (cleared first): + /// name bytes followed by value bytes, split at `HeaderValue::name_len`. + pub(crate) fn decode( + &self, + src_buffer: &[u8], + scratch: &mut Vec, + ) -> Result { self.hpack.with_mut(|hpack| { if let Some(hpack) = hpack.as_mut() { - return hpack + let result = hpack .decode(src_buffer) - .map_err(|e| hpack_error_to_core(&e)); + .map_err(|e| hpack_error_to_core(&e))?; + scratch.clear(); + scratch.extend_from_slice(result.name); + scratch.extend_from_slice(result.value); + return Ok(HeaderValue { + name_len: result.name.len(), + never_index: result.never_index, + well_know: result.well_know, + next: result.next, + }); } Err(bun_core::err!("UnableToDecode")) }) @@ -3339,8 +3360,9 @@ impl H2FrameParser { // for every other stream. The rejection is applied once after the loop. let mut rejected = false; + let mut scratch: Vec = Vec::new(); while offset < payload.len() { - let header = match self.decode(&payload[offset..]) { + let header = match self.decode(&payload[offset..], &mut scratch) { Ok(h) => h, Err(_) => { // RFC 9113 ยง4.3: a decoding error in a header block is a @@ -3356,13 +3378,14 @@ impl H2FrameParser { } }; offset += header.next; + let (name, value) = scratch.split_at(header.name_len); bun_output::scoped_log!( H2FrameParser, "header {} {}", - BStr::new(header.name), - BStr::new(header.value) + BStr::new(name), + BStr::new(value) ); - if self.is_server.get() && header.name == b":status" { + if self.is_server.get() && name == b":status" { self.send_go_away( stream_id, ErrorCode::PROTOCOL_ERROR, @@ -3375,8 +3398,7 @@ impl H2FrameParser { // RFC 7540 Section 6.5.2: Calculate header list size // Size = name length + value length + HPACK entry overhead per header - stream.header_block_size += - header.name.len() + header.value.len() + HPACK_ENTRY_OVERHEAD; + stream.header_block_size += name.len() + value.len() + HPACK_ENTRY_OVERHEAD; stream.header_block_count += 1; // Check against maxHeaderListSize / maxHeaderListPairs. @@ -3389,7 +3411,7 @@ impl H2FrameParser { continue; } - let is_pseudo_header = header.name.first() == Some(&b':'); + let is_pseudo_header = name.first() == Some(&b':'); if is_pseudo_header { if seen_regular_header { malformed = true; @@ -3397,8 +3419,8 @@ impl H2FrameParser { } else { seen_regular_header = true; } - if is_pseudo_header || header.name == b"content-length" { - if let Some(idx) = single_value_headers_index_of(header.name) { + if is_pseudo_header || name == b"content-length" { + if let Some(idx) = single_value_headers_index_of(name) { if single_value_headers[idx] { malformed = true; } @@ -3407,14 +3429,14 @@ impl H2FrameParser { } if malformed - || is_malformed_field_name(header.name) - || is_malformed_field_value(header.value) - || is_forbidden_connection_specific_header(header.name, header.value) + || is_malformed_field_name(name) + || is_malformed_field_value(value) + || is_forbidden_connection_specific_header(name, value) || (is_pseudo_header && !if self.is_server.get() { - is_valid_request_pseudo_header(header.name) + is_valid_request_pseudo_header(name) } else { - is_valid_response_pseudo_header(header.name) + is_valid_response_pseudo_header(name) }) { malformed = true; @@ -3424,7 +3446,7 @@ impl H2FrameParser { headers.push(&global_object, js_header_name)?; headers.push( &global_object, - bun_jsc::bun_string_jsc::create_utf8_for_js(&global_object, header.value)?, + bun_jsc::bun_string_jsc::create_utf8_for_js(&global_object, value)?, )?; if header.never_index { if sensitive_headers.is_undefined() { @@ -3435,9 +3457,9 @@ impl H2FrameParser { } } else { let js_header_name = - bun_jsc::bun_string_jsc::create_utf8_for_js(&global_object, header.name)?; + bun_jsc::bun_string_jsc::create_utf8_for_js(&global_object, name)?; let js_header_value = - bun_jsc::bun_string_jsc::create_utf8_for_js(&global_object, header.value)?; + bun_jsc::bun_string_jsc::create_utf8_for_js(&global_object, value)?; if header.never_index { if sensitive_headers.is_undefined() {