From ed130264ab5962ec45ad44caad5dc7b34d79bfe2 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 5 Jul 2026 04:53:29 +0000 Subject: [PATCH 1/2] ffi: toArrayBuffer/toBuffer throw RangeError instead of aborting on a huge byteLength toArrayBuffer(ptr, 0, byteLength) aborted the process for any byteLength at or above 2^32: ArrayBuffer::from_bytes narrowed the length through u32::try_from(..).expect("int cast"), a leftover from when the descriptor's len/byte_len were u32. They are usize now, and the C ABI they mirror (Bun__ArrayBuffer) has always used size_t, so the cast only served to panic. toBuffer aborted the same way past 2^32, there by tripping JSC's RELEASE_ASSERT(m_sizeInBytes <= MAX_ARRAY_BUFFER_SIZE). Drop the narrowing casts and bound the byteLength in the FFI layer instead. The limit is MAX_ARRAY_BUFFER_SIZE (2^32 inclusive, what new ArrayBuffer(2**32) and require("buffer").kMaxLength already accept), so a caller with a mapping larger than that now gets a RangeError it can handle by windowing. The bound covers the NUL-scan path too, not just an explicit byteLength. ArrayBuffer::MAX_SIZE is documented as kMaxLength but held u32::MAX; correct it and use it. Its one other consumer, node:crypto's MAX_POSSIBLE_LENGTH = min(MAX_SIZE, i32::MAX), is unchanged. CString keeps the address bound: its result is capped by WTF::String::MaxLength in code units, which a UTF-8 byte count does not map onto. get_ptr_slice handed its validation failures back as Error objects in the return slot rather than throwing them, which would have left the new RangeError uncatchable. Make them throw, along with the finalizer-argument checks in toArrayBuffer/toBuffer. A "cstring" symbol whose C function returns one of the sentinel addresses now throws from the call rather than yielding a CString whose text is the error message. --- src/jsc/array_buffer.rs | 15 +- src/runtime/ffi/FFIObject.rs | 295 ++++++++++++++++++----------------- test/js/bun/ffi/ffi.test.js | 76 +++++++++ 3 files changed, 240 insertions(+), 146 deletions(-) diff --git a/src/jsc/array_buffer.rs b/src/jsc/array_buffer.rs index 01a8e12a08af..5f15fa403a79 100644 --- a/src/jsc/array_buffer.rs +++ b/src/jsc/array_buffer.rs @@ -1,4 +1,4 @@ -use core::ffi::{c_uint, c_void}; +use core::ffi::c_void; use core::ptr; use crate as jsc; @@ -155,9 +155,10 @@ impl ArrayBuffer { self.value.unpin_array_buffer(); } - // require('buffer').kMaxLength. + // The largest byte length JavaScriptCore backs an ArrayBuffer with + // (`MAX_ARRAY_BUFFER_SIZE`), exposed to JS as `require('buffer').kMaxLength`. // keep in sync with Bun::Buffer::kMaxLength - pub const MAX_SIZE: c_uint = c_uint::MAX; + pub const MAX_SIZE: usize = 1 << 32; // 4 MB or so is pretty good for mmap() const MMAP_THRESHOLD: usize = 1024 * 1024 * 4; @@ -424,8 +425,8 @@ impl ArrayBuffer { pub fn from_bytes(bytes: &mut [u8], typed_array_type: JSType) -> ArrayBuffer { ArrayBuffer { - len: u32::try_from(bytes.len()).expect("int cast") as usize, - byte_len: u32::try_from(bytes.len()).expect("int cast") as usize, + len: bytes.len(), + byte_len: bytes.len(), typed_array_type, ptr: bytes.as_mut_ptr(), ..Default::default() @@ -446,8 +447,8 @@ impl ArrayBuffer { // this is an FFI hand-off, not a leak. let ptr = bun_core::heap::into_raw(bytes).cast::(); ArrayBuffer { - len: u32::try_from(len).expect("int cast") as usize, - byte_len: u32::try_from(len).expect("int cast") as usize, + len, + byte_len: len, typed_array_type, ptr, ..Default::default() diff --git a/src/runtime/ffi/FFIObject.rs b/src/runtime/ffi/FFIObject.rs index 01d678418cea..5f9d2db671ee 100644 --- a/src/runtime/ffi/FFIObject.rs +++ b/src/runtime/ffi/FFIObject.rs @@ -80,14 +80,18 @@ pub(crate) fn new_cstring( byte_offset: Option, length_value: Option, ) -> JsResult { - match get_ptr_slice(global_this, value, byte_offset, length_value) { - ValueOrError::Err(err) => Ok(err), - ValueOrError::Slice(ptr, len) => { - // SAFETY: ptr/len point to FFI-owned memory whose lifetime the caller guarantees. - let bytes = unsafe { core::slice::from_raw_parts(ptr, len) }; - jsc::bun_string_jsc::create_utf8_for_js(global_this, bytes) - } - } + // A C string's JS representation is capped by `WTF::String::MaxLength` in code + // units, which a UTF-8 byte count does not map onto; only the address is bounded. + let (ptr, len) = get_ptr_slice( + global_this, + value, + byte_offset, + length_value, + MAX_ADDRESSABLE_MEMORY, + )?; + // SAFETY: ptr/len point to FFI-owned memory whose lifetime the caller guarantees. + let bytes = unsafe { core::slice::from_raw_parts(ptr, len) }; + jsc::bun_string_jsc::create_utf8_for_js(global_this, bytes) } // DOMJIT fast-path descriptor + slow-path host fn, represented here as a const @@ -484,35 +488,30 @@ fn ptr_(global_this: &JSGlobalObject, value: JSValue, byte_offset: Option, byte_length: Option, -) -> ValueOrError { + max_byte_length: usize, +) -> JsResult<(*mut u8, usize)> { if !value.is_number() || value.as_number() < 0.0 || value.as_number() > usize::MAX as f64 { - return ValueOrError::Err( - global_this.to_invalid_arguments(format_args!("ptr must be a number.")), - ); + return Err(global_this.throw_invalid_arguments(format_args!("ptr must be a number."))); } let num = value.as_ptr_address(); if num == 0 { - return ValueOrError::Err(global_this.to_invalid_arguments(format_args!( + return Err(global_this.throw_invalid_arguments(format_args!( "ptr cannot be zero, that would segfault Bun :(" ))); } @@ -529,69 +528,81 @@ fn get_ptr_slice( } if addr == 0 { - return ValueOrError::Err(global_this.to_invalid_arguments(format_args!( + return Err(global_this.throw_invalid_arguments(format_args!( "ptr cannot be zero, that would segfault Bun :(" ))); } if !byte_off.as_number().is_finite() { - return ValueOrError::Err( - global_this.to_invalid_arguments(format_args!("ptr must be a finite number.")), - ); + return Err(global_this + .throw_invalid_arguments(format_args!("ptr must be a finite number."))); } } else if !byte_off.is_empty_or_undefined_or_null() { // do nothing } else { - return ValueOrError::Err( - global_this.to_invalid_arguments(format_args!("Expected number for byteOffset")), + return Err( + global_this.throw_invalid_arguments(format_args!("Expected number for byteOffset")) ); } } if addr == 0xDEADBEEF || addr == 0xaaaaaaaa || addr == 0xAAAAAAAA { - return ValueOrError::Err(global_this.to_invalid_arguments(format_args!( + return Err(global_this.throw_invalid_arguments(format_args!( "ptr to invalid memory, that would segfault Bun :(" ))); } - if let Some(value_length) = byte_length { - if !value_length.is_empty_or_undefined_or_null() { + let explicit_length = byte_length.filter(|len| !len.is_empty_or_undefined_or_null()); + let length_i = match explicit_length { + Some(value_length) => { if !value_length.is_number() { - return ValueOrError::Err( - global_this.to_invalid_arguments(format_args!("length must be a number.")), + return Err( + global_this.throw_invalid_arguments(format_args!("length must be a number.")) ); } if value_length.as_number() == 0.0 { - return ValueOrError::Err(global_this.to_invalid_arguments(format_args!( + return Err(global_this.throw_invalid_arguments(format_args!( "length must be > 0. This usually means a bug in your code." ))); } let length_i = value_length.to_int64(); if length_i < 0 { - return ValueOrError::Err(global_this.to_invalid_arguments(format_args!( + return Err(global_this.throw_invalid_arguments(format_args!( "length must be > 0. This usually means a bug in your code." ))); } - if length_i > i64::try_from(MAX_ADDRESSABLE_MEMORY).expect("int cast") { - return ValueOrError::Err(global_this.to_invalid_arguments(format_args!( - "length exceeds max addressable memory. This usually means a bug in your code." - ))); - } - - let length = usize::try_from(length_i).expect("int cast"); - return ValueOrError::Slice(addr as *mut u8, length); + length_i + } + // Scan for the NUL terminator. The scanned length is bounded too: a C + // string longer than the result object can represent aborts inside JSC. + None => { + // SAFETY: caller asserts `addr` points at a NUL-terminated C string. + let len = unsafe { bun_core::ffi::cstr(addr as *const core::ffi::c_char) } + .to_bytes() + .len(); + i64::try_from(len).expect("int cast") } + }; + + let max = i64::try_from(max_byte_length).expect("int cast"); + if length_i > max { + return Err(global_this.throw_range_error( + length_i, + jsc::RangeErrorOptions { + field_name: b"byteLength", + max, + ..Default::default() + }, + )); } - // Scan for the NUL terminator. - // SAFETY: caller asserts `addr` points at a NUL-terminated C string. - let len = unsafe { bun_core::ffi::cstr(addr as *const core::ffi::c_char) } - .to_bytes() - .len(); - ValueOrError::Slice(addr as *mut u8, len) + Ok(( + addr as *mut u8, + usize::try_from(length_i).expect("int cast"), + )) } fn get_cptr(value: JSValue) -> Option { @@ -619,56 +630,59 @@ pub(crate) fn to_array_buffer( finalization_ctx_or_ptr: Option, finalization_callback: Option, ) -> JsResult { - match get_ptr_slice(global_this, value, byte_offset, value_length) { - ValueOrError::Err(erro) => Ok(erro), - ValueOrError::Slice(ptr, len) => { - let mut callback: jsc::JSTypedArrayBytesDeallocator = None; - let mut ctx: Option<*mut c_void> = None; - if let Some(callback_value) = finalization_callback { - if let Some(callback_ptr) = get_cptr(callback_value) { - // SAFETY: user-supplied raw fn pointer address. - callback = unsafe { deallocator_from_addr(callback_ptr) }; - - if let Some(ctx_value) = finalization_ctx_or_ptr { - if let Some(ctx_ptr) = get_cptr(ctx_value) { - ctx = Some(ctx_ptr as *mut c_void); - } else if !ctx_value.is_undefined_or_null() { - return Ok(global_this.to_invalid_arguments(format_args!( - "Expected user data to be a C pointer (number or BigInt)" - ))); - } - } - } else if !callback_value.is_empty_or_undefined_or_null() { - return Ok(global_this.to_invalid_arguments(format_args!( - "Expected callback to be a C pointer (number or BigInt)" + let (ptr, len) = get_ptr_slice( + global_this, + value, + byte_offset, + value_length, + ArrayBuffer::MAX_SIZE, + )?; + + let mut callback: jsc::JSTypedArrayBytesDeallocator = None; + let mut ctx: Option<*mut c_void> = None; + if let Some(callback_value) = finalization_callback { + if let Some(callback_ptr) = get_cptr(callback_value) { + // SAFETY: user-supplied raw fn pointer address. + callback = unsafe { deallocator_from_addr(callback_ptr) }; + + if let Some(ctx_value) = finalization_ctx_or_ptr { + if let Some(ctx_ptr) = get_cptr(ctx_value) { + ctx = Some(ctx_ptr as *mut c_void); + } else if !ctx_value.is_undefined_or_null() { + return Err(global_this.throw_invalid_arguments(format_args!( + "Expected user data to be a C pointer (number or BigInt)" ))); } - } else if let Some(callback_value) = finalization_ctx_or_ptr { - if let Some(callback_ptr) = get_cptr(callback_value) { - // SAFETY: user-supplied raw fn pointer address. - callback = unsafe { deallocator_from_addr(callback_ptr) }; - } else if !callback_value.is_empty_or_undefined_or_null() { - return Ok(global_this.to_invalid_arguments(format_args!( - "Expected callback to be a C pointer (number or BigInt)" - ))); - } - } - - // SAFETY: ptr/len came from get_ptr_slice; FFI-owned memory. The - // `bun:ffi` user asserts the pointer stays valid for the object's - // lifetime and that their finalization callback/ctx pair, if - // provided, is sound to invoke once at GC — `toArrayBuffer(ptr, - // ...)` is an inherently trusting FFI API. - unsafe { - let slice = core::slice::from_raw_parts_mut(ptr, len); - ArrayBuffer::from_bytes(slice, jsc::JSType::ArrayBuffer).to_js_with_context( - global_this, - ctx.unwrap_or(core::ptr::null_mut()), - callback, - ) } + } else if !callback_value.is_empty_or_undefined_or_null() { + return Err(global_this.throw_invalid_arguments(format_args!( + "Expected callback to be a C pointer (number or BigInt)" + ))); + } + } else if let Some(callback_value) = finalization_ctx_or_ptr { + if let Some(callback_ptr) = get_cptr(callback_value) { + // SAFETY: user-supplied raw fn pointer address. + callback = unsafe { deallocator_from_addr(callback_ptr) }; + } else if !callback_value.is_empty_or_undefined_or_null() { + return Err(global_this.throw_invalid_arguments(format_args!( + "Expected callback to be a C pointer (number or BigInt)" + ))); } } + + // SAFETY: ptr/len came from get_ptr_slice; FFI-owned memory. The + // `bun:ffi` user asserts the pointer stays valid for the object's + // lifetime and that their finalization callback/ctx pair, if + // provided, is sound to invoke once at GC — `toArrayBuffer(ptr, + // ...)` is an inherently trusting FFI API. + unsafe { + let slice = core::slice::from_raw_parts_mut(ptr, len); + ArrayBuffer::from_bytes(slice, jsc::JSType::ArrayBuffer).to_js_with_context( + global_this, + ctx.unwrap_or(core::ptr::null_mut()), + callback, + ) + } } pub(crate) fn to_buffer( @@ -679,57 +693,60 @@ pub(crate) fn to_buffer( finalization_ctx_or_ptr: Option, finalization_callback: Option, ) -> JsResult { - match get_ptr_slice(global_this, value, byte_offset, value_length) { - ValueOrError::Err(err) => Ok(err), - ValueOrError::Slice(ptr, len) => { - let mut callback: jsc::JSTypedArrayBytesDeallocator = None; - let mut ctx: Option<*mut c_void> = None; - if let Some(callback_value) = finalization_callback { - if let Some(callback_ptr) = get_cptr(callback_value) { - // SAFETY: user-supplied raw fn pointer address. - callback = unsafe { deallocator_from_addr(callback_ptr) }; - - if let Some(ctx_value) = finalization_ctx_or_ptr { - if let Some(ctx_ptr) = get_cptr(ctx_value) { - ctx = Some(ctx_ptr as *mut c_void); - } else if !ctx_value.is_empty_or_undefined_or_null() { - return Ok(global_this.to_invalid_arguments(format_args!( - "Expected user data to be a C pointer (number or BigInt)" - ))); - } - } - } else if !callback_value.is_empty_or_undefined_or_null() { - return Ok(global_this.to_invalid_arguments(format_args!( - "Expected callback to be a C pointer (number or BigInt)" - ))); - } - } else if let Some(callback_value) = finalization_ctx_or_ptr { - if let Some(callback_ptr) = get_cptr(callback_value) { - // SAFETY: user-supplied raw fn pointer address. - callback = unsafe { deallocator_from_addr(callback_ptr) }; - } else if !callback_value.is_empty_or_undefined_or_null() { - return Ok(global_this.to_invalid_arguments(format_args!( - "Expected callback to be a C pointer (number or BigInt)" + let (ptr, len) = get_ptr_slice( + global_this, + value, + byte_offset, + value_length, + ArrayBuffer::MAX_SIZE, + )?; + + let mut callback: jsc::JSTypedArrayBytesDeallocator = None; + let mut ctx: Option<*mut c_void> = None; + if let Some(callback_value) = finalization_callback { + if let Some(callback_ptr) = get_cptr(callback_value) { + // SAFETY: user-supplied raw fn pointer address. + callback = unsafe { deallocator_from_addr(callback_ptr) }; + + if let Some(ctx_value) = finalization_ctx_or_ptr { + if let Some(ctx_ptr) = get_cptr(ctx_value) { + ctx = Some(ctx_ptr as *mut c_void); + } else if !ctx_value.is_empty_or_undefined_or_null() { + return Err(global_this.throw_invalid_arguments(format_args!( + "Expected user data to be a C pointer (number or BigInt)" ))); } } - - // SAFETY: ptr/len came from get_ptr_slice; FFI-owned memory. - let slice = unsafe { core::slice::from_raw_parts_mut(ptr, len) }; - if callback.is_some() || ctx.is_some() { - return Ok(create_buffer_with_ctx( - global_this, - slice, - ctx.unwrap_or(core::ptr::null_mut()), - callback, - )); - } - - // `JSValue::create_buffer` installs `MarkedArrayBuffer_deallocator` so - // the slice is `mi_free`d on GC (including the free-foreign-memory footgun). - Ok(JSValue::create_buffer(global_this, slice)) + } else if !callback_value.is_empty_or_undefined_or_null() { + return Err(global_this.throw_invalid_arguments(format_args!( + "Expected callback to be a C pointer (number or BigInt)" + ))); } + } else if let Some(callback_value) = finalization_ctx_or_ptr { + if let Some(callback_ptr) = get_cptr(callback_value) { + // SAFETY: user-supplied raw fn pointer address. + callback = unsafe { deallocator_from_addr(callback_ptr) }; + } else if !callback_value.is_empty_or_undefined_or_null() { + return Err(global_this.throw_invalid_arguments(format_args!( + "Expected callback to be a C pointer (number or BigInt)" + ))); + } + } + + // SAFETY: ptr/len came from get_ptr_slice; FFI-owned memory. + let slice = unsafe { core::slice::from_raw_parts_mut(ptr, len) }; + if callback.is_some() || ctx.is_some() { + return Ok(create_buffer_with_ctx( + global_this, + slice, + ctx.unwrap_or(core::ptr::null_mut()), + callback, + )); } + + // `JSValue::create_buffer` installs `MarkedArrayBuffer_deallocator` so + // the slice is `mi_free`d on GC (including the free-foreign-memory footgun). + Ok(JSValue::create_buffer(global_this, slice)) } pub(crate) fn getter(global_object: &JSGlobalObject, _: &JSObject) -> JSValue { diff --git a/test/js/bun/ffi/ffi.test.js b/test/js/bun/ffi/ffi.test.js index 2546e1d1fcfd..9aa6ccb11708 100644 --- a/test/js/bun/ffi/ffi.test.js +++ b/test/js/bun/ffi/ffi.test.js @@ -707,6 +707,82 @@ it.skipIf(!isWindows || isFFIUnavailable)("dlopen accepts non-ASCII library path }); }); +// `expect(fn).toThrow()` is satisfied by a *returned* Error, so spell the +// difference out: these used to hand the caller an Error object as the result. +it("toArrayBuffer and toBuffer throw argument errors instead of returning them", () => { + const address = ptr(new Uint8Array(8)); + const outcome = view => { + try { + const result = view(); + return `returned ${result instanceof Error ? result.constructor.name : typeof result}`; + } catch (error) { + return `threw ${error.constructor.name}: ${error.message}`; + } + }; + + expect([ + outcome(() => toArrayBuffer(0)), + outcome(() => toBuffer(0)), + outcome(() => toArrayBuffer(address, 0, 0)), + outcome(() => toBuffer(address, 0, -1)), + outcome(() => toArrayBuffer(address, 0, 8, "not a pointer")), + outcome(() => toBuffer(address, 0, 8, "not a pointer")), + ]).toEqual([ + "threw TypeError: ptr cannot be zero, that would segfault Bun :(", + "threw TypeError: ptr cannot be zero, that would segfault Bun :(", + "threw TypeError: length must be > 0. This usually means a bug in your code.", + "threw TypeError: length must be > 0. This usually means a bug in your code.", + "threw TypeError: Expected callback to be a C pointer (number or BigInt)", + "threw TypeError: Expected callback to be a C pointer (number or BigInt)", + ]); +}); + +// A byteLength JSC cannot back an ArrayBuffer with used to abort the process: +// toArrayBuffer panicked on an int cast, toBuffer tripped a JSC RELEASE_ASSERT. +it("toArrayBuffer and toBuffer reject a byteLength past the max ArrayBuffer size", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + `import { ptr, toArrayBuffer, toBuffer } from "bun:ffi"; + const address = ptr(new Uint8Array(64)); + for (const [name, view] of [["toArrayBuffer", toArrayBuffer], ["toBuffer", toBuffer]]) { + for (const byteLength of [2 ** 32 + 1, 2 ** 33, Number.MAX_SAFE_INTEGER]) { + try { + view(address, 0, byteLength); + console.log(name, byteLength, "did not throw"); + } catch (e) { + console.log(name, e.constructor.name, e.code, e.message); + } + } + } + // exactly MAX_ARRAY_BUFFER_SIZE is what new ArrayBuffer(2 ** 32) accepts. + // Held alive so neither view's backing store is finalized before exit. + globalThis.views = [toArrayBuffer(address, 0, 2 ** 32), toBuffer(address, 0, 2 ** 32)]; + console.log("at the limit", globalThis.views[0].byteLength, globalThis.views[1].byteLength);`, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + const outOfRange = "is out of range. It must be <= 4294967296. Received"; + expect({ stdout: stdout.split("\n"), stderr, exitCode }).toEqual({ + stdout: [ + `toArrayBuffer RangeError ERR_OUT_OF_RANGE The value of "byteLength" ${outOfRange} 4294967297`, + `toArrayBuffer RangeError ERR_OUT_OF_RANGE The value of "byteLength" ${outOfRange} 8589934592`, + `toArrayBuffer RangeError ERR_OUT_OF_RANGE The value of "byteLength" ${outOfRange} 9007199254740991`, + `toBuffer RangeError ERR_OUT_OF_RANGE The value of "byteLength" ${outOfRange} 4294967297`, + `toBuffer RangeError ERR_OUT_OF_RANGE The value of "byteLength" ${outOfRange} 8589934592`, + `toBuffer RangeError ERR_OUT_OF_RANGE The value of "byteLength" ${outOfRange} 9007199254740991`, + "at the limit 4294967296 4294967296", + "", + ], + stderr: "", + exitCode: 0, + }); +}); + it('suffix does not start with a "."', () => { expect(suffix).not.toMatch(/^\./); }); From 75bc1fcc275096aba4a6c121e0b90bc303ed77a7 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 5 Jul 2026 06:41:00 +0000 Subject: [PATCH 2/2] ffi: accept an omitted byteOffset, reject a non-number one The two else-if arms were inverted, so `toArrayBuffer(ptr, undefined, len)` errored while `toArrayBuffer(ptr, "garbage", len)` silently ignored the bad offset. CString#arrayBuffer passes a byteOffset that defaults to undefined, so it cached a TypeError as its ArrayBuffer. `ptr()` already has the right polarity; match it. The byteLength test held a toBuffer view over a pointer bun does not own. toBuffer without a finalizer installs MarkedArrayBuffer_deallocator, which is libc free under cfg(bun_asan), and BUN_DESTRUCT_VM_ON_EXIT=1 on the ASan lane finalizes it at teardown: a bad-free on JSC memory. toArrayBuffer installs no deallocator, so it alone holds the at-the-limit view. --- src/runtime/ffi/FFIObject.rs | 4 ++-- test/js/bun/ffi/ffi.test.js | 39 ++++++++++++++++++++++++++++-------- 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/src/runtime/ffi/FFIObject.rs b/src/runtime/ffi/FFIObject.rs index 5f9d2db671ee..f734881aa8fd 100644 --- a/src/runtime/ffi/FFIObject.rs +++ b/src/runtime/ffi/FFIObject.rs @@ -537,8 +537,8 @@ fn get_ptr_slice( return Err(global_this .throw_invalid_arguments(format_args!("ptr must be a finite number."))); } - } else if !byte_off.is_empty_or_undefined_or_null() { - // do nothing + } else if byte_off.is_empty_or_undefined_or_null() { + // an omitted byteOffset leaves `addr` alone } else { return Err( global_this.throw_invalid_arguments(format_args!("Expected number for byteOffset")) diff --git a/test/js/bun/ffi/ffi.test.js b/test/js/bun/ffi/ffi.test.js index 9aa6ccb11708..6f82fcceeda9 100644 --- a/test/js/bun/ffi/ffi.test.js +++ b/test/js/bun/ffi/ffi.test.js @@ -737,6 +737,28 @@ it("toArrayBuffer and toBuffer throw argument errors instead of returning them", ]); }); +// An omitted byteOffset used to be the error case and a non-number the +// silently-ignored one, so CString#arrayBuffer (byteOffset defaults to +// undefined) cached a TypeError as its ArrayBuffer. +it("toArrayBuffer accepts an omitted byteOffset and rejects a non-number one", () => { + const bytes = new Uint8Array(16); + bytes.set(Buffer.from("hi\0")); + const address = ptr(bytes); + + expect(toArrayBuffer(address, undefined, 8).byteLength).toBe(8); + expect(toArrayBuffer(address, null, 8).byteLength).toBe(8); + expect(() => toArrayBuffer(address, "garbage", 8)).toThrow("Expected number for byteOffset"); + expect(() => toArrayBuffer(address, {}, 8)).toThrow("Expected number for byteOffset"); + + const arrayBuffer = new CString(address).arrayBuffer; + expect(arrayBuffer).toBeInstanceOf(ArrayBuffer); + expect(new TextDecoder().decode(arrayBuffer)).toBe("hi"); + + // the view aliases `bytes`, which also keeps it reachable across the decode + new Uint8Array(arrayBuffer)[0] = "H".charCodeAt(0); + expect(bytes[0]).toBe("H".charCodeAt(0)); +}); + // A byteLength JSC cannot back an ArrayBuffer with used to abort the process: // toArrayBuffer panicked on an int cast, toBuffer tripped a JSC RELEASE_ASSERT. it("toArrayBuffer and toBuffer reject a byteLength past the max ArrayBuffer size", async () => { @@ -745,7 +767,8 @@ it("toArrayBuffer and toBuffer reject a byteLength past the max ArrayBuffer size bunExe(), "-e", `import { ptr, toArrayBuffer, toBuffer } from "bun:ffi"; - const address = ptr(new Uint8Array(64)); + const backing = new Uint8Array(64); + const address = ptr(backing); for (const [name, view] of [["toArrayBuffer", toArrayBuffer], ["toBuffer", toBuffer]]) { for (const byteLength of [2 ** 32 + 1, 2 ** 33, Number.MAX_SAFE_INTEGER]) { try { @@ -757,17 +780,18 @@ it("toArrayBuffer and toBuffer reject a byteLength past the max ArrayBuffer size } } // exactly MAX_ARRAY_BUFFER_SIZE is what new ArrayBuffer(2 ** 32) accepts. - // Held alive so neither view's backing store is finalized before exit. - globalThis.views = [toArrayBuffer(address, 0, 2 ** 32), toBuffer(address, 0, 2 ** 32)]; - console.log("at the limit", globalThis.views[0].byteLength, globalThis.views[1].byteLength);`, + // Only toArrayBuffer holds a view here: toBuffer without a finalizer installs + // a deallocator that frees a pointer it does not own, which VM teardown runs. + console.log("at the limit", toArrayBuffer(address, 0, 2 ** 32).byteLength, backing.length);`, ], env: bunEnv, stdout: "pipe", stderr: "pipe", }); - const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + // stderr is drained, not asserted: ASAN and debug builds emit benign warnings. + const [stdout, , exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); const outOfRange = "is out of range. It must be <= 4294967296. Received"; - expect({ stdout: stdout.split("\n"), stderr, exitCode }).toEqual({ + expect({ stdout: stdout.split("\n"), exitCode }).toEqual({ stdout: [ `toArrayBuffer RangeError ERR_OUT_OF_RANGE The value of "byteLength" ${outOfRange} 4294967297`, `toArrayBuffer RangeError ERR_OUT_OF_RANGE The value of "byteLength" ${outOfRange} 8589934592`, @@ -775,10 +799,9 @@ it("toArrayBuffer and toBuffer reject a byteLength past the max ArrayBuffer size `toBuffer RangeError ERR_OUT_OF_RANGE The value of "byteLength" ${outOfRange} 4294967297`, `toBuffer RangeError ERR_OUT_OF_RANGE The value of "byteLength" ${outOfRange} 8589934592`, `toBuffer RangeError ERR_OUT_OF_RANGE The value of "byteLength" ${outOfRange} 9007199254740991`, - "at the limit 4294967296 4294967296", + "at the limit 4294967296 64", "", ], - stderr: "", exitCode: 0, }); });