Skip to content

Extract a by-reference Request body after the other init members are read #48493

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
robobun wants to merge 4 commits into
base: main
Choose a base branch
from
+414 −129
Open

Extract a by-reference Request body after the other init members are read #48493

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
43d575e
Extract a by-reference Request body after the other init members are …
robobun Nov 10, 2026
fffca97
Keep a Headers init value alive while a by-reference body is extracted
robobun Nov 10, 2026
89f60f1
Shorten the comments on the deferred body extraction
robobun Nov 10, 2026
f0ee09b
Pin the error messages in the body ordering tests, cover a DataView w…
robobun Nov 10, 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
24 changes: 24 additions & 0 deletions src/runtime/webcore/Body.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -910,6 +910,30 @@ impl Value {
Ok(stream_value)
}

/// A `body` init that [`from_js`](Self::from_js) reads by reference, so the
/// fetch spec extracts it after the other init members are converted.
Comment thread
robobun marked this conversation as resolved.
Resolved
pub fn is_extracted_by_reference(value: JSValue) -> bool {
if !value.is_cell() || value.is_string() {
return false;
}
let js_type = value.js_type();
if js_type.is_array_buffer_like()
|| matches!(js_type, jsc::JSType::Array | jsc::JSType::DerivedArray)
{
return true;
}
if DOMFormData::from_js(value).is_some() || URLSearchParams::from_js(value).is_some() {
return true;
}
if js_type == jsc::JSType::DOMWrapper
&& (value.as_class_ref::<Blob>().is_some()
|| value.as_class_ref::<crate::image::Image>().is_some())
{
return true;
}
ReadableStream::is_readable_stream(value)
Comment thread
robobun marked this conversation as resolved.
Resolved
}

