Skip to content
Closed
Show file tree
Hide file tree
Changes from 4 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
5 changes: 5 additions & 0 deletions src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1526,6 +1526,11 @@ JSC_DEFINE_HOST_FUNCTION(jsWebStreamsHandler_onReadableStreamToFormDataFulfilled
auto scope = DECLARE_THROW_SCOPE(vm);
JSValue blob = callFrame->argument(0);
JSValue contentType = callFrame->argument(1);
if (contentType.isFalse()) {
// Sentinel from Body.formData() when the MIME check failed: stream is drained, now reject.
Bun::throwError(globalObject, scope, Bun::ErrorCode::ERR_FORMDATA_PARSE_ERROR, "Can't decode form data from body because of incorrect MIME type/boundary"_s);
return {};
}
JSValue constructor = JSDOMFormData::getConstructor(vm, globalObject);
JSValue fromFunction = constructor.get(globalObject, vm.propertyNames->from);
RETURN_IF_EXCEPTION(scope, {});
Expand Down
68 changes: 34 additions & 34 deletions src/runtime/webcore/Body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,8 @@ use crate::jsc::HTTPHeaderName;
pub use crate::webcore::InternalBlob;
use crate::webcore::form_data::AsyncFormDataExt as _;
use crate::webcore::sink::{self, ArrayBufferSink};
use bun_core::{MutableString, String as BunString, ZigString};
use bun_core::{MutableString, String as BunString};
use bun_core::{WTFStringImpl, WTFStringImplExt as _, WTFStringImplStruct};
use bun_jsc::ZigStringJsc as _;
use bun_jsc::{JsCell, StringJsc as _};

/// Deref the `Value::WTFStringImpl` / `AnyBlob::WTFStringImpl` payload.
Expand Down Expand Up @@ -388,15 +387,16 @@ impl PendingValue {
Action::GetText => global_this.readable_stream_to_text(readable.value),
Action::GetBlob => global_this.readable_stream_to_blob(readable.value),
Action::GetFormData(form_data) => 'brk: {
let fd = form_data.take().unwrap();
// defer: form_data already taken; action.getFormData = None handled by take()
let encoding_js = match &fd.encoding {
bun_core::form_data::Encoding::Multipart(multipart) => {
BunString::init(&multipart[..]).to_js(global_this)?
}
bun_core::form_data::Encoding::URLEncoded => JSValue::UNDEFINED,
let encoding_js = match form_data.take() {
// `false` survives the reaction-context plumbing; null/undefined do not.
None => JSValue::FALSE,
Some(fd) => match &fd.encoding {
bun_core::form_data::Encoding::Multipart(multipart) => {
BunString::init(&multipart[..]).to_js(global_this)?
}
bun_core::form_data::Encoding::URLEncoded => JSValue::UNDEFINED,
},
};
// fd dropped at end of scope (Box<AsyncFormData> -> Drop)
break 'brk global_this
.readable_stream_to_form_data(readable.value, encoding_js);
}
Expand Down Expand Up @@ -1065,14 +1065,8 @@ impl Value {
Action::GetFormData(form_data_slot) => 'inner: {
let mut blob = new.use_as_any_blob();
let Some(async_form_data) = form_data_slot.take() else {
// `blob.detach()` below covers the reject error path too.
let r = promise.reject(
global,
ZigString::init(
b"Internal error: task for FormData must not be null",
)
.to_error_instance(global),
);
// MIME check failed up front; body is consumed above, now reject.
let r = promise.reject(global, form_data_mime_error(global));
blob.detach();
r?;
break 'inner;
Expand Down Expand Up @@ -1993,17 +1987,8 @@ pub(crate) trait BodyMixin: BodyOwnerJs + Sized {
}
}

let Some(encoder) = self.get_form_data_encoding()? else {
// TODO: catch specific errors from getFormDataEncoding
return Ok(global_object
.err(
jsc::ErrorCode::FORMDATA_PARSE_ERROR,
format_args!(
"Can't decode form data from body because of incorrect MIME type/boundary"
),
)
.reject());
};
// Spec "consume body" fully reads before the MIME check, so consume even when encoder is None.
let encoder = self.get_form_data_encoding()?;

let value = self.get_body_value();
if let Value::Locked(_locked) = value {
Expand All @@ -2013,14 +1998,18 @@ pub(crate) trait BodyMixin: BodyOwnerJs + Sized {
let Value::Locked(locked) = value else {
unreachable!()
};
return locked.set_promise(
global_object,
Action::GetFormData(Some(encoder)),
owned_readable,
);
return locked.set_promise(global_object, Action::GetFormData(encoder), owned_readable);
}

let mut blob: AnyBlob = value.use_as_any_blob();
let Some(encoder) = encoder else {
blob.detach();
return Ok(JSPromise::rejected_promise(
global_object,
form_data_mime_error(global_object),
)
.to_js());
};
// `encoder.encoding` is `bun_core::form_data::Encoding`; convert
// to the `webcore::form_data::Encoding` shape FormData::to_js expects.
let encoding = match encoder.encoding {
Expand Down Expand Up @@ -2142,6 +2131,17 @@ fn handle_body_already_used(global_object: &JSGlobalObject) -> JSValue {
.reject()
}

fn form_data_mime_error(global_object: &JSGlobalObject) -> JSValue {
global_object
.err(
jsc::ErrorCode::FORMDATA_PARSE_ERROR,
format_args!(
"Can't decode form data from body because of incorrect MIME type/boundary"
),
)
.to_js()
}

/// If the body already failed, reject the read with that error. Every body
/// reader must call this before its `Locked` handling: `Value::Error` would
/// otherwise fall through to `use_as_any_blob_*` and resolve empty.
Expand Down
76 changes: 76 additions & 0 deletions test/js/web/fetch/body.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -749,6 +749,82 @@ describe.concurrent("string body consumption does not leak", () => {
}
});

// https://fetch.spec.whatwg.org/#concept-body-consume-body
// The spec fully reads the body before packageData runs (and may throw),
// so a rejected formData() must still consume the body. Node/undici agree:
// bodyUsed becomes true and further reads reject with TypeError.
describe("formData() rejecting on MIME type still consumes the body", () => {
const stringStream = () =>
new ReadableStream({
start(c) {
c.enqueue(new TextEncoder().encode("hello world"));
c.close();
},
});

for (const { body, fn } of bodyTypes) {
describe(body.name, () => {
test.each([
["string", () => fn("hello world")],
["Uint8Array", () => fn(new TextEncoder().encode("hello world"))],
["Blob", () => fn(new Blob(["hello world"]))],
["ReadableStream", () => fn(stringStream(), { "content-type": "text/plain" })],
["application/json", () => fn(JSON.stringify({ a: 1 }), { "content-type": "application/json" })],
])("%s", async (_, make) => {
const r = make();
await expect(r.formData()).rejects.toThrow(TypeError);
expect(r.bodyUsed).toBe(true);
await expect(r.text()).rejects.toThrow(TypeError);
await expect(r.json()).rejects.toThrow(TypeError);
await expect(r.arrayBuffer()).rejects.toThrow(TypeError);
await expect(r.formData()).rejects.toThrow(TypeError);
});

// Null body: spec says body is null so fully-read is vacuous; Node leaves
// bodyUsed false and subsequent text() resolves "".
test("null body stays unconsumed", async () => {
const r = fn();
await expect(r.formData()).rejects.toThrow(TypeError);
expect(r.bodyUsed).toBe(false);
expect(await r.text()).toBe("");
});
});
}

test("Bun.serve request body", async () => {
await using server = Bun.serve({
port: 0,
async fetch(req) {
let fd: string;
try {
await req.formData();
fd = "RESOLVED";
} catch (e) {
fd = "REJECTED " + (e as Error).constructor.name;
}
const bodyUsed = req.bodyUsed;
let text: string;
try {
text = "RESOLVED " + (await req.text());
} catch (e) {
text = "REJECTED " + (e as Error).constructor.name;
}
return Response.json({ fd, bodyUsed, text });
},
});
const res = await fetch(`http://localhost:${server.port}/`, {
method: "POST",
body: "hello server",
headers: { "content-type": "text/plain" },
});
expect(await res.json()).toEqual({
fd: "REJECTED TypeError",
bodyUsed: true,
text: "REJECTED TypeError",
});
});
});

// https://github.com/oven-sh/bun/issues/6860
describe("constructing a body from an unusable ReadableStream", () => {
const bytes = () =>
Expand Down
Loading