diff --git a/src/runtime/webcore/ArrayBufferSink.rs b/src/runtime/webcore/ArrayBufferSink.rs index 4f0bd90b1a4..91a2cbabc70 100644 --- a/src/runtime/webcore/ArrayBufferSink.rs +++ b/src/runtime/webcore/ArrayBufferSink.rs @@ -260,9 +260,8 @@ impl crate::webcore::sink::JsSinkType for ArrayBufferSink { const HAS_FLUSH_FROM_JS: bool = true; const START_TAG: Option = Some(streams::StartTag::ArrayBufferSink); - fn memory_cost(&self) -> usize { - Self::memory_cost(self) - } + crate::impl_js_sink_forwarders!(); + fn finalize(&mut self) { // The `JSSink::finalize` C export owns destroying the heap // allocation; the trait impl here is the *inner* finalize. @@ -271,18 +270,6 @@ impl crate::webcore::sink::JsSinkType for ArrayBufferSink { fn construct(this: &mut core::mem::MaybeUninit) { Self::construct(this); } - fn write_bytes(&mut self, data: &streams::Result) -> streams::result::Writable { - Self::write(self, data) - } - fn write_utf16(&mut self, data: &streams::Result) -> streams::result::Writable { - Self::write_utf16(self, data) - } - fn write_latin1(&mut self, data: &streams::Result) -> streams::result::Writable { - Self::write_latin1(self, data) - } - fn end(&mut self, err: Option) -> bun_sys::Result<()> { - Self::end(self, err) - } fn end_from_js(&mut self, global: &JSGlobalObject) -> bun_sys::Result { match Self::end_from_js(self, global) { bun_sys::Result::Ok(ab) => bun_sys::Result::Ok(match ab.to_js_unchecked(global) { @@ -292,15 +279,6 @@ impl crate::webcore::sink::JsSinkType for ArrayBufferSink { bun_sys::Result::Err(e) => bun_sys::Result::Err(e), } } - fn flush(&mut self) -> bun_sys::Result<()> { - Self::flush(self) - } - fn flush_from_js(&mut self, global: &JSGlobalObject, wait: bool) -> bun_sys::Result { - Self::flush_from_js(self, global, wait) - } - fn start(&mut self, config: streams::Start) -> bun_sys::Result<()> { - Self::start(self, &config) - } fn signal(&mut self) -> Option<&mut Signal> { Some(&mut self.signal) } diff --git a/src/runtime/webcore/Blob.rs b/src/runtime/webcore/Blob.rs index 9f75d4c46de..5612be20283 100644 --- a/src/runtime/webcore/Blob.rs +++ b/src/runtime/webcore/Blob.rs @@ -1337,31 +1337,7 @@ impl BlobExt for Blob { } if let Some(content_type) = options_object.get_truthy(global_this, "type")? { // override the content type - if !content_type.is_string() { - return Err(global_this.throw_invalid_argument_type( - "write", - "options.type", - "string", - )); - } - let content_type_str = content_type.to_slice(global_this)?; - let slice = content_type_str.slice(); - if is_valid_blob_type(slice) { - self.free_content_type(); - self.content_type_was_set.set(true); - - // SAFETY: bun_vm() never returns null for a Bun-owned global. - if let Some(mime) = global_this.bun_vm().as_mut().mime_type(slice) { - self.content_type - .set(std::ptr::from_ref::<[u8]>(mime.value.as_ref())); - } else { - let mut buf = vec![0u8; slice.len()]; - strings::copy_lowercase(slice, &mut buf); - self.content_type - .set(bun_core::heap::into_raw(buf.into_boxed_slice())); - self.content_type_allocated.set(true); - } - } + set_content_type_from_js(global_this, self, content_type)?; } } else if !options_object.is_empty_or_undefined_or_null() { return Err(global_this.throw_invalid_argument_type("write", "options", "object")); @@ -1767,30 +1743,7 @@ impl BlobExt for Blob { let options = arg0; if let Some(content_type) = options.get_truthy(global_this, "type")? { // override the content type - if !content_type.is_string() { - return Err(global_this.throw_invalid_argument_type( - "write", - "options.type", - "string", - )); - } - let content_type_str = content_type.to_slice(global_this)?; - let slice = content_type_str.slice(); - if is_valid_blob_type(slice) { - self.free_content_type(); - self.content_type_was_set.set(true); - // SAFETY: see other `mime_type` call sites. - if let Some(mime) = global_this.bun_vm().as_mut().mime_type(slice) { - self.content_type - .set(std::ptr::from_ref::<[u8]>(mime.value.as_ref())); - } else { - let mut buf = vec![0u8; slice.len()]; - strings::copy_lowercase(slice, &mut buf); - self.content_type - .set(bun_core::heap::into_raw(buf.into_boxed_slice())); - self.content_type_allocated.set(true); - } - } + set_content_type_from_js(global_this, self, content_type)?; } let content_disposition_str: Option = @@ -3372,20 +3325,7 @@ impl BlobExt for Blob { } } - jsc::JSType::ArrayBuffer - | jsc::JSType::Int8Array - | jsc::JSType::Uint8Array - | jsc::JSType::Uint8ClampedArray - | jsc::JSType::Int16Array - | jsc::JSType::Uint16Array - | jsc::JSType::Int32Array - | jsc::JSType::Uint32Array - | jsc::JSType::Float16Array - | jsc::JSType::Float32Array - | jsc::JSType::Float64Array - | jsc::JSType::BigInt64Array - | jsc::JSType::BigUint64Array - | jsc::JSType::DataView => { + t if t.is_array_buffer_like() => { return Blob::try_create( top_value.as_array_buffer(global).unwrap().byte_slice(), global, @@ -3520,21 +3460,8 @@ impl BlobExt for Blob { continue; } match item.js_type_loose() { - jsc::JSType::String - | jsc::JSType::ArrayBuffer - | jsc::JSType::Int8Array - | jsc::JSType::Uint8Array - | jsc::JSType::Uint8ClampedArray - | jsc::JSType::Int16Array - | jsc::JSType::Uint16Array - | jsc::JSType::Int32Array - | jsc::JSType::Uint32Array - | jsc::JSType::Float16Array - | jsc::JSType::Float32Array - | jsc::JSType::Float64Array - | jsc::JSType::BigInt64Array - | jsc::JSType::BigUint64Array - | jsc::JSType::DataView => {} + jsc::JSType::String => {} + t if t.is_array_buffer_like() => {} jsc::JSType::DOMWrapper if item.as_class_ref::().is_some() => {} _ => { @@ -3563,20 +3490,7 @@ impl BlobExt for Blob { joiner.push_cloned(sliced.slice()); continue; } - jsc::JSType::ArrayBuffer - | jsc::JSType::Int8Array - | jsc::JSType::Uint8Array - | jsc::JSType::Uint8ClampedArray - | jsc::JSType::Int16Array - | jsc::JSType::Uint16Array - | jsc::JSType::Int32Array - | jsc::JSType::Uint32Array - | jsc::JSType::Float16Array - | jsc::JSType::Float32Array - | jsc::JSType::Float64Array - | jsc::JSType::BigInt64Array - | jsc::JSType::BigUint64Array - | jsc::JSType::DataView => { + t if t.is_array_buffer_like() => { could_have_non_ascii = true; let buf = item.as_array_buffer(global).unwrap(); if parts_can_run_js { @@ -3653,20 +3567,7 @@ impl BlobExt for Blob { } } - jsc::JSType::ArrayBuffer - | jsc::JSType::Int8Array - | jsc::JSType::Uint8Array - | jsc::JSType::Uint8ClampedArray - | jsc::JSType::Int16Array - | jsc::JSType::Uint16Array - | jsc::JSType::Int32Array - | jsc::JSType::Uint32Array - | jsc::JSType::Float16Array - | jsc::JSType::Float32Array - | jsc::JSType::Float64Array - | jsc::JSType::BigInt64Array - | jsc::JSType::BigUint64Array - | jsc::JSType::DataView => { + t if t.is_array_buffer_like() => { let buf = current.as_array_buffer(global).unwrap(); // SAFETY: this arm is only reached when the typed array is the // top-level value (the walk stack is empty), so no user JS runs @@ -5346,6 +5247,36 @@ fn validate_writable_blob(global_this: &JSGlobalObject, blob: &Blob) -> JsResult Ok(()) } +/// Overrides `blob`'s content type from a write-path `options.type` value. +/// Throws if the value is not a string; silently ignores invalid blob types. +fn set_content_type_from_js( + global_this: &JSGlobalObject, + blob: &Blob, + content_type: JSValue, +) -> JsResult<()> { + if !content_type.is_string() { + return Err(global_this.throw_invalid_argument_type("write", "options.type", "string")); + } + let content_type_str = content_type.to_slice(global_this)?; + let slice = content_type_str.slice(); + if is_valid_blob_type(slice) { + blob.free_content_type(); + blob.content_type_was_set.set(true); + // SAFETY: bun_vm() never returns null for a Bun-owned global. + if let Some(mime) = global_this.bun_vm().as_mut().mime_type(slice) { + blob.content_type + .set(std::ptr::from_ref::<[u8]>(mime.value.as_ref())); + } else { + let mut buf = vec![0u8; slice.len()]; + strings::copy_lowercase(slice, &mut buf); + blob.content_type + .set(bun_core::heap::into_raw(buf.into_boxed_slice())); + blob.content_type_allocated.set(true); + } + } + Ok(()) +} + /// `Bun.write(destination, input, options?)` pub fn write_file(global_this: &JSGlobalObject, callframe: &CallFrame) -> JsResult { let arguments = callframe.arguments(); @@ -7338,6 +7269,93 @@ pub trait FileCloser: Sized { } } +/// Implements [`FileCloser`] for a struct with the standard field set +/// (`opened_fd`, `close_after_io`, `state`, `io_request`, `io_poll`, `task`), +/// an inherent `update()`, and a [`bun_io::Tag`] variant named after the type. +/// Requires `bun_threading::intrusive_work_task!` and +/// `bun_io::intrusive_io_request!` on the type for the intrusive-pointer +/// recovery in the two trampolines. +macro_rules! impl_file_closer { + ($T:ident) => { + impl crate::webcore::blob::FileCloser for $T { + const IO_TAG: ::bun_io::Tag = ::bun_io::Tag::$T; + fn opened_fd(&self) -> ::bun_sys::Fd { + self.opened_fd + } + fn set_opened_fd(&mut self, fd: ::bun_sys::Fd) { + self.opened_fd = fd; + } + fn close_after_io(&self) -> bool { + self.close_after_io + } + fn set_close_after_io(&mut self, v: bool) { + self.close_after_io = v; + } + fn state(&self) -> &::core::sync::atomic::AtomicU8 { + &self.state + } + fn io_request(&mut self) -> Option<&mut ::bun_io::Request> { + Some(&mut self.io_request) + } + fn io_poll(&mut self) -> &mut ::bun_io::Poll { + &mut self.io_poll + } + fn task(&mut self) -> &mut ::bun_jsc::WorkPoolTask { + &mut self.task + } + fn update(&mut self) { + $T::update(self) + } + #[cfg(windows)] + fn loop_(&self) -> *mut ::bun_libuv_sys::uv_loop_t { + unreachable!() + } + + fn schedule_close(request: &mut ::bun_io::Request) -> ::bun_io::Action<'_> { + // SAFETY: request is &mut self.io_request (intrusive); recover parent. + let this = unsafe { + &mut *<$T as ::bun_io::IntrusiveIoRequest>::from_io_request( + ::core::ptr::from_mut(request), + ) + }; + fn on_done(ctx: *mut ()) { + // SAFETY: ctx is `self as *mut Self` set below. + let this = unsafe { ::bun_ptr::callback_ctx::<$T>(ctx.cast()) }; + <$T as crate::webcore::blob::FileCloser>::on_io_request_closed(this); + } + // reshaped for borrowck — compute the parent raw pointer + // before mutably borrowing `io_poll` so the two borrows do not overlap. + let ctx = ::core::ptr::from_mut::<$T>(this).cast::<()>(); + let fd = this.opened_fd; + ::bun_io::Action::Close(::bun_io::CloseAction { + fd, + poll: &mut this.io_poll, + ctx, + tag: ::IO_TAG, + on_done, + }) + } + + // `FileCloser` fixes `on_close_io_request` to take `*mut WorkPoolTask`; + // the trait method cannot be marked `unsafe fn`, so the lint is + // unsatisfiable here. The pointer is the intrusive `&mut self.task` set + // in `on_io_request_closed` and is guaranteed live. + #[allow(clippy::not_unsafe_ptr_arg_deref)] + fn on_close_io_request(task: *mut ::bun_jsc::WorkPoolTask) { + // SAFETY: only reached via `WorkPoolTask::callback` with `task` = + // `&mut self.task` (intrusive) registered in `on_io_request_closed`; + // recover parent. + let this = unsafe { + &mut *<$T as ::bun_threading::IntrusiveWorkTask>::from_task_ptr(task) + }; + this.close_after_io = false; + $T::update(this); + } + } + }; +} +pub(crate) use impl_file_closer; + // ────────────────────────────────────────────────────────────────────────── // isAllASCII / takeOwnership / heap-alloc helpers / external_shared_descriptor // ────────────────────────────────────────────────────────────────────────── diff --git a/src/runtime/webcore/FileSink.rs b/src/runtime/webcore/FileSink.rs index 970c5e5f500..2c5f25270b5 100644 --- a/src/runtime/webcore/FileSink.rs +++ b/src/runtime/webcore/FileSink.rs @@ -1422,9 +1422,8 @@ impl crate::webcore::sink::JsSinkType for FileSink { const HAS_GET_FD: bool = true; const START_TAG: Option = Some(streams::StartTag::FileSink); - fn memory_cost(&self) -> usize { - Self::memory_cost(self) - } + crate::impl_js_sink_forwarders!(); + fn finalize(&mut self) { Self::finalize(self) } @@ -1433,30 +1432,9 @@ impl crate::webcore::sink::JsSinkType for FileSink { // the C++ `JSFileSink` wrapper `js_construct` is about to create. this.write(Self::construct()); } - fn write_bytes(&mut self, data: &streams::Result) -> streams::result::Writable { - Self::write(self, data) - } - fn write_utf16(&mut self, data: &streams::Result) -> streams::result::Writable { - Self::write_utf16(self, data) - } - fn write_latin1(&mut self, data: &streams::Result) -> streams::result::Writable { - Self::write_latin1(self, data) - } - fn end(&mut self, err: Option) -> sys::Result<()> { - Self::end(self, err) - } fn end_from_js(&mut self, global: &JSGlobalObject) -> sys::Result { Self::end_from_js(self, global) } - fn flush(&mut self) -> sys::Result<()> { - Self::flush(self) - } - fn flush_from_js(&mut self, global: &JSGlobalObject, wait: bool) -> sys::Result { - Self::flush_from_js(self, global, wait) - } - fn start(&mut self, config: streams::Start) -> sys::Result<()> { - Self::start(self, &config) - } fn signal(&mut self) -> Option<&mut streams::Signal> { // SAFETY: JsCell — trait receiver is `&mut self`; sole borrow of `signal`. Some(unsafe { self.signal.get_mut() }) diff --git a/src/runtime/webcore/S3Client.rs b/src/runtime/webcore/S3Client.rs index 0e6cba37d08..7a21afe6f3e 100644 --- a/src/runtime/webcore/S3Client.rs +++ b/src/runtime/webcore/S3Client.rs @@ -81,6 +81,16 @@ impl S3CredentialsExt for S3Credentials { } } +/// How [`S3Client::blob_and_options`] reports a missing/unparseable leading +/// path argument. Mirrors the per-method divergence in the Zig reference: +/// presign/exists/size/stat throw "invalid arguments" when an argument was +/// present but is not a path, while unlink always throws `MISSING_ARGS`. +#[derive(Clone, Copy)] +enum MissingPathError { + MissingOrInvalid, + AlwaysMissingArgs, +} + #[inline] fn opt_js(v: JSValue) -> Option { if v.is_empty_or_undefined_or_null() { @@ -338,6 +348,60 @@ impl S3Client { Ok(()) } + /// Constructs the S3 blob for `path` using this client's credentials and + /// per-client defaults, merged with the per-call `options` object. + /// `defer blob.detach()` from the Zig reference is handled by Drop of the + /// returned blob's `Option` field. + fn construct_blob( + &self, + global: &JSGlobalObject, + path: PathLike, + options: Option, + ) -> JsResult { + S3File::construct_s3_file_with_s3_credentials_and_options( + global, + path, + options, + &self.credentials, + self.options, + self.acl, + self.storage_class, + self.request_payer, + ) + } + + /// Shared prologue for the path-taking instance methods: parses the + /// leading path argument, eats the trailing options argument, and + /// constructs the S3 blob via [`Self::construct_blob`]. `verb` completes + /// the "Expected a path to {verb}" error message. + fn blob_and_options( + &self, + global: &JSGlobalObject, + callframe: &CallFrame, + verb: &str, + missing_path: MissingPathError, + ) -> JsResult<(crate::webcore::blob::Blob, Option)> { + let arguments = callframe.arguments_old::<2>(); + // SAFETY: `bun_vm()` returns the live VM pointer for `global`. + let vm = global.bun_vm(); + let mut args = bun_jsc::call_frame::ArgumentsSlice::init(vm, arguments.slice()); + let Some(path) = PathLike::from_js(global, &mut args)? else { + return Err(match missing_path { + MissingPathError::MissingOrInvalid if args.len() != 0 => { + global.throw_invalid_arguments(format_args!("Expected a path to {verb}")) + } + _ => global + .err( + ErrorCode::MISSING_ARGS, + format_args!("Expected a path to {verb}"), + ) + .throw(), + }); + }; + let options = args.next_eat(); + Ok((self.construct_blob(global, path, options)?, options)) + } + #[bun_jsc::host_fn(method)] pub(crate) fn file( ptr: &Self, @@ -362,18 +426,7 @@ impl S3Client { let options = args.next_eat(); // `Blob::new` heap-promotes and marks `ref_count = 1` so // the JSS3File wrapper's `finalize` knows to free the blob. - let blob = crate::webcore::blob::Blob::new( - S3File::construct_s3_file_with_s3_credentials_and_options( - global, - path, - options, - &ptr.credentials, - ptr.options, - ptr.acl, - ptr.storage_class, - ptr.request_payer, - )?, - ); + let blob = crate::webcore::blob::Blob::new(ptr.construct_blob(global, path, options)?); // `to_js` runs `calculateEstimatedByteSize()` // before wrapping the heap Blob in a JSS3File so JSC sees the correct // GC pressure. Route through `BlobExt::to_js` (the `&mut self` method @@ -389,39 +442,11 @@ impl S3Client { global: &JSGlobalObject, callframe: &CallFrame, ) -> JsResult { - let arguments = callframe.arguments_old::<2>(); - // SAFETY: `bun_vm()` returns the live VM pointer for `global`. - let vm = global.bun_vm(); - let mut args = bun_jsc::call_frame::ArgumentsSlice::init(vm, arguments.slice()); - let path: PathLike = match PathLike::from_js(global, &mut args)? { - Some(p) => p, - None => { - if args.len() == 0 { - return Err(global - .err( - ErrorCode::MISSING_ARGS, - format_args!("Expected a path to presign"), - ) - .throw()); - } - return Err( - global.throw_invalid_arguments(format_args!("Expected a path to presign")) - ); - } - }; - - let options = args.next_eat(); - // `defer blob.detach()` — `Blob`'s `store: Option` field - // drops at scope exit, which calls `Store::deref()` (same as detach). - let mut blob = S3File::construct_s3_file_with_s3_credentials_and_options( + let (mut blob, options) = ptr.blob_and_options( global, - path, - options, - &ptr.credentials, - ptr.options, - ptr.acl, - ptr.storage_class, - ptr.request_payer, + callframe, + "presign", + MissingPathError::MissingOrInvalid, )?; S3File::get_presign_url_from(&mut blob, global, options) } @@ -432,37 +457,11 @@ impl S3Client { global: &JSGlobalObject, callframe: &CallFrame, ) -> JsResult { - let arguments = callframe.arguments_old::<2>(); - // SAFETY: `bun_vm()` returns the live VM pointer for `global`. - let vm = global.bun_vm(); - let mut args = bun_jsc::call_frame::ArgumentsSlice::init(vm, arguments.slice()); - let path: PathLike = match PathLike::from_js(global, &mut args)? { - Some(p) => p, - None => { - if args.len() == 0 { - return Err(global - .err( - ErrorCode::MISSING_ARGS, - format_args!("Expected a path to check if it exists"), - ) - .throw()); - } - return Err(global.throw_invalid_arguments(format_args!( - "Expected a path to check if it exists" - ))); - } - }; - let options = args.next_eat(); - // `defer blob.detach()` — handled by Drop of `Option` field. - let blob = S3File::construct_s3_file_with_s3_credentials_and_options( + let (blob, _) = ptr.blob_and_options( global, - path, - options, - &ptr.credentials, - ptr.options, - ptr.acl, - ptr.storage_class, - ptr.request_payer, + callframe, + "check if it exists", + MissingPathError::MissingOrInvalid, )?; S3File::S3BlobStatTask::exists(global, &blob) } @@ -473,37 +472,11 @@ impl S3Client { global: &JSGlobalObject, callframe: &CallFrame, ) -> JsResult { - let arguments = callframe.arguments_old::<2>(); - // SAFETY: `bun_vm()` returns the live VM pointer for `global`. - let vm = global.bun_vm(); - let mut args = bun_jsc::call_frame::ArgumentsSlice::init(vm, arguments.slice()); - let path: PathLike = match PathLike::from_js(global, &mut args)? { - Some(p) => p, - None => { - if args.len() == 0 { - return Err(global - .err( - ErrorCode::MISSING_ARGS, - format_args!("Expected a path to check the size of"), - ) - .throw()); - } - return Err(global.throw_invalid_arguments(format_args!( - "Expected a path to check the size of" - ))); - } - }; - let options = args.next_eat(); - // `defer blob.detach()` — handled by Drop of `Option` field. - let mut blob = S3File::construct_s3_file_with_s3_credentials_and_options( + let (mut blob, _) = ptr.blob_and_options( global, - path, - options, - &ptr.credentials, - ptr.options, - ptr.acl, - ptr.storage_class, - ptr.request_payer, + callframe, + "check the size of", + MissingPathError::MissingOrInvalid, )?; S3File::S3BlobStatTask::size(global, &mut blob) } @@ -514,37 +487,11 @@ impl S3Client { global: &JSGlobalObject, callframe: &CallFrame, ) -> JsResult { - let arguments = callframe.arguments_old::<2>(); - // SAFETY: `bun_vm()` returns the live VM pointer for `global`. - let vm = global.bun_vm(); - let mut args = bun_jsc::call_frame::ArgumentsSlice::init(vm, arguments.slice()); - let path: PathLike = match PathLike::from_js(global, &mut args)? { - Some(p) => p, - None => { - if args.len() == 0 { - return Err(global - .err( - ErrorCode::MISSING_ARGS, - format_args!("Expected a path to check the stat of"), - ) - .throw()); - } - return Err(global.throw_invalid_arguments(format_args!( - "Expected a path to check the stat of" - ))); - } - }; - let options = args.next_eat(); - // `defer blob.detach()` — handled by Drop of `Option` field. - let blob = S3File::construct_s3_file_with_s3_credentials_and_options( + let (blob, _) = ptr.blob_and_options( global, - path, - options, - &ptr.credentials, - ptr.options, - ptr.acl, - ptr.storage_class, - ptr.request_payer, + callframe, + "check the stat of", + MissingPathError::MissingOrInvalid, )?; S3File::S3BlobStatTask::stat(global, &blob) } @@ -580,16 +527,7 @@ impl S3Client { }; let options = args.next_eat(); - let blob = S3File::construct_s3_file_with_s3_credentials_and_options( - global, - path, - options, - &ptr.credentials, - ptr.options, - ptr.acl, - ptr.storage_class, - ptr.request_payer, - )?; + let blob = ptr.construct_blob(global, path, options)?; // Move into `PathOrBlob` directly; cleanup of the moved-out value is // handled by `Drop`. let mut blob_internal = crate::webcore::node_types::PathOrBlob::Blob(Box::new(blob)); @@ -641,32 +579,11 @@ impl S3Client { global: &JSGlobalObject, callframe: &CallFrame, ) -> JsResult { - let arguments = callframe.arguments_old::<2>(); - // SAFETY: `bun_vm()` returns the live VM pointer for `global`. - let vm = global.bun_vm(); - let mut args = bun_jsc::call_frame::ArgumentsSlice::init(vm, arguments.slice()); - let path: PathLike = match PathLike::from_js(global, &mut args)? { - Some(p) => p, - None => { - return Err(global - .err( - ErrorCode::MISSING_ARGS, - format_args!("Expected a path to unlink"), - ) - .throw()); - } - }; - let options = args.next_eat(); - // `defer blob.detach()` — handled by Drop of `Option` field. - let blob = S3File::construct_s3_file_with_s3_credentials_and_options( + let (blob, options) = ptr.blob_and_options( global, - path, - options, - &ptr.credentials, - ptr.options, - ptr.acl, - ptr.storage_class, - ptr.request_payer, + callframe, + "unlink", + MissingPathError::AlwaysMissingArgs, )?; let store = blob.store.get().as_ref().unwrap(); store.data.as_s3().unlink(store, global, options) diff --git a/src/runtime/webcore/S3File.rs b/src/runtime/webcore/S3File.rs index 8122390ea10..582eab312b9 100644 --- a/src/runtime/webcore/S3File.rs +++ b/src/runtime/webcore/S3File.rs @@ -106,14 +106,15 @@ where Ok(()) } -#[bun_jsc::host_fn] -pub(crate) fn presign(global: &JSGlobalObject, callframe: &CallFrame) -> JsResult { - let arguments = callframe.arguments_old::<3>(); - // SAFETY: bun_vm() returns the live VM raw ptr. - let mut args = bun_jsc::call_frame::ArgumentsSlice::init(global.bun_vm(), arguments.slice()); - +/// Shared prologue for the S3 static host fns: parse the first argument as a +/// path or Blob and require Blob arguments to be S3-backed. +fn parse_s3_path_or_blob( + global: &JSGlobalObject, + args: &mut bun_jsc::call_frame::ArgumentsSlice, + error_message: &str, +) -> JsResult { // accept a path or a blob - let path_or_blob = PathOrBlob::from_js_no_copy(global, &mut args)?; + let path_or_blob = PathOrBlob::from_js_no_copy(global, args)?; // PathOrBlob impls Drop — path variant cleaned up automatically on `?` if let PathOrBlob::Blob(blob) = &path_or_blob { @@ -123,65 +124,57 @@ pub(crate) fn presign(global: &JSGlobalObject, callframe: &CallFrame) -> JsResul blob::store::Data::S3(_) ) { - return Err( - global.throw_invalid_arguments(format_args!("Expected a S3 or path to presign")) - ); + return Err(global.throw_invalid_arguments(format_args!("{error_message}"))); } } + Ok(path_or_blob) +} +/// Resolve a parsed argument to an S3 blob: reject file-descriptor paths and +/// construct the internal S3 store for path arguments. Returns the blob along +/// with the eaten options argument. +fn resolve_s3_blob( + global: &JSGlobalObject, + args: &mut bun_jsc::call_frame::ArgumentsSlice, + path_or_blob: PathOrBlob, + error_message: &str, +) -> JsResult<(Box, Option)> { + let options = args.next_eat(); match path_or_blob { PathOrBlob::Path(path) => { if matches!(path, crate::node::PathOrFileDescriptor::Fd(_)) { - return Err(global - .throw_invalid_arguments(format_args!("Expected a S3 or path to presign"))); + return Err(global.throw_invalid_arguments(format_args!("{error_message}"))); } - let options = args.next_eat(); - let mut blob = construct_s3_file_internal_store(global, path.path().clone(), options)?; - get_presign_url_from(&mut blob, global, options) + let blob = construct_s3_file_internal_store(global, path.path().clone(), options)?; + Ok((Box::new(blob), options)) } - PathOrBlob::Blob(mut blob) => get_presign_url_from(&mut blob, global, args.next_eat()), + PathOrBlob::Blob(blob) => Ok((blob, options)), } } #[bun_jsc::host_fn] -pub(crate) fn unlink(global: &JSGlobalObject, callframe: &CallFrame) -> JsResult { +pub(crate) fn presign(global: &JSGlobalObject, callframe: &CallFrame) -> JsResult { let arguments = callframe.arguments_old::<3>(); // SAFETY: bun_vm() returns the live VM raw ptr. let mut args = bun_jsc::call_frame::ArgumentsSlice::init(global.bun_vm(), arguments.slice()); - // accept a path or a blob - let path_or_blob = PathOrBlob::from_js_no_copy(global, &mut args)?; + let error_message = "Expected a S3 or path to presign"; + let path_or_blob = parse_s3_path_or_blob(global, &mut args, error_message)?; + let (mut blob, options) = resolve_s3_blob(global, &mut args, path_or_blob, error_message)?; + get_presign_url_from(&mut blob, global, options) +} - if let PathOrBlob::Blob(blob) = &path_or_blob { - if blob.store.get().is_none() - || !matches!( - blob.store.get().as_ref().unwrap().data, - blob::store::Data::S3(_) - ) - { - return Err( - global.throw_invalid_arguments(format_args!("Expected a S3 or path to delete")) - ); - } - } +#[bun_jsc::host_fn] +pub(crate) fn unlink(global: &JSGlobalObject, callframe: &CallFrame) -> JsResult { + let arguments = callframe.arguments_old::<3>(); + // SAFETY: bun_vm() returns the live VM raw ptr. + let mut args = bun_jsc::call_frame::ArgumentsSlice::init(global.bun_vm(), arguments.slice()); - match path_or_blob { - PathOrBlob::Path(path) => { - if matches!(path, crate::node::PathOrFileDescriptor::Fd(_)) { - return Err( - global.throw_invalid_arguments(format_args!("Expected a S3 or path to delete")) - ); - } - let options = args.next_eat(); - let blob = construct_s3_file_internal_store(global, path.path().clone(), options)?; - let store = blob.store.get().as_ref().unwrap(); - store.data.as_s3().unlink(store, global, options) - } - PathOrBlob::Blob(blob) => { - let store = blob.store.get().as_ref().unwrap(); - store.data.as_s3().unlink(store, global, args.next_eat()) - } - } + let error_message = "Expected a S3 or path to delete"; + let path_or_blob = parse_s3_path_or_blob(global, &mut args, error_message)?; + let (blob, options) = resolve_s3_blob(global, &mut args, path_or_blob, error_message)?; + let store = blob.store.get().as_ref().unwrap(); + store.data.as_s3().unlink(store, global, options) } #[bun_jsc::host_fn] @@ -190,21 +183,8 @@ pub fn write(global: &JSGlobalObject, callframe: &CallFrame) -> JsResult JsResult { - let options = args.next_eat(); - if matches!(path, crate::node::PathOrFileDescriptor::Fd(_)) { - return Err( - global.throw_invalid_arguments(format_args!("Expected a S3 or path to upload")) - ); - } - let blob = construct_s3_file_internal_store(global, path.path().clone(), options)?; - - let mut blob_internal = PathOrBlob::Blob(Box::new(blob)); - blob::write_file_internal( - global, - &mut blob_internal, - data, - blob::WriteFileOptions { - mkdirp_if_not_exists: Some(false), - extra_options: options, - ..Default::default() - }, - ) - } - PathOrBlob::Blob(blob) => { - // Reshaped for borrowck — match consumes path_or_blob; rebuild to pass &mut PathOrBlob - let mut pob = PathOrBlob::Blob(blob); - blob::write_file_internal( - global, - &mut pob, - data, - blob::WriteFileOptions { - mkdirp_if_not_exists: Some(false), - extra_options: args.next_eat(), - ..Default::default() - }, - ) - } - } + let (blob, options) = resolve_s3_blob(global, &mut args, path_or_blob, error_message)?; + // `write_file_internal` takes `&mut PathOrBlob`; wrap the resolved blob. + let mut blob_internal = PathOrBlob::Blob(blob); + blob::write_file_internal( + global, + &mut blob_internal, + data, + blob::WriteFileOptions { + mkdirp_if_not_exists: Some(false), + extra_options: options, + ..Default::default() + }, + ) } #[bun_jsc::host_fn] @@ -260,35 +216,12 @@ pub(crate) fn size(global: &JSGlobalObject, callframe: &CallFrame) -> JsResult { - let options = args.next_eat(); - if matches!(path, crate::node::PathOrFileDescriptor::Fd(_)) { - return Err(global - .throw_invalid_arguments(format_args!("Expected a S3 or path to get size"))); - } - let mut blob = construct_s3_file_internal_store(global, path.path().clone(), options)?; - - S3BlobStatTask::size(global, &mut blob) - } - PathOrBlob::Blob(blob) => Ok(blob.get_size(global)), - } + let error_message = "Expected a S3 or path to get size"; + let mut blob = match parse_s3_path_or_blob(global, &mut args, error_message)? { + PathOrBlob::Blob(blob) => return Ok(blob.get_size(global)), + path => resolve_s3_blob(global, &mut args, path, error_message)?.0, + }; + S3BlobStatTask::size(global, &mut blob) } #[bun_jsc::host_fn] @@ -297,36 +230,12 @@ pub(crate) fn exists(global: &JSGlobalObject, callframe: &CallFrame) -> JsResult // SAFETY: bun_vm() returns the live VM raw ptr. let mut args = bun_jsc::call_frame::ArgumentsSlice::init(global.bun_vm(), arguments.slice()); - // accept a path or a blob - let mut path_or_blob = PathOrBlob::from_js_no_copy(global, &mut args)?; - - if let PathOrBlob::Blob(blob) = &path_or_blob { - if blob.store.get().is_none() - || !matches!( - blob.store.get().as_ref().unwrap().data, - blob::store::Data::S3(_) - ) - { - return Err(global.throw_invalid_arguments(format_args!( - "Expected a S3 or path to check if it exists" - ))); - } - } - - match &mut path_or_blob { - PathOrBlob::Path(path) => { - let options = args.next_eat(); - if matches!(path, crate::node::PathOrFileDescriptor::Fd(_)) { - return Err(global.throw_invalid_arguments(format_args!( - "Expected a S3 or path to check if it exists" - ))); - } - let blob = construct_s3_file_internal_store(global, path.path().clone(), options)?; - - S3BlobStatTask::exists(global, &blob) - } - PathOrBlob::Blob(blob) => blob.get_exists(global, callframe), - } + let error_message = "Expected a S3 or path to check if it exists"; + let blob = match parse_s3_path_or_blob(global, &mut args, error_message)? { + PathOrBlob::Blob(blob) => return blob.get_exists(global, callframe), + path => resolve_s3_blob(global, &mut args, path, error_message)?.0, + }; + S3BlobStatTask::exists(global, &blob) } fn construct_s3_file_internal_store( @@ -358,7 +267,7 @@ pub(crate) fn construct_s3_file_with_s3_credentials_and_options( default_storage_class: Option, default_request_payer: bool, ) -> JsResult { - let aws_options = ::get_credentials_with_options( + let mut aws_options = ::get_credentials_with_options( default_credentials, default_options, options, @@ -368,18 +277,28 @@ pub(crate) fn construct_s3_file_with_s3_credentials_and_options( global, )?; - let mut store = 'brk: { - if aws_options.changed_credentials { - break 'brk blob::Store::init_s3(path, None, aws_options.credentials).expect("oom"); - } else { - // The `Store::S3` field is `Rc` (separate rc - // layer), so we can't share the existing intrusive allocation — - // deep-clone the value instead and let `init_s3` `Rc::new` it. - // PERF: profile if hot once Store.rs migrates - // `Rc` → `IntrusiveRc`. - break 'brk blob::Store::init_s3(path, None, default_credentials.clone()).expect("oom"); - } + let credentials = if aws_options.changed_credentials { + std::mem::take(&mut aws_options.credentials) + } else { + // The `Store::S3` field is `Rc` (separate rc + // layer), so we can't share the existing intrusive allocation — + // deep-clone the value instead and let `init_s3` `Rc::new` it. + // PERF: profile if hot once Store.rs migrates + // `Rc` → `IntrusiveRc`. + default_credentials.clone() }; + let store = blob::Store::init_s3(path, None, credentials).expect("oom"); + finish_s3_blob(global, store, &aws_options, options) +} + +/// Shared construction epilogue: copy the parsed S3 options onto the store, +/// wrap it in a `Blob`, and apply any `options.type` content-type override. +fn finish_s3_blob( + global: &JSGlobalObject, + mut store: Box, + aws_options: &s3::S3CredentialsWithOptions, + options: Option, +) -> JsResult { // store cleanup on early return is handled by Drop store.data.as_s3_mut().options = aws_options.options; store.data.as_s3_mut().acl = aws_options.acl; @@ -437,7 +356,7 @@ pub(crate) fn construct_s3_file_with_s3_credentials( options: Option, existing_credentials: &s3::S3Credentials, ) -> JsResult { - let aws_options = ::get_credentials_with_options( + let mut aws_options = ::get_credentials_with_options( existing_credentials, Default::default(), options, @@ -446,56 +365,9 @@ pub(crate) fn construct_s3_file_with_s3_credentials( false, global, )?; - let mut store = blob::Store::init_s3(path, None, aws_options.credentials).expect("oom"); - // store cleanup on early return is handled by Drop - store.data.as_s3_mut().options = aws_options.options; - store.data.as_s3_mut().acl = aws_options.acl; - store.data.as_s3_mut().storage_class = aws_options.storage_class; - store.data.as_s3_mut().request_payer = aws_options.request_payer; - - let blob = Blob::init_with_store(store, global); - if let Some(opts) = options { - if opts.is_object() { - if let Some(file_type) = opts.get_truthy(global, "type")? { - 'inner: { - if file_type.is_string() { - let str = file_type.to_slice(global)?; - let slice = str.slice(); - if !blob::is_valid_blob_type(slice) { - break 'inner; - } - blob.content_type_was_set.set(true); - // SAFETY: bun_vm() returns the live VM raw ptr. - if let Some(entry) = global.bun_vm().as_mut().mime_type(str.slice()) { - // `MimeType.value` is `Cow<'static, [u8]>`; the - // canonical-table hit (via `Compact::to_mime_type`) is always - // `Borrowed(&'static)`. If a future table source ever yields - // `Owned`, hand the buffer to the blob's allocated-content-type - // path so `Blob::deinit` reclaims it. - match entry.value { - std::borrow::Cow::Borrowed(s) => { - blob.content_type.set(std::ptr::from_ref::<[u8]>(s)); - } - std::borrow::Cow::Owned(v) => { - blob.content_type - .set(bun_core::heap::into_raw(v.into_boxed_slice())); - blob.content_type_allocated.set(true); - } - } - break 'inner; - } - let mut content_type_buf = vec![0u8; slice.len()]; - strings::copy_lowercase(slice, &mut content_type_buf); - blob.content_type.set(bun_core::heap::into_raw( - content_type_buf.into_boxed_slice(), - )); - blob.content_type_allocated.set(true); - } - } - } - } - } - Ok(blob) + let credentials = std::mem::take(&mut aws_options.credentials); + let store = blob::Store::init_s3(path, None, credentials).expect("oom"); + finish_s3_blob(global, store, &aws_options, options) } fn construct_s3_file_internal( @@ -858,35 +730,10 @@ pub(crate) fn stat(global: &JSGlobalObject, callframe: &CallFrame) -> JsResult { - let options = args.next_eat(); - if matches!(path, crate::node::PathOrFileDescriptor::Fd(_)) { - return Err(global - .throw_invalid_arguments(format_args!("Expected a S3 or path to get size"))); - } - let blob = construct_s3_file_internal_store(global, path.path().clone(), options)?; - - S3BlobStatTask::stat(global, &blob) - } - PathOrBlob::Blob(blob) => S3BlobStatTask::stat(global, blob), - } + let error_message = "Expected a S3 or path to get size"; + let path_or_blob = parse_s3_path_or_blob(global, &mut args, error_message)?; + let (blob, _options) = resolve_s3_blob(global, &mut args, path_or_blob, error_message)?; + S3BlobStatTask::stat(global, &blob) } pub(crate) fn construct_internal_js( diff --git a/src/runtime/webcore/blob/copy_file.rs b/src/runtime/webcore/blob/copy_file.rs index 831c84a0bee..d78b752b107 100644 --- a/src/runtime/webcore/blob/copy_file.rs +++ b/src/runtime/webcore/blob/copy_file.rs @@ -309,6 +309,44 @@ impl<'a> CopyFile<'a> { Ok(()) } + /// Copies the remaining bytes with a blocking read/write loop (we run on + /// a concurrent task thread, not the JS thread) and truncates the + /// destination to the copied length. Used when `copy_file_range`, + /// `sendfile`, and `splice` are unavailable or unusable. + #[cfg(any(target_os = "linux", target_os = "android"))] + fn fallback_read_write( + &mut self, + src_fd: Fd, + dest_fd: Fd, + remain: usize, + unknown_size: bool, + total_written: &mut u64, + ) -> Result<(), bun_core::Error> { + match node_fs::NodeFS::copy_file_using_read_write_loop( + bun_core::ZStr::EMPTY, + bun_core::ZStr::EMPTY, + src_fd, + dest_fd, + if unknown_size { 0 } else { remain }, + total_written, + ) { + bun_sys::Result::Err(err) => { + self.system_error = Some(err.to_system_error()); + Err(bun_core::errno_to_zig_err(err.errno as i32)) + } + bun_sys::Result::Ok(()) => { + // SAFETY: dest_fd is a valid open fd; raw ftruncate(2). + let _ = unsafe { + libc::ftruncate( + dest_fd.native(), + i64::try_from(*total_written).expect("int cast"), + ) + }; + Ok(()) + } + } + } + #[cfg(any(target_os = "linux", target_os = "android"))] pub fn do_copy_file_range( &mut self, @@ -343,29 +381,13 @@ impl<'a> CopyFile<'a> { // If they can't use copy_file_range, they probably also can't // use sendfile() or splice() if !bun_sys::copy_file::can_use_copy_file_range_syscall() { - match node_fs::NodeFS::copy_file_using_read_write_loop( - bun_core::ZStr::EMPTY, - bun_core::ZStr::EMPTY, + return self.fallback_read_write( src_fd, dest_fd, - if unknown_size { 0 } else { remain }, + remain, + unknown_size, &mut total_written, - ) { - bun_sys::Result::Err(err) => { - self.system_error = Some(err.to_system_error()); - return Err(bun_core::errno_to_zig_err(err.errno as i32)); - } - bun_sys::Result::Ok(()) => { - // SAFETY: dest_fd is a valid open fd; raw ftruncate(2). - let _ = unsafe { - libc::ftruncate( - dest_fd.native(), - i64::try_from(total_written).expect("int cast"), - ) - }; - return Ok(()); - } - } + ); } loop { @@ -417,30 +439,13 @@ impl<'a> CopyFile<'a> { // NOSYS: syscall not available // OPNOTSUPP: filesystem doesn't support this operation bun_sys::E::ENOSYS | bun_sys::E::EXDEV | bun_sys::E::ENOTSUP => { - // TODO: this should use non-blocking I/O. - match node_fs::NodeFS::copy_file_using_read_write_loop( - bun_core::ZStr::EMPTY, - bun_core::ZStr::EMPTY, + return self.fallback_read_write( src_fd, dest_fd, - if unknown_size { 0 } else { remain }, + remain, + unknown_size, &mut total_written, - ) { - bun_sys::Result::Err(err) => { - self.system_error = Some(err.to_system_error()); - return Err(bun_core::errno_to_zig_err(err.errno as i32)); - } - bun_sys::Result::Ok(()) => { - // SAFETY: dest_fd is a valid open fd; raw ftruncate(2). - let _ = unsafe { - libc::ftruncate( - dest_fd.native(), - i64::try_from(total_written).expect("int cast"), - ) - }; - return Ok(()); - } - } + ); } // EINVAL: eCryptfs and other filesystems may not support copy_file_range. @@ -474,30 +479,13 @@ impl<'a> CopyFile<'a> { // incompatible with the chosen syscall, fall back // to a read/write loop if total_written == 0 { - // TODO: this should use non-blocking I/O. - match node_fs::NodeFS::copy_file_using_read_write_loop( - bun_core::ZStr::EMPTY, - bun_core::ZStr::EMPTY, + return self.fallback_read_write( src_fd, dest_fd, - if unknown_size { 0 } else { remain }, + remain, + unknown_size, &mut total_written, - ) { - bun_sys::Result::Err(err) => { - self.system_error = Some(err.to_system_error()); - return Err(bun_core::errno_to_zig_err(err.errno as i32)); - } - bun_sys::Result::Ok(()) => { - // SAFETY: dest_fd is a valid open fd; raw ftruncate(2). - let _ = unsafe { - libc::ftruncate( - dest_fd.native(), - i64::try_from(total_written).expect("int cast"), - ) - }; - return Ok(()); - } - } + ); } self.system_error = Some( diff --git a/src/runtime/webcore/blob/read_file.rs b/src/runtime/webcore/blob/read_file.rs index a79cdc06134..7ef422ec6ed 100644 --- a/src/runtime/webcore/blob/read_file.rs +++ b/src/runtime/webcore/blob/read_file.rs @@ -204,6 +204,7 @@ pub struct ReadFile { } bun_threading::intrusive_work_task!(ReadFile, task); +bun_io::intrusive_io_request!(ReadFile, io_request); // The default methods on the FileOpener/FileCloser traits provide the bodies. impl FileOpener for ReadFile { @@ -240,81 +241,7 @@ impl FileOpener for ReadFile { } } -impl FileCloser for ReadFile { - const IO_TAG: bun_io::Tag = bun_io::Tag::ReadFile; - fn opened_fd(&self) -> Fd { - self.opened_fd - } - fn set_opened_fd(&mut self, fd: Fd) { - self.opened_fd = fd; - } - fn close_after_io(&self) -> bool { - self.close_after_io - } - fn set_close_after_io(&mut self, v: bool) { - self.close_after_io = v; - } - fn state(&self) -> &AtomicU8 { - &self.state - } - fn io_request(&mut self) -> Option<&mut bun_io::Request> { - Some(&mut self.io_request) - } - fn io_poll(&mut self) -> &mut bun_io::Poll { - &mut self.io_poll - } - fn task(&mut self) -> &mut bun_jsc::WorkPoolTask { - &mut self.task - } - fn update(&mut self) { - ReadFile::update(self) - } - #[cfg(windows)] - fn loop_(&self) -> *mut bun_libuv_sys::uv_loop_t { - unreachable!() - } - - fn schedule_close(request: &mut bun_io::Request) -> bun_io::Action<'_> { - // SAFETY: request is &mut self.io_request (intrusive); recover parent. - let this: &mut ReadFile = unsafe { - &mut *(bun_core::from_field_ptr!( - ReadFile, - io_request, - std::ptr::from_mut::(request) - )) - }; - fn on_done(ctx: *mut ()) { - // SAFETY: ctx is `self as *mut ReadFile` set below. - let this = unsafe { bun_ptr::callback_ctx::(ctx.cast()) }; - ::on_io_request_closed(this); - } - // reshaped for borrowck — compute the parent raw pointer - // before mutably borrowing `io_poll` so the two borrows do not overlap. - let ctx = std::ptr::from_mut::(this).cast::<()>(); - let fd = this.opened_fd; - io::Action::Close(io::CloseAction { - fd, - poll: &mut this.io_poll, - ctx, - tag: ::IO_TAG, - on_done, - }) - } - - // `FileCloser` fixes `on_close_io_request` to take `*mut WorkPoolTask`; - // the trait method cannot be marked `unsafe fn`, so the lint is - // unsatisfiable here. The pointer is the intrusive `&mut self.task` set - // in `on_io_request_closed` and is guaranteed live. - #[allow(clippy::not_unsafe_ptr_arg_deref)] - fn on_close_io_request(task: *mut bun_jsc::WorkPoolTask) { - // SAFETY: only reached via `WorkPoolTask::callback` with `task` = - // `&mut self.task` (intrusive) registered in `on_io_request_closed`; - // recover parent. - let this = unsafe { &mut *ReadFile::from_task_ptr(task) }; - this.close_after_io = false; - ReadFile::update(this); - } -} +crate::webcore::blob::impl_file_closer!(ReadFile); impl ReadFile { pub fn update(&mut self) { diff --git a/src/runtime/webcore/blob/write_file.rs b/src/runtime/webcore/blob/write_file.rs index f49196680e5..59f33e8a984 100644 --- a/src/runtime/webcore/blob/write_file.rs +++ b/src/runtime/webcore/blob/write_file.rs @@ -13,9 +13,10 @@ use bun_jsc::{self as jsc, JSGlobalObject, JSPromise, JSValue, JsTerminated, Sys use bun_sys::{self as sys, Fd}; use bun_threading::{IntrusiveWorkTask as _, WorkPool, WorkPoolTask}; +#[cfg(not(windows))] +use crate::webcore::blob::FileCloser; use crate::webcore::blob::{ - self, Blob, ClosingState, FileCloser, FileOpener, MkdirpTarget, Retry, SizeType, - mkdir_if_not_exists, + self, Blob, ClosingState, FileOpener, MkdirpTarget, Retry, SizeType, mkdir_if_not_exists, }; use crate::webcore::body; @@ -152,75 +153,7 @@ impl MkdirpTarget for WriteFile { } } -impl FileCloser for WriteFile { - const IO_TAG: io::Tag = io::Tag::WriteFile; - fn opened_fd(&self) -> Fd { - self.opened_fd - } - fn set_opened_fd(&mut self, fd: Fd) { - self.opened_fd = fd; - } - fn close_after_io(&self) -> bool { - self.close_after_io - } - fn set_close_after_io(&mut self, v: bool) { - self.close_after_io = v; - } - fn state(&self) -> &AtomicU8 { - &self.state - } - fn io_request(&mut self) -> Option<&mut io::Request> { - Some(&mut self.io_request) - } - fn io_poll(&mut self) -> &mut io::Poll { - &mut self.io_poll - } - fn task(&mut self) -> &mut bun_jsc::WorkPoolTask { - &mut self.task - } - fn update(&mut self) { - WriteFile::update(self) - } - #[cfg(windows)] - fn loop_(&self) -> *mut bun_libuv_sys::uv_loop_t { - unreachable!() - } - - fn schedule_close(request: &mut io::Request) -> io::Action<'_> { - // SAFETY: request is &mut self.io_request (intrusive); recover parent. - let this = unsafe { &mut *WriteFile::from_io_request(std::ptr::from_mut(request)) }; - fn on_done(ctx: *mut ()) { - // SAFETY: ctx is `self as *mut WriteFile` set below. - let this = unsafe { bun_ptr::callback_ctx::(ctx.cast()) }; - ::on_io_request_closed(this); - } - // reshaped for borrowck — compute the parent raw pointer - // before mutably borrowing `io_poll` so the two borrows do not overlap. - let ctx = std::ptr::from_mut::(this).cast::<()>(); - let fd = this.opened_fd; - io::Action::Close(io::CloseAction { - fd, - poll: &mut this.io_poll, - ctx, - tag: ::IO_TAG, - on_done, - }) - } - - // `FileCloser` fixes `on_close_io_request` to take `*mut WorkPoolTask`; - // the trait method cannot be marked `unsafe fn`, so the lint is - // unsatisfiable here. The pointer is the intrusive `&mut self.task` set - // in `on_io_request_closed` and is guaranteed live. - #[allow(clippy::not_unsafe_ptr_arg_deref)] - fn on_close_io_request(task: *mut bun_jsc::WorkPoolTask) { - // SAFETY: only reached via `WorkPoolTask::callback` with `task` = - // `&mut self.task` (intrusive) registered in `on_io_request_closed`; - // recover parent. - let this = unsafe { &mut *WriteFile::from_task_ptr(task) }; - this.close_after_io = false; - WriteFile::update(this); - } -} +crate::webcore::blob::impl_file_closer!(WriteFile); impl WriteFile { pub const IO_TAG: io::Tag = io::Tag::WriteFile; diff --git a/src/runtime/webcore/streams.rs b/src/runtime/webcore/streams.rs index dedc030ef92..2cc900d6e98 100644 --- a/src/runtime/webcore/streams.rs +++ b/src/runtime/webcore/streams.rs @@ -2002,6 +2002,56 @@ impl HTTPServerWritable { crate::impl_sink_handler!([const SSL: bool, const HTTP3: bool] HTTPServerWritable); +/// Emits the standard `JsSinkType` forwarders (`memory_cost`, `write_bytes`, +/// `write_utf16`, `write_latin1`, `end`, `flush`, `flush_from_js`, `start`) +/// inside an `impl JsSinkType for T` block. `Self::name` prefers the inherent +/// item over the trait item being defined, so the forward never recurses +/// (same rule as `impl_sink_handler!`). The genuinely divergent items +/// (`finalize`, `construct`, `end_from_js`, `signal`, `done`, the `HAS_*` +/// consts) stay hand-written per type. +#[macro_export] +macro_rules! impl_js_sink_forwarders { + () => { + fn memory_cost(&self) -> usize { + Self::memory_cost(self) + } + fn write_bytes( + &mut self, + data: &$crate::webcore::streams::Result, + ) -> $crate::webcore::streams::result::Writable { + Self::write(self, data) + } + fn write_utf16( + &mut self, + data: &$crate::webcore::streams::Result, + ) -> $crate::webcore::streams::result::Writable { + Self::write_utf16(self, data) + } + fn write_latin1( + &mut self, + data: &$crate::webcore::streams::Result, + ) -> $crate::webcore::streams::result::Writable { + Self::write_latin1(self, data) + } + fn end(&mut self, err: ::core::option::Option<::bun_sys::Error>) -> ::bun_sys::Result<()> { + Self::end(self, err) + } + fn flush(&mut self) -> ::bun_sys::Result<()> { + Self::flush(self) + } + fn flush_from_js( + &mut self, + global: &::bun_jsc::JSGlobalObject, + wait: bool, + ) -> ::bun_sys::Result<::bun_jsc::JSValue> { + Self::flush_from_js(self, global, wait) + } + fn start(&mut self, config: $crate::webcore::streams::Start) -> ::bun_sys::Result<()> { + Self::start(self, &config) + } + }; +} + // `JsSinkType` impl: routes the codegen `${name}__{construct,write,end,flush, // start,getInternalFd,memoryCost}` thunks (via `JSSink::::js_*`) into // the inherent streaming methods above. Mirrors `Sink.JSSink(@This(), name)`. @@ -2020,36 +2070,14 @@ impl crate::webcore::sink::JsSinkType StartTag::HTTPResponseSink }); - fn memory_cost(&self) -> usize { - Self::memory_cost(self) - } + crate::impl_js_sink_forwarders!(); + fn finalize(&mut self) { Self::finalize(self) } - fn write_bytes(&mut self, data: &StreamResult) -> Writable { - Self::write(self, data) - } - fn write_utf16(&mut self, data: &StreamResult) -> Writable { - Self::write_utf16(self, data) - } - fn write_latin1(&mut self, data: &StreamResult) -> Writable { - Self::write_latin1(self, data) - } - fn end(&mut self, err: Option) -> bun_sys::Result<()> { - Self::end(self, err) - } fn end_from_js(&mut self, global: &JSGlobalObject) -> bun_sys::Result { Self::end_from_js(self, global) } - fn flush(&mut self) -> bun_sys::Result<()> { - Self::flush(self) - } - fn flush_from_js(&mut self, global: &JSGlobalObject, wait: bool) -> bun_sys::Result { - Self::flush_from_js(self, global, wait) - } - fn start(&mut self, config: Start) -> bun_sys::Result<()> { - Self::start(self, &config) - } fn signal(&mut self) -> Option<&mut Signal> { Some(&mut self.signal) } @@ -2378,36 +2406,14 @@ impl crate::webcore::sink::JsSinkType for NetworkSink { const HAS_FLUSH_FROM_JS: bool = true; const START_TAG: Option = Some(StartTag::NetworkSink); - fn memory_cost(&self) -> usize { - Self::memory_cost(self) - } + crate::impl_js_sink_forwarders!(); + fn finalize(&mut self) { Self::finalize(self) } - fn write_bytes(&mut self, data: &StreamResult) -> Writable { - Self::write(self, data) - } - fn write_utf16(&mut self, data: &StreamResult) -> Writable { - Self::write_utf16(self, data) - } - fn write_latin1(&mut self, data: &StreamResult) -> Writable { - Self::write_latin1(self, data) - } - fn end(&mut self, err: Option) -> bun_sys::Result<()> { - Self::end(self, err) - } fn end_from_js(&mut self, global: &JSGlobalObject) -> bun_sys::Result { Self::end_from_js(self, global) } - fn flush(&mut self) -> bun_sys::Result<()> { - Self::flush(self) - } - fn flush_from_js(&mut self, global: &JSGlobalObject, wait: bool) -> bun_sys::Result { - Self::flush_from_js(self, global, wait) - } - fn start(&mut self, config: Start) -> bun_sys::Result<()> { - Self::start(self, &config) - } fn signal(&mut self) -> Option<&mut Signal> { Some(&mut self.signal) } diff --git a/test/js/bun/s3/s3-argument-validation.test.ts b/test/js/bun/s3/s3-argument-validation.test.ts new file mode 100644 index 00000000000..cd251b4a51a --- /dev/null +++ b/test/js/bun/s3/s3-argument-validation.test.ts @@ -0,0 +1,79 @@ +import { describe, expect, test } from "bun:test"; + +// Argument validation for the S3 static and instance methods. Every case here +// is rejected synchronously, before any network request, so no server is +// needed. + +// The static methods accept a path or an S3 blob; everything else is rejected +// up front with a per-method message. Numbers parse as file descriptor paths, +// which S3 also rejects. +describe("S3Client static method argument validation", () => { + const staticCases = [ + ["presign", "Expected a S3 or path to presign", []], + ["unlink", "Expected a S3 or path to delete", []], + ["write", "Expected a S3 or path to upload", ["data"]], + ["size", "Expected a S3 or path to get size", []], + ["exists", "Expected a S3 or path to check if it exists", []], + // stat reuses the size wording + ["stat", "Expected a S3 or path to get size", []], + ] as const; + + test.each(staticCases)("S3Client.%s rejects non-S3 arguments", (method, message, extra) => { + const expected = expect.objectContaining({ code: "ERR_INVALID_ARG_TYPE", message }); + const fn = (Bun.S3Client as any)[method]; + // a data-backed Blob is not S3-backed + expect(() => fn(new Blob(["x"]), ...extra)).toThrow(expected); + // a local file Blob is not S3-backed either + expect(() => fn(Bun.file(import.meta.path), ...extra)).toThrow(expected); + // a number is parsed as a file descriptor path + expect(() => fn(0, ...extra)).toThrow(expected); + }); + + test("S3Client.write requires data", () => { + expect(() => Bun.S3Client.write("some-key")).toThrow( + expect.objectContaining({ code: "ERR_MISSING_ARGS", message: "Expected a Blob-y thing to upload" }), + ); + }); +}); + +describe("S3Client instance method argument validation", () => { + const client = new Bun.S3Client({ + accessKeyId: "test", + secretAccessKey: "test", + bucket: "bucket", + endpoint: "http://127.0.0.1:1", + }); + + const instanceCases = [ + ["presign", "Expected a path to presign"], + ["exists", "Expected a path to check if it exists"], + ["size", "Expected a path to check the size of"], + ["stat", "Expected a path to check the stat of"], + ] as const; + + test.each(instanceCases)("client.%s distinguishes a missing path from an invalid one", (method, message) => { + const fn = (client as any)[method].bind(client); + // no argument at all: MISSING_ARGS + expect(() => fn()).toThrow(expect.objectContaining({ code: "ERR_MISSING_ARGS", message })); + // an argument that is not a path: INVALID_ARG_TYPE, same message + expect(() => fn(123)).toThrow(expect.objectContaining({ code: "ERR_INVALID_ARG_TYPE", message })); + expect(() => fn({})).toThrow(expect.objectContaining({ code: "ERR_INVALID_ARG_TYPE", message })); + }); + + test("client.unlink reports MISSING_ARGS for both missing and invalid paths", () => { + const expected = expect.objectContaining({ code: "ERR_MISSING_ARGS", message: "Expected a path to unlink" }); + expect(() => (client as any).unlink()).toThrow(expected); + expect(() => (client as any).unlink(123)).toThrow(expected); + expect(() => (client as any).unlink({})).toThrow(expected); + }); + + test("S3 file writer() rejects a non-string type option before any request is made", () => { + const s3file = client.file("some-key.bin"); + expect(() => s3file.writer({ type: 123 as any })).toThrow( + expect.objectContaining({ + code: "ERR_INVALID_ARG_TYPE", + message: "Expected options.type to be a string for 'write'.", + }), + ); + }); +}); diff --git a/test/js/web/fetch/blob-write.test.ts b/test/js/web/fetch/blob-write.test.ts index 9fc61c0a110..b656ac7766b 100644 --- a/test/js/web/fetch/blob-write.test.ts +++ b/test/js/web/fetch/blob-write.test.ts @@ -84,3 +84,43 @@ test("Bun.file(path).stat() returns stats", async () => { expect(stat).toBeDefined(); expect(stat.size).toBe(13); // "Hello, world!" is 13 bytes }); + +// Bun.file().write() accepts an options.type override: non-strings throw, +// valid types are stored lowercased (through the mime table when known), and +// invalid blob types are silently ignored. +test("Bun.file(path).write() rejects a non-string options.type", async () => { + const dir = tempDirWithFiles("blob-write-type", { "a.txt": "hello" }); + const file = Bun.file(path.join(dir, "a.txt")); + let err: any; + try { + await file.write("x", { type: 123 as any }); + } catch (e) { + err = e; + } + expect(err).toMatchObject({ + code: "ERR_INVALID_ARG_TYPE", + message: "Expected options.type to be a string for 'write'.", + }); +}); + +test("Bun.file(path).write() lowercases and applies a valid options.type", async () => { + const dir = tempDirWithFiles("blob-write-type", { "a.txt": "hello" }); + const file = Bun.file(path.join(dir, "a.txt")); + await file.write("x", { type: "TEXT/PLAIN; CHARSET=UTF-8" }); + expect(file.type).toBe("text/plain; charset=utf-8"); +}); + +test("Bun.file(path).write() resolves a known options.type through the mime table", async () => { + const dir = tempDirWithFiles("blob-write-type", { "a.txt": "hello" }); + const file = Bun.file(path.join(dir, "a.txt")); + await file.write("x", { type: "APPLICATION/JSON" }); + expect(file.type).toBe("application/json"); +}); + +test("Bun.file(path).write() silently ignores an invalid options.type", async () => { + const dir = tempDirWithFiles("blob-write-type", { "a.txt": "hello" }); + const file = Bun.file(path.join(dir, "a.txt")); + await file.write("x", { type: "bad\r\ntype" }); + // the .txt default is kept + expect(file.type).toBe("text/plain;charset=utf-8"); +}); diff --git a/test/js/web/fetch/blob.test.ts b/test/js/web/fetch/blob.test.ts index dc5ccbd1adf..581a502ad66 100644 --- a/test/js/web/fetch/blob.test.ts +++ b/test/js/web/fetch/blob.test.ts @@ -556,3 +556,49 @@ describe("slice bounds are respected when streaming and serving", () => { expect(await get.text()).toBe("3456"); }); }); + +// Blob conversion accepts every ArrayBuffer-like type, both as a direct body +// value and as a part inside an array. +describe("Blob from ArrayBuffer-like values", () => { + const bytes = Uint8Array.from({ length: 16 }, (_, i) => i + 1); + const views = [ + ["ArrayBuffer", () => bytes.slice().buffer], + ["DataView", () => new DataView(bytes.slice().buffer)], + ["Int8Array", () => new Int8Array(bytes.slice().buffer)], + ["Uint8Array", () => bytes.slice()], + ["Uint8ClampedArray", () => new Uint8ClampedArray(bytes.slice().buffer)], + ["Int16Array", () => new Int16Array(bytes.slice().buffer)], + ["Uint16Array", () => new Uint16Array(bytes.slice().buffer)], + ["Int32Array", () => new Int32Array(bytes.slice().buffer)], + ["Uint32Array", () => new Uint32Array(bytes.slice().buffer)], + ["Float16Array", () => new Float16Array(bytes.slice().buffer)], + ["Float32Array", () => new Float32Array(bytes.slice().buffer)], + ["Float64Array", () => new Float64Array(bytes.slice().buffer)], + ["BigInt64Array", () => new BigInt64Array(bytes.slice().buffer)], + ["BigUint64Array", () => new BigUint64Array(bytes.slice().buffer)], + ] as const; + + test.each(views)("new Blob([%s]) copies the bytes", async (_name, make) => { + const view = make(); + const blob = new Blob([view as any]); + expect(blob.size).toBe(view.byteLength); + expect(await blob.bytes()).toEqual(bytes); + }); + + test.each(views)("new Response(%s).blob() copies the bytes", async (_name, make) => { + const view = make(); + const blob = await new Response(view as any).blob(); + expect(blob.size).toBe(view.byteLength); + expect(await blob.bytes()).toEqual(bytes); + }); + + test("string, view, and Blob parts concatenate in order", async () => { + const blob = new Blob([ + "ab", + new Uint8Array([0x63, 0x64]), + new Blob(["ef"]), + new DataView(new Uint8Array([0x67, 0x68]).buffer), + ]); + expect(await blob.text()).toBe("abcdefgh"); + }); +});