pub fn from_js(global_this: &JSGlobalObject, value: JSValue) -> JsResult<Value> {
value.ensure_still_alive();

Expand Down
58 changes: 39 additions & 19 deletions src/runtime/webcore/Request.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1055,6 +1055,10 @@ impl Request {
let values_to_try =
&values_to_try_[0..((!is_first_argument_a_url) as usize + init.is_object() as usize)];

// A by-reference `body`, extracted after the other members and the URL check.
let mut body_init: Option<JSValue> = None;
let mut keepalive_with_stream_body = false;

for &value in values_to_try {
let value_type = value.js_type();
let explicit_check = values_to_try.len() == 2
Expand Down Expand Up @@ -1182,34 +1186,34 @@ impl Request {
match value.fast_get(cx.global(), bun_jsc::BuiltinName::Body) {
Ok(Some(body_)) => {
fields.insert(Fields::Body);
// fetch spec Request(init): `keepalive: true` with a ReadableStream
// body throws before body extraction (Node's message is "keepalive").
if crate::webcore::ReadableStream::is_readable_stream(body_) {
match value.get(cx.global(), "keepalive") {
Ok(Some(keepalive)) if keepalive.to_boolean() => {
bail!(Err(cx
.global()
.throw_type_error(format_args!("keepalive"))));
if BodyValue::is_extracted_by_reference(body_) {
body_init = Some(body_);
// `keepalive: true` with a stream body throws "keepalive" (as Node).
if crate::webcore::ReadableStream::is_readable_stream(body_) {
match value.get(cx.global(), "keepalive") {
Ok(Some(keepalive)) => {
keepalive_with_stream_body = keepalive.to_boolean();
}
Ok(None) => {}
Err(e) => bail!(Err(e)),
}
}
} else {
match BodyValue::from_js(cx.global(), body_) {
Ok(v) => {
*req.body_value_mut() = v;
}
Ok(_) => {}
Err(e) => bail!(Err(e)),
}
}
match BodyValue::from_js(cx.global(), body_) {
Ok(v) => {
*req.body_value_mut() = v;
// BodyValue::from_js() throws without returning Err; see Blob::from_dom_form_data
if cx.global().has_exception() {
bail!(Err(JsError::Thrown));
}
Err(e) => bail!(Err(e)),
}
}
Ok(None) => {}
Err(e) => bail!(Err(e)),
}

// BodyValue::from_js() throws without returning Err; see Blob::from_dom_form_data
if cx.global().has_exception() {
bail!(Err(JsError::Thrown));
}
}

if !fields.contains(Fields::Url) {
Expand Down Expand Up @@ -1379,6 +1383,22 @@ impl Request {

req.url.set(href);

if let Some(body_) = body_init {
if keepalive_with_stream_body {
bail!(Err(cx.global().throw_type_error(format_args!("keepalive"))));
}
match BodyValue::from_js(cx.global(), body_) {
Ok(v) => {
*req.body_value_mut() = v;
}
Err(e) => bail!(Err(e)),
}
// BodyValue::from_js() throws without returning Err; see Blob::from_dom_form_data
if cx.global().has_exception() {
bail!(Err(JsError::Thrown));
}
}

if matches!(req.body_value(), BodyValue::Blob(_)) && req.headers.get().is_some() {
if let BodyValue::Blob(blob) = req.body_value() {
let ct: &[u8] = blob.content_type_slice();
Expand Down
225 changes: 115 additions & 110 deletions src/runtime/webcore/fetch.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -986,30 +986,108 @@ fn fetch_impl<const ALLOW_GET_BODY: bool>(
break 'extract_signal None;
};

// We do this 2nd to last instead of last so that if it's a FormData
// object, we can still insert the boundary.
//
// We must always get the Body before the Headers That way, we can set
// the Content-Type header from the Blob if no Content-Type header is
// set in the Headers
//
// which is important for FormData.
// https://github.com/oven-sh/bun/issues/2264
//
// body: BodyInit | null | undefined;
//
let mut body = 'extract_body: {
// A by-reference body is extracted after `headers` is read and converted.
let init_body: Option<JSValue> = 'brk: {
if let Some(options) = options_object {
if let Some(body__) = options.fast_get(global_this, jsc::BuiltinName::Body)? {
if !body__.is_undefined() {
break 'extract_body Some(HTTPRequestBody::from_js(
&ctx.js_thread(context),
body__,
)?);
break 'brk Some(body__);
}
}
}

if let Some(req) = request_init_object {
if let Some(body__) = req.fast_get(global_this, jsc::BuiltinName::Body)? {
if !body__.is_undefined() {
break 'brk Some(body__);
}
}
}

break 'brk None;
};

let eager_body = match init_body {
Some(body__) if !BodyValue::is_extracted_by_reference(body__) => {
Some(HTTPRequestBody::from_js(&ctx.js_thread(context), body__)?)
}
_ => None,
};

// HTTPRequestBody::from_js() throws without returning Err; see Blob::from_dom_form_data
if global_this.has_exception() {
return Err(jsc::JsError::Thrown);
}

// headers: Headers | undefined;
//
// Holds the +1 from `create_from_js` until the headers are applied.
Comment thread
robobun marked this conversation as resolved.
Resolved
Comment thread
robobun marked this conversation as resolved.
Resolved
Comment thread
robobun marked this conversation as resolved.
Resolved
let mut created_headers: Option<HeadersRef> = None;
// Roots a borrowed `Headers` wrapper across the body extraction, which can run JS.
let mut headers_guard: Option<jsc::EnsureStillAlive> = None;
let fetch_headers: Option<*mut FetchHeaders> = 'brk: {
let init_headers: Option<JSValue> = 'init: {
if let Some(options) = options_object {
if let Some(headers_value) =
options.fast_get(global_this, jsc::BuiltinName::Headers)?
{
if !headers_value.is_undefined() {
break 'init Some(headers_value);
}
}
}

if let Some(req) = request_ref!() {
if let Some(head) = req.get_fetch_headers_unless_empty() {
break 'brk Some(head.as_ptr());
}
break 'brk None;
}

if let Some(options) = request_init_object {
if let Some(headers_value) =
options.fast_get(global_this, jsc::BuiltinName::Headers)?
{
if !headers_value.is_undefined() {
break 'init Some(headers_value);
}
}
}

break 'init None;
};

let Some(headers_value) = init_headers else {
break 'brk None;
};

if let Some(headers__) = FetchHeaders::cast(headers_value) {
// `FetchHeaders` is an opaque ZST FFI handle (S008) — safe deref.
if bun_opaque::opaque_deref_mut(headers__.as_ptr()).is_empty() {
break 'brk None;
}
headers_guard = Some(jsc::EnsureStillAlive(headers_value));
break 'brk Some(headers__.as_ptr());
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

if let Some(headers__) = HeadersRef::create_from_js(ctx, headers_value)? {
break 'brk Some(created_headers.insert(headers__).as_ptr());
}

break 'brk None;
};

let mut body = 'extract_body: {
if eager_body.is_some() {
break 'extract_body eager_body;
}

if let Some(body__) = init_body {
break 'extract_body Some(HTTPRequestBody::from_js(&ctx.js_thread(context), body__)?);
Comment thread
claude[bot] marked this conversation as resolved.
Resolved
}

if let Some(req) = request_ref!() {
let body_value = req.get_body_value();
let already_used = match body_value {
Expand Down Expand Up @@ -1075,17 +1153,6 @@ fn fetch_impl<const ALLOW_GET_BODY: bool>(
));
}

if let Some(req) = request_init_object {
if let Some(body__) = req.fast_get(global_this, jsc::BuiltinName::Body)? {
if !body__.is_undefined() {
break 'extract_body Some(HTTPRequestBody::from_js(
&ctx.js_thread(context),
body__,
)?);
}
}
}

break 'extract_body None;
}
.unwrap_or_default();
Expand All @@ -1095,94 +1162,32 @@ fn fetch_impl<const ALLOW_GET_BODY: bool>(
return Err(jsc::JsError::Thrown);
}

// headers: Headers | undefined;
headers = 'extract_headers: {
// Holds the +1 from `create_from_js` until this block exits.
let mut created_headers: Option<HeadersRef> = None;

let fetch_headers: Option<*mut FetchHeaders> = 'brk: {
if let Some(options) = options_object {
if let Some(headers_value) =
options.fast_get(global_this, jsc::BuiltinName::Headers)?
{
if !headers_value.is_undefined() {
if let Some(headers__) = FetchHeaders::cast(headers_value) {
// `FetchHeaders` is an opaque ZST FFI handle (S008) — safe deref.
if bun_opaque::opaque_deref_mut(headers__.as_ptr()).is_empty() {
break 'brk None;
}
break 'brk Some(headers__.as_ptr());
}

if let Some(headers__) = HeadersRef::create_from_js(ctx, headers_value)? {
break 'brk Some(created_headers.insert(headers__).as_ptr());
}

break 'brk None;
}
}
}

if let Some(req) = request_ref!() {
if let Some(head) = req.get_fetch_headers_unless_empty() {
break 'brk Some(head.as_ptr());
}
break 'brk None;
}

if let Some(options) = request_init_object {
if let Some(headers_value) =
options.fast_get(global_this, jsc::BuiltinName::Headers)?
{
if !headers_value.is_undefined() {
if let Some(headers__) = FetchHeaders::cast(headers_value) {
// `FetchHeaders` is an opaque ZST FFI handle (S008) — safe deref.
if bun_opaque::opaque_deref_mut(headers__.as_ptr()).is_empty() {
break 'brk None;
}
break 'brk Some(headers__.as_ptr());
}

if let Some(headers__) = HeadersRef::create_from_js(ctx, headers_value)? {
break 'brk Some(created_headers.insert(headers__).as_ptr());
}

break 'brk None;
}
}
}

break 'extract_headers headers;
};

let result = if let Some(headers_) = fetch_headers {
// `headers_` points to a live FetchHeaders (either JS-owned or
// refcounted via `created_headers` above). `FetchHeaders` is
// an opaque ZST FFI handle (S008) — safe `*mut → &mut` deref.
let headers_ref = bun_opaque::opaque_deref_mut(headers_);
if url.is_s3() {
if let Some(range_) = headers_ref.fast_get(HTTPHeaderName::Range) {
range = Some(range_.to_owned_slice_z());
}
// A Blob or FormData Content-Type is applied after the body is extracted (#2264).
if let Some(headers_) = fetch_headers {
// `headers_` points to a live FetchHeaders (either JS-owned or
// refcounted via `created_headers` above). `FetchHeaders` is
// an opaque ZST FFI handle (S008) — safe `*mut → &mut` deref.
Comment thread
robobun marked this conversation as resolved.
Resolved
let headers_ref = bun_opaque::opaque_deref_mut(headers_);
if url.is_s3() {
if let Some(range_) = headers_ref.fast_get(HTTPHeaderName::Range) {
range = Some(range_.to_owned_slice_z());
}
}

if let Some(upgrade_) = headers_ref.fast_get(HTTPHeaderName::Upgrade) {
if http::upgrade_header_is_not_h2(upgrade_.to_utf8().slice()) {
upgraded_connection = true;
}
if let Some(upgrade_) = headers_ref.fast_get(HTTPHeaderName::Upgrade) {
if http::upgrade_header_is_not_h2(upgrade_.to_utf8().slice()) {
upgraded_connection = true;
}
}

Some(from_fetch_headers(
global_this,
Some(headers_ref),
any_blob_content_type_opt(body.get_any_blob().map(|b| &*b)),
)?)
} else {
headers
};

break 'extract_headers result;
};
headers = Some(from_fetch_headers(
global_this,
Some(headers_ref),
any_blob_content_type_opt(body.get_any_blob().map(|b| &*b)),
)?);
}
drop(headers_guard);
drop(created_headers);

if proxy.is_some() && !unix_socket_path.is_empty() {
let err = ctx.to_type_error(
Expand Down
Loading
Loading