Skip to content
Open
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
42 changes: 24 additions & 18 deletions src/runtime/webcore/Blob.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,14 @@ pub trait BlobExt {
) -> Blob
where
Self: Sized;
fn from_dom_form_data(global_this: &JSGlobalObject, form_data: &mut jsc::DOMFormData) -> Blob
/// Serializes `form_data` as `multipart/form-data`, reading its file-backed
/// entries eagerly. Returns the first read failure instead of throwing it:
/// `Request`/`Response` construction throws it as-is, while a `fetch()`
/// body that cannot be read is a network error and rejects as a `TypeError`.
fn from_dom_form_data(
global_this: &JSGlobalObject,
form_data: &mut jsc::DOMFormData,
) -> Result<Blob, bun_sys::Error>
where
Self: Sized;
fn content_type(&self) -> &[u8];
Expand Down Expand Up @@ -869,7 +876,10 @@ impl BlobExt for Blob {
blob
}

fn from_dom_form_data(global_this: &JSGlobalObject, form_data: &mut jsc::DOMFormData) -> Blob {
fn from_dom_form_data(
global_this: &JSGlobalObject,
form_data: &mut jsc::DOMFormData,
) -> Result<Blob, bun_sys::Error> {
// "----WebKitFormBoundary" (22 bytes) + 32 lowercase-hex chars of a fresh UUID.
const BOUNDARY_PREFIX: &[u8; 22] = b"----WebKitFormBoundary";
let mut boundary_buf = [0u8; BOUNDARY_PREFIX.len() + 32];
Expand All @@ -884,8 +894,7 @@ impl BlobExt for Blob {
let mut context = FormDataContext {
joiner: bun_core::string_joiner::StringJoiner::default(),
boundary,
failed: false,
global_this,
read_error: None,
};
// Size the node list up front: a file entry pushes at most 13 slices, a
// string entry 8, plus 3 for the closing boundary.
Expand Down Expand Up @@ -951,12 +960,12 @@ impl BlobExt for Blob {
(&raw mut context).cast::<c_void>(),
for_each_thunk,
);
if context.failed {
if let Some(err) = context.read_error {
// Drop the joiner (Drop runs StringJoiner::deinit) so every
// heap-owned slice already pushed — escaped names, non-ASCII
// conversions, NodeFS read_file result buffers — is freed.
drop(context.joiner);
return Blob::init_empty(global_this);
return Err(err);
}

context.joiner.push_static(b"--");
Expand All @@ -979,7 +988,7 @@ impl BlobExt for Blob {
.set(BlobContentType::Owned(std::sync::Arc::from(ct)));
blob.content_type_was_set.set(true);

blob
Ok(blob)
}

fn content_type(&self) -> &[u8] {
Expand Down Expand Up @@ -3859,14 +3868,14 @@ where
// ──────────────────────────────────────────────────────────────────────────

/// Stack-local helper for `Blob::from_dom_form_data`. `boundary` borrows the
/// caller's `hex_buf` and `global_this` borrows the incoming `&JSGlobalObject`;
/// both strictly outlive this struct, so they are stored as plain references
/// rather than raw pointers.
/// caller's `boundary_buf`, which strictly outlives this struct, so it is
/// stored as a plain reference rather than a raw pointer.
struct FormDataContext<'a> {
joiner: StringJoiner<'a>,
boundary: &'a [u8], // borrowed; outlives the joiner
failed: bool,
global_this: &'a JSGlobalObject,
/// First file-backed entry that could not be read; once set, the remaining
/// entries are skipped and `from_dom_form_data` returns it.
read_error: Option<bun_sys::Error>,
}

/// Which piece of a `multipart/form-data` entry a string is, selecting the
Expand Down Expand Up @@ -3959,12 +3968,11 @@ impl FormDataContext<'_> {
}

fn on_entry(&mut self, name: ZigString, entry: FormDataEntry<'_>) {
if self.failed {
if self.read_error.is_some() {
return;
}
// Copy the borrowed refs out first (disjoint-field reads) so the
// Copy the borrowed ref out first (disjoint-field reads) so the
// long-lived `&mut self.joiner` below doesn't conflict.
let global_this = self.global_this;
let boundary = self.boundary;
let joiner = &mut self.joiner;

Expand Down Expand Up @@ -4030,9 +4038,7 @@ impl FormDataContext<'_> {
let res = node_fs.read_file(&rf_args, crate::node::fs::Flavor::Sync);
match res {
Err(err) => {
self.failed = true;
let js_err = err.to_js(global_this);
let _ = global_this.throw_value(js_err);
self.read_error = Some(err);
}
Ok(mut result) => {
joiner.push_cloned(result.slice());
Expand Down
9 changes: 5 additions & 4 deletions src/runtime/webcore/Body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use crate::webcore::form_data::AsyncFormDataExt as _;
use bun_core::{String as BunString, ZigString};
use bun_core::{WTFStringImpl, WTFStringImplExt as _, WTFStringImplStruct};
use bun_jsc::ZigStringJsc as _;
use bun_jsc::{JsCell, StringJsc as _};
use bun_jsc::{JsCell, StringJsc as _, SysErrorJsc as _};

/// Deref the `Value::WTFStringImpl` / `AnyBlob::WTFStringImpl` payload.
/// Centralises the per-site `(**s)` raw deref at the dozen `match` arms below
Expand Down Expand Up @@ -973,9 +973,10 @@ impl Value {

if let Some(form_data) = as_dom_form_data(value) {
// SAFETY: shim returns a live JSC heap cell.
return Ok(Value::Blob(Blob::from_dom_form_data(global_this, unsafe {
&mut *form_data
})));
return match Blob::from_dom_form_data(global_this, unsafe { &mut *form_data }) {
Ok(blob) => Ok(Value::Blob(blob)),
Err(err) => Err(err.throw(global_this)),
};
}

if let Some(search_params) = as_url_search_params(value) {
Expand Down
16 changes: 15 additions & 1 deletion src/runtime/webcore/fetch/FetchTasklet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ use bun_threading::Mutex;
use bun_url::URL as ZigURL;

use crate::api::bun_x509 as X509;
use crate::webcore::blob::{Any as AnyBlob, Blob, SizeType as BlobSizeType, Store as BlobStore};
use crate::webcore::blob::{
Any as AnyBlob, Blob, BlobExt as _, SizeType as BlobSizeType, Store as BlobStore,
};
use crate::webcore::body::{self, Body, Value as BodyValue, ValueError as BodyValueError};
use crate::webcore::fetch::fetch_request_body_sink::{FetchRequestBodySink, RequestBodyChunk};
use crate::webcore::readable_stream::{ReadableStream, Strong as ReadableStreamStrong};
Expand Down Expand Up @@ -211,6 +213,18 @@ impl HTTPRequestBody {
}

pub fn from_js(global_this: &JSGlobalObject, value: JSValue) -> JsResult<HTTPRequestBody> {
if let Some(form_data) = jsc::DOMFormData::from_js(value) {
return match Blob::from_dom_form_data(global_this, form_data) {
Ok(blob) => Ok(HTTPRequestBody::AnyBlob(AnyBlob::Blob(blob))),
// A request body that cannot be read is a network error, so it
// rejects the way fetch() reports those: a TypeError that still
// carries `code`/`syscall`/`path`.
Err(err) => {
let err: jsc::SystemError = err.to_system_error().into();
Err(global_this.throw_value(err.to_type_error_instance(global_this)))
}
};
}
let mut body_value = BodyValue::from_js(global_this, value)?;
if matches!(body_value, BodyValue::Used)
|| (matches!(&body_value, BodyValue::Locked(l) if !l.action.is_none() || l.is_disturbed2(global_this)))
Expand Down
88 changes: 88 additions & 0 deletions test/js/bun/http/fetch-file-upload.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -239,3 +239,91 @@ test("missing file throws the expected error", async () => {
});
Bun.gc(true);
});

// A request body that cannot be read is a network error, so fetch() rejects the
// way it rejects its other network errors: a TypeError that still carries the
// system error fields.
describe("FormData body with a Bun.file() that cannot be read", () => {
// The FormData is serialized before anything is connected, so the promise
// comes back already rejected; checking that pins the rejection to the body
// rather than to the connection this URL would refuse.
async function bodyRejection(promise: Promise<Response>): Promise<any> {
expect(Bun.peek.status(promise)).toBe("rejected");
return await promise.then(
() => {
throw new Error("fetch() resolved");
},
error => error,
);
}

test.concurrent("fetch(url, { body }) rejects with a TypeError carrying the ENOENT", async () => {
using dir = tempDir("fetch-formdata-missing-file", { "present.txt": "present" });
const missingPath = join(String(dir), "missing.txt");

const body = new FormData();
body.append("field", "value");
body.append("present", Bun.file(join(String(dir), "present.txt")));
body.append("missing", Bun.file(missingPath));

const error = await bodyRejection(fetch("http://127.0.0.1:1/", { method: "POST", body }));
expect(error).toBeInstanceOf(TypeError);
expect(error).toMatchObject({
name: "TypeError",
code: "ENOENT",
syscall: "open",
path: missingPath,
errno: expect.any(Number),
});
expect(error.message).toContain("no such file or directory");
});

test.concurrent("fetch({ url, body }) rejects with the same TypeError", async () => {
using dir = tempDir("fetch-formdata-missing-file-init", {});
const missingPath = join(String(dir), "missing.txt");

const body = new FormData();
body.append("missing", Bun.file(missingPath));

const error = await bodyRejection(fetch({ url: "http://127.0.0.1:1/", method: "POST", body } as any));
expect(error).toBeInstanceOf(TypeError);
expect(error).toMatchObject({ code: "ENOENT", syscall: "open", path: missingPath });
});

// Opening a directory succeeds and reading it fails, so this covers the error
// coming out of the read itself. Not pinned on Windows, where a directory
// stats as 0 bytes and the size-bounded read need not fail the same way.
test.concurrent.skipIf(isWindows)("a read failure (EISDIR) rejects with a TypeError too", async () => {
using dir = tempDir("fetch-formdata-directory", { "subdir/.keep": "" });

const body = new FormData();
body.append("dir", Bun.file(join(String(dir), "subdir")));

const error = await bodyRejection(fetch("http://127.0.0.1:1/", { method: "POST", body }));
expect(error).toBeInstanceOf(TypeError);
expect(error).toMatchObject({ code: "EISDIR", syscall: "read" });
});

test.concurrent("new Request() and new Response() still throw the system error itself", () => {
using dir = tempDir("fetch-formdata-missing-file-ctor", {});
const missingPath = join(String(dir), "missing.txt");

for (const construct of [
(body: FormData) => new Response(body),
(body: FormData) => new Request("http://localhost/", { method: "POST", body }),
]) {
const body = new FormData();
body.append("missing", Bun.file(missingPath));

let error: unknown;
try {
construct(body);
} catch (e) {
error = e;
}
expect(error).toBeInstanceOf(Error);
expect(error).not.toBeInstanceOf(TypeError);
expect(error).toMatchObject({ name: "Error", code: "ENOENT", syscall: "open", path: missingPath });
}
});
});