Skip to content
Open
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
22b97b8
node:http2: send array values in raw-form header lists as one field p…
robobun Aug 12, 2026
d9d5f35
test: state the element rule the rejection cases exercise
robobun Aug 12, 2026
a6f9214
Trim the comments on the raw header walk and rawHeadersToObject
robobun Aug 12, 2026
917c345
test: run the raw-headers array tests sequentially
robobun Aug 12, 2026
6826664
node:http2: judge the single-value rule on the raw header list slot b…
robobun Aug 12, 2026
fee4e36
Shorten the assertSingleValueHeaders comment
robobun Aug 12, 2026
46fdf86
node:http2: copy the raw header name before coercing its values
robobun Aug 12, 2026
cab32ee
Shorten the owned-name comment
robobun Aug 12, 2026
07414d2
node:http2: apply the raw walk's single-value check once per slot
robobun Aug 12, 2026
73fb09c
node:http2: let request() take an array in the :method slot
robobun Aug 12, 2026
7d565f8
ci: retrigger
robobun Aug 12, 2026
acd595c
Merge branch 'robobun/12c95c7e/http2-raw-list-kept-array' into farm/7…
robobun Oct 8, 2026
4ff37ff
node:http2: send each element of an array in a raw header list as one…
robobun Oct 8, 2026
05cb9ea
node:http2: judge every raw header list slot by slot before the encod…
robobun Oct 15, 2026
704626f
node:http2: refuse a null element of a raw header list before the blo…
robobun Oct 15, 2026
8de7a9f
node:http2: shorten the comments on the raw header list helpers
robobun Oct 15, 2026
b77dff7
node:http2: decide the method of a raw header list from its :method slot
robobun Oct 17, 2026
0d9bc01
ci: retrigger
robobun Oct 18, 2026
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
87 changes: 49 additions & 38 deletions src/js/node/http2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3752,19 +3752,7 @@
headers.push(HTTP2_HEADER_DATE, utcDate());
}
rawHeadersList = headers;
const headersObject = { __proto__: null };
for (let i = 0; i < rawHeadersList.length; i += 2) {
const key = rawHeadersList[i];
const value = rawHeadersList[i + 1];
const existing = headersObject[key];
if (existing === undefined) headersObject[key] = value;
else if ($isArray(existing)) existing.push(value);
else headersObject[key] = [existing, value];
}
if (rawHeadersList[sensitiveHeaders] !== undefined) {
headersObject[sensitiveHeaders] = rawHeadersList[sensitiveHeaders];
}
headers = headersObject;
headers = rawHeadersToObject(rawHeadersList, rawHeadersList[sensitiveHeaders]);
} else if (!$isObject(headers)) {
throw $ERR_INVALID_ARG_TYPE("headers", "object", headers);
} else {
Expand All @@ -3781,7 +3769,9 @@
const sensitiveNames = buildSensitiveNames(headers, sensitives);
// Pre-validate single-value headers in JS so a throwing respond() leaves no partial state in
// the shared HPACK table (same rule request() applies).
if (session[kStrictSingleValueFields] !== false) assertSingleValueHeaders(headers);
if (session[kStrictSingleValueFields] !== false) {
assertSingleValueHeaders(rawHeadersList !== null ? rawHeadersList : headers);
}
// node keeps the never-index list visible on sentHeaders (symbol keys are not iterated by the
// wire-encoding path, so re-attaching is safe).
if (sensitives !== undefined) headers[sensitiveHeaders] = sensitives;
Expand Down Expand Up @@ -4019,22 +4009,36 @@
// node validates header constraints in JS before anything reaches the native encoder, so a
// throwing request leaves no partial state in the shared HPACK table. Mirror the single-value
// rule here: duplicated single-value fields (across case variants) and multi-element arrays for
// them throw before encoding starts.
// them throw before encoding starts. Accepts the object form or the raw [name, value, ...] list.
function assertSingleValueHeaders(headers) {
let seen = null;
if ($isArray(headers)) {
for (let i = 0; i < headers.length; i += 2) {
const name = headers[i];
const value = headers[i + 1];
// A slot the encoder skips is not an occurrence, wherever it is in the list.
if (typeof name !== "string" || value === undefined || ($isArray(value) && value.length === 0)) continue;
seen = noteSingleValueField(seen, name, value);
}
return;
}
const keys = Object.keys(headers);
for (let i = 0; i < keys.length; i++) {
const lower = keys[i].toLowerCase();
if (!kSingleValueHeaders.has(lower)) continue;
const value = headers[keys[i]];
if (($isArray(value) && value.length > 1) || (seen !== null && seen.has(lower))) {
throw $ERR_HTTP2_HEADER_SINGLE_VALUE(`Header field "${lower}" must only have a single value`);
}
if (seen === null) seen = new SafeSet();
seen.add(lower);
seen = noteSingleValueField(seen, keys[i], headers[keys[i]]);
}
}

function noteSingleValueField(seen, name, value) {
const lower = name.toLowerCase();
if (!kSingleValueHeaders.has(lower)) return seen;
if (($isArray(value) && value.length > 1) || (seen !== null && seen.has(lower))) {
throw $ERR_HTTP2_HEADER_SINGLE_VALUE(`Header field "${lower}" must only have a single value`);
}
if (seen === null) seen = new SafeSet();
seen.add(lower);
return seen;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
}

// Renders a received value the way node's determineSpecificType does for error messages.
function receivedValueLabel(value) {
if (value === null) return "null";
Expand Down Expand Up @@ -4069,6 +4073,23 @@
return map;
}

// Backs sentHeaders for the raw [name, value, ...] form.
function rawHeadersToObject(rawHeadersList, sensitives) {
const headersObject = { __proto__: null };
for (let i = 0; i < rawHeadersList.length; i += 2) {
const key = rawHeadersList[i];
let value = rawHeadersList[i + 1];
// The list itself is what gets encoded: a later duplicate must not be pushed into the caller's array.
if ($isArray(value)) value = value.slice();
const existing = headersObject[key];
if (existing === undefined) headersObject[key] = value;
else if ($isArray(existing)) existing.push(value);
Comment thread
robobun marked this conversation as resolved.
Resolved
else headersObject[key] = [existing, value];
}
if (sensitives !== undefined) headersObject[sensitiveHeaders] = sensitives;
return headersObject;
}
Comment thread
robobun marked this conversation as resolved.
Resolved

function toHeaderObject(headers, sensitiveHeadersValue) {
const obj = { __proto__: null, [sensitiveHeaders]: sensitiveHeadersValue };
for (let n = 0; n < headers.length; n += 2) {
Expand Down Expand Up @@ -6173,19 +6194,7 @@
if (path !== undefined) throw $ERR_HTTP2_CONNECT_PATH();
}
rawHeadersList = additionalPseudoHeaders.length ? additionalPseudoHeaders.concat(raw) : raw;
const headersObject = { __proto__: null };
for (let i = 0; i < rawHeadersList.length; i += 2) {
const key = rawHeadersList[i];
const value = rawHeadersList[i + 1];
const existing = headersObject[key];
if (existing === undefined) headersObject[key] = value;
else if ($isArray(existing)) existing.push(value);
else headersObject[key] = [existing, value];
}
if (raw[sensitiveHeaders] !== undefined) {
headersObject[sensitiveHeaders] = raw[sensitiveHeaders];
}
headers = headersObject;
headers = rawHeadersToObject(rawHeadersList, raw[sensitiveHeaders]);
} else if (!$isObject(headers)) {
throw $ERR_INVALID_ARG_TYPE("headers", "object", headers);
} else {
Expand Down Expand Up @@ -6243,7 +6252,9 @@
}
// Validate single-value constraints before anything is encoded (a mid-encode throw would
// desync the shared HPACK table from the peer).
if (this[kStrictSingleValueFields] !== false) assertSingleValueHeaders(headers);
if (this[kStrictSingleValueFields] !== false) {
assertSingleValueHeaders(rawHeadersList !== null ? rawHeadersList : headers);
}
Comment thread
robobun marked this conversation as resolved.
Resolved
// node keeps the never-index list visible on the request's sentHeaders (symbol keys are
// not iterated by the wire-encoding path, so re-attaching is safe).
if (sensitives !== undefined) headers[sensitiveHeaders] = sensitives;
Expand Down Expand Up @@ -6302,7 +6313,7 @@
}

