diff --git a/src/runtime/webcore/Blob.rs b/src/runtime/webcore/Blob.rs index 3e9b5efb447a..c9138ced112a 100644 --- a/src/runtime/webcore/Blob.rs +++ b/src/runtime/webcore/Blob.rs @@ -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 where Self: Sized; fn content_type(&self) -> &[u8]; @@ -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 { // "----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]; @@ -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. @@ -951,12 +960,12 @@ impl BlobExt for Blob { (&raw mut context).cast::(), 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"--"); @@ -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] { @@ -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, } /// Which piece of a `multipart/form-data` entry a string is, selecting the @@ -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; @@ -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()); diff --git a/src/runtime/webcore/Body.rs b/src/runtime/webcore/Body.rs index 93041722ba08..ac35ec747793 100644 --- a/src/runtime/webcore/Body.rs +++ b/src/runtime/webcore/Body.rs @@ -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 @@ -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) { diff --git a/src/runtime/webcore/fetch/FetchTasklet.rs b/src/runtime/webcore/fetch/FetchTasklet.rs index 1e8ea8278f0c..cc22ec8a0f01 100644 --- a/src/runtime/webcore/fetch/FetchTasklet.rs +++ b/src/runtime/webcore/fetch/FetchTasklet.rs @@ -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}; @@ -211,6 +213,18 @@ impl HTTPRequestBody { } pub fn from_js(global_this: &JSGlobalObject, value: JSValue) -> JsResult { + 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))) diff --git a/test/js/bun/http/fetch-file-upload.test.ts b/test/js/bun/http/fetch-file-upload.test.ts index 38ea1ae6c323..19585ccd5021 100644 --- a/test/js/bun/http/fetch-file-upload.test.ts +++ b/test/js/bun/http/fetch-file-upload.test.ts @@ -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): Promise { + 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 }); + } + }); +});