let rejectContentLengthOnNoPayload = false;
if (NoPayloadMethods.has(method.toUpperCase())) {
if (typeof method === "string" && NoPayloadMethods.has(method.toUpperCase())) {

Check warning on line 6316 in src/js/node/http2.ts

View check run for this annotation

Claude / Claude Code Review

typeof-method guard lets ["CONNECT"] bypass CONNECT validation and send :scheme/:path

The `typeof method === "string"` guard added here (73fb09c1) means `request({":method": ["CONNECT"]})` and `request([":method", ["CONNECT"]])` no longer die at `.toUpperCase()` — but every CONNECT check upstream (6169/6265/6293) uses `=== "CONNECT"`, so an array is treated as non-CONNECT: `:scheme`/`:path` are defaulted and `ERR_HTTP2_CONNECT_AUTHORITY` is bypassed, and the encoder puts `:method CONNECT` plus `:scheme`/`:path` on the wire (RFC 9113 §8.5 → peer PROTOCOL_ERROR). Before this PR bot
Comment thread
robobun marked this conversation as resolved.
Resolved
// Like Node, a payload-meaningless method only defaults endStream to
// true when the caller expressed no preference; an explicit endStream
// (validated above) is honored, so { endStream: false } stays open.
Expand Down
125 changes: 82 additions & 43 deletions src/runtime/api/bun/h2_frame_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8852,7 +8852,8 @@ impl H2FrameParser {
}

let name_str = name_js.to_js_string(global_object)?;
let name_slice = name_str.to_slice(global_object);
// Owned: still used after the values' toString() may have dropped the list's reference to it.
let name_slice = name_str.to_slice_clone(global_object)?;
let name = name_slice.slice();
if name.is_empty() {
continue;
Expand Down Expand Up @@ -8895,8 +8896,18 @@ impl H2FrameParser {
continue;
}

let elements = if value_js.js_type().is_array() {
Some(value_js.array_iterator(global_object)?)
} else {
None
};
let field_count = elements.as_ref().map_or(1, |items| items.len);
// node (buildNgHeaderString): [] sends nothing and is not an occurrence.
if field_count == 0 {
continue;
}
if let Some(idx) = this.single_value_index_checked(validated_name) {
if single_value_headers[idx] {
if field_count > 1 || single_value_headers[idx] {
let exception = global_object.to_type_error(
bun_jsc::ErrorCode::HTTP2_HEADER_SINGLE_VALUE,
format_args!(
Expand All @@ -8909,8 +8920,6 @@ impl H2FrameParser {
single_value_headers[idx] = true;
}

let value_str = value_js.to_js_string(global_object)?;

let never_index = if Self::is_index_like_name(validated_name) {
false
} else {
Expand All @@ -8920,48 +8929,78 @@ impl H2FrameParser {
}
};

let value_slice = value_str.to_slice(global_object);
let value = value_slice.slice();
if !is_valid_header_value(value) {
return Err(global_object
.err(
JscErrorCode::HTTP2_INVALID_HEADER_VALUE,
format_args!(
"Invalid value for header \"{}\"",
BStr::new(validated_name)
),
)
.throw());
}
bun_output::scoped_log!(
H2FrameParser,
"encode header {} {}",
BStr::new(validated_name),
BStr::new(value)
);

if let Err(err) = this.encode_header_into_list(
&mut encoded_headers,
validated_name,
value,
never_index,
) {
if matches!(err, crate::Error::Alloc(_)) {
let mut encode_value = |item: JSValue| -> JsResult<Option<JSValue>> {
let value_str = item.to_js_string(global_object)?;
let value_slice = value_str.to_slice(global_object);
let value = value_slice.slice();
if !is_valid_header_value(value) {
return Err(global_object
.throw(format_args!("Failed to allocate header buffer")));
.err(
JscErrorCode::HTTP2_INVALID_HEADER_VALUE,
format_args!(
"Invalid value for header \"{}\"",
BStr::new(validated_name)
),
)
.throw());
}
let Some(stream) = this.handle_received_stream_id(stream_id) else {
return Ok(JSValue::js_number(-1.0));
};
// SAFETY: stream is a *mut Stream from self.streams (heap::alloc); valid while the map entry exists
let stream = unsafe { &mut *stream };
if !stream_ctx_arg.is_empty_or_undefined_or_null()
&& stream_ctx_arg.is_object()
{
stream.set_context(stream_ctx_arg, global_object);
bun_output::scoped_log!(
H2FrameParser,
"encode header {} {}",
BStr::new(validated_name),
BStr::new(value)
);

if let Err(err) = this.encode_header_into_list(
&mut encoded_headers,
validated_name,
value,
never_index,
) {
if matches!(err, crate::Error::Alloc(_)) {
return Err(global_object
.throw(format_args!("Failed to allocate header buffer")));
}
let Some(stream) = this.handle_received_stream_id(stream_id) else {
return Ok(Some(JSValue::js_number(-1.0)));
};
// SAFETY: stream is a *mut Stream from self.streams (heap::alloc); valid while the map entry exists
let stream = unsafe { &mut *stream };
if !stream_ctx_arg.is_empty_or_undefined_or_null()
&& stream_ctx_arg.is_object()
{
stream.set_context(stream_ctx_arg, global_object);
}
this.schedule_header_compression_session_error();
return Ok(Some(JSValue::js_number(stream_id as f64)));
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
Ok(None)
};

match elements {
Some(mut items) => {
while let Some(item) = items.next()? {
if item.is_empty_or_undefined_or_null() {
return Err(global_object
.err(
JscErrorCode::HTTP2_INVALID_HEADER_VALUE,
format_args!(
"Invalid value for header \"{}\"",
BStr::new(validated_name)
),
)
.throw());
}
if let Some(ret) = encode_value(item)? {
return Ok(ret);
}
}
}
None => {
if let Some(ret) = encode_value(value_js)? {
return Ok(ret);
}
}
this.schedule_header_compression_session_error();
return Ok(JSValue::js_number(stream_id as f64));
}
}
}
Expand Down
Loading