diff --git a/src/js/builtins/shell.ts b/src/js/builtins/shell.ts index df705d48d2d1..72dcf6df761a 100644 --- a/src/js/builtins/shell.ts +++ b/src/js/builtins/shell.ts @@ -1,7 +1,7 @@ export function createBunShellTemplateFunction(createShellInterpreter_, createParsedShellScript_) { const createShellInterpreter = createShellInterpreter_ as ( resolve: (code: number, stdout: Buffer, stderr: Buffer) => void, - reject: (code: number, stdout: Buffer, stderr: Buffer) => void, + reject: (error: unknown) => void, args: $ZigGeneratedClasses.ParsedShellScript, ) => $ZigGeneratedClasses.ShellInterpreter; const createParsedShellScript = createParsedShellScript_ as ( @@ -108,7 +108,7 @@ export function createBunShellTemplateFunction(createShellInterpreter_, createPa #hasRun: boolean = false; #throws: boolean = true; #resolve: (code: number, stdout: Buffer, stderr: Buffer) => void; - #reject: (code: number, stdout: Buffer, stderr: Buffer) => void; + #reject: (error: unknown) => void; constructor(args: $ZigGeneratedClasses.ParsedShellScript, throws: boolean) { // Create the error immediately so it captures the stacktrace at the point @@ -131,9 +131,10 @@ export function createBunShellTemplateFunction(createShellInterpreter_, createPa res(out); } }; - reject = (code, stdout, stderr) => { - potentialError!.initialize(new ShellOutput(stdout, stderr, code), code); - rej(potentialError); + // Only for a failure to build the output buffers; exit codes go through `resolve`. + reject = error => { + potentialError = undefined; + rej(error); }; }); diff --git a/src/jsc/array_buffer.rs b/src/jsc/array_buffer.rs index 656767cab1e5..17c9b34b6345 100644 --- a/src/jsc/array_buffer.rs +++ b/src/jsc/array_buffer.rs @@ -181,18 +181,19 @@ impl ArrayBuffer { impl ArrayBuffer { /// Only use this when reading from the file descriptor is _very_ cheap. Like, for example, an in-memory file descriptor. /// Do not use this for pipes, however tempting it may seem. - pub(crate) fn to_js_buffer_from_fd(fd: Fd, size: usize, global: &JSGlobalObject) -> JSValue { + pub(crate) fn to_js_buffer_from_fd( + fd: Fd, + size: usize, + global: &JSGlobalObject, + ) -> JsResult { // SAFETY: FFI — `global` is a live &JSGlobalObject (opaque ZST handle, coerces to // *const); fn accepts null ptr with explicit size. // Wrapped in `from_js_host_call` so the C++ throw scope opened by // `Bun__createUint8ArrayForCopy` is checked before `as_array_buffer` below // declares `ASSERT_NO_PENDING_EXCEPTION` (validateExceptionChecks). - let buffer_value = match crate::host_fn::from_js_host_call(global, || unsafe { + let buffer_value = crate::host_fn::from_js_host_call(global, || unsafe { Bun__createUint8ArrayForCopy(global, ptr::null(), size, true) - }) { - Ok(v) => v, - Err(_) => return JSValue::ZERO, - }; + })?; let mut array_buffer = buffer_value.as_array_buffer(global).expect("Unexpected"); let mut bytes = array_buffer.byte_slice_mut(); @@ -214,16 +215,14 @@ impl ArrayBuffer { } } bun_sys::Result::Err(err) => { - let err_js = err.to_js(global); - let _ = global.throw_value(err_js); - return JSValue::ZERO; + return Err(global.throw_value(err.to_js(global))); } } } buffer_value.ensure_still_alive(); - buffer_value + Ok(buffer_value) } #[inline] @@ -263,7 +262,7 @@ impl ArrayBuffer { let result = Self::to_js_buffer_from_fd(fd, usize::try_from(size).expect("int cast"), global); fd.close(); - return Ok(result); + return result; } // bun_sys::mmap takes raw i32 prot/flags. @@ -278,14 +277,12 @@ impl ArrayBuffer { match result { bun_sys::Result::Ok(buf) => { - // `buf` is a fresh mmap region whose ownership transfers to JSC. - Ok(JSBuffer__fromMmap(global, buf.cast(), map_len)) - } - bun_sys::Result::Err(err) => { - let err_js = err.to_js(global); - let _ = global.throw_value(err_js); - Ok(JSValue::ZERO) + // `buf` is a fresh mmap region whose ownership transfers to JSC (on `Err` too). + crate::host_fn::from_js_host_call(global, || { + JSBuffer__fromMmap(global, buf.cast(), map_len) + }) } + bun_sys::Result::Err(err) => Err(global.throw_value(err.to_js(global))), } } diff --git a/src/jsc/bindings/JSBuffer.cpp b/src/jsc/bindings/JSBuffer.cpp index bc56bc63e42d..0b0f24574d63 100644 --- a/src/jsc/bindings/JSBuffer.cpp +++ b/src/jsc/bindings/JSBuffer.cpp @@ -380,16 +380,30 @@ template<> class IDLOperation { } +bool Bun::rejectBytesNoCopyAboveArrayBufferLimit(JSC::JSGlobalObject* globalObject, JSC::ThrowScope& scope, const void* bytes, size_t length, JSTypedArrayBytesDeallocator deallocator, void* deallocatorContext) +{ + if (length <= MAX_ARRAY_BUFFER_SIZE) [[likely]] + return false; + + if (deallocator) + deallocator(const_cast(bytes), deallocatorContext); + JSC::throwOutOfMemoryError(globalObject, scope); + return true; +} + JSC::EncodedJSValue JSBuffer__bufferFromPointerAndLengthAndDeinit(JSC::JSGlobalObject* lexicalGlobalObject, char* ptr, size_t length, void* ctx, JSTypedArrayBytesDeallocator bytesDeallocator) { JSC::JSUint8Array* uint8Array = nullptr; auto* globalObject = defaultGlobalObject(lexicalGlobalObject); auto* subclassStructure = globalObject->JSBufferSubclassStructure(); - auto scope = DECLARE_TOP_EXCEPTION_SCOPE(lexicalGlobalObject->vm()); + auto scope = DECLARE_THROW_SCOPE(lexicalGlobalObject->vm()); if (length > 0) [[likely]] { ASSERT(bytesDeallocator); + if (Bun::rejectBytesNoCopyAboveArrayBufferLimit(lexicalGlobalObject, scope, ptr, length, bytesDeallocator, ctx)) [[unlikely]] + return {}; + auto buffer = ArrayBuffer::createFromBytes({ reinterpret_cast(ptr), length }, createSharedTask([=](void* p) { bytesDeallocator(p, ctx); })); @@ -2585,19 +2599,29 @@ static JSC::EncodedJSValue jsBufferPrototypeFunction_writeBody(JSC::JSGlobalObje RELEASE_AND_RETURN(scope, writeToBuffer(lexicalGlobalObject, castedThis, str, offset, length, encoding)); } +static void unmapBufferBytes(void* mapping, void* lengthAsContext) +{ +#if !OS(WINDOWS) + munmap(mapping, reinterpret_cast(lengthAsContext)); +#else + UNUSED_PARAM(lengthAsContext); + UnmapViewOfFile(mapping); +#endif +} + extern "C" JSC::EncodedJSValue JSBuffer__fromMmap(Zig::GlobalObject* globalObject, void* ptr, size_t length) { auto& vm = JSC::getVM(globalObject); auto scope = DECLARE_THROW_SCOPE(vm); + void* lengthAsContext = reinterpret_cast(length); + if (Bun::rejectBytesNoCopyAboveArrayBufferLimit(globalObject, scope, ptr, length, unmapBufferBytes, lengthAsContext)) [[unlikely]] + return {}; + auto* structure = globalObject->JSBufferSubclassStructure(); - auto buffer = ArrayBuffer::createFromBytes({ static_cast(ptr), length }, createSharedTask([length](void* p) { -#if !OS(WINDOWS) - munmap(p, length); -#else - UnmapViewOfFile(p); -#endif + auto buffer = ArrayBuffer::createFromBytes({ static_cast(ptr), length }, createSharedTask([lengthAsContext](void* p) { + unmapBufferBytes(p, lengthAsContext); })); auto* view = JSC::JSUint8Array::create(globalObject, structure, WTF::move(buffer), 0, length); diff --git a/src/jsc/bindings/JSBuffer.h b/src/jsc/bindings/JSBuffer.h index 98e228f47106..269cfcb19673 100644 --- a/src/jsc/bindings/JSBuffer.h +++ b/src/jsc/bindings/JSBuffer.h @@ -23,6 +23,7 @@ #include "root.h" #include +#include #include #include "BufferEncodingType.h" @@ -39,6 +40,11 @@ namespace Bun { std::optional byteLength(JSC::JSString* str, JSC::JSGlobalObject* lexicalGlobalObject, WebCore::BufferEncodingType encoding); +// ArrayBuffer::createFromBytes RELEASE_ASSERTs above MAX_ARRAY_BUFFER_SIZE. Above +// it, this releases the adopted bytes through the deallocator, throws the +// RangeError `new ArrayBuffer(length)` would throw, and returns true. +bool rejectBytesNoCopyAboveArrayBufferLimit(JSC::JSGlobalObject*, JSC::ThrowScope&, const void* bytes, size_t length, JSTypedArrayBytesDeallocator, void* deallocatorContext); + namespace Buffer { const size_t kMaxLength = MAX_ARRAY_BUFFER_SIZE; diff --git a/src/runtime/api/bun/js_bun_spawn_bindings.rs b/src/runtime/api/bun/js_bun_spawn_bindings.rs index 935fa3fc5800..4c0157716f81 100644 --- a/src/runtime/api/bun/js_bun_spawn_bindings.rs +++ b/src/runtime/api/bun/js_bun_spawn_bindings.rs @@ -2031,6 +2031,8 @@ fn spawn_maybe_sync( } if global_this.has_exception() { // e.g. a termination exception. + // SAFETY: same as the `finalize` below; `subprocess` is not used after this line. + SubprocessT::finalize(unsafe { Box::from_raw(subprocess_ptr) }); return Ok(JSValue::ZERO); } @@ -2038,17 +2040,17 @@ fn spawn_maybe_sync( let signal_code = SubprocessT::get_signal_code(subprocess, global_this); let exit_code = SubprocessT::get_exit_code(subprocess, global_this); - let stdout = subprocess + // Propagated after `finalize`, which must run even when building the output throws. + let output = subprocess .stdout - .with_mut(|s| s.to_buffered_value(global_this))?; - let stderr = subprocess - .stderr - .with_mut(|s| s.to_buffered_value(global_this))?; - let resource_usage: JSValue = if !global_this.has_exception() { - subprocess.create_resource_usage_object(global_this)? - } else { - JSValue::ZERO - }; + .with_mut(|s| s.to_buffered_value(global_this)) + .and_then(|stdout| { + let stderr = subprocess + .stderr + .with_mut(|s| s.to_buffered_value(global_this))?; + let resource_usage = subprocess.create_resource_usage_object(global_this)?; + Ok((stdout, stderr, resource_usage)) + }); let exited_due_to_timeout = did_timeout; let exited_due_to_max_buffer = subprocess.exited_due_to_maxbuf.get(); let result_pid = JSValue::js_number_from_int32(subprocess.pid()); @@ -2056,6 +2058,7 @@ fn spawn_maybe_sync( // above (spawnSync path: never handed to a JS wrapper); reclaim ownership. // `subprocess` (`&mut *subprocess_ptr`) is not used after this line. SubprocessT::finalize(unsafe { Box::from_raw(subprocess_ptr) }); + let (stdout, stderr, resource_usage) = output?; let sync_value = JSValue::create_empty_object(global_this, 0); sync_value.put(global_this, b"exitCode", exit_code); diff --git a/src/runtime/api/bun/subprocess/Readable.rs b/src/runtime/api/bun/subprocess/Readable.rs index d59aa218de65..d696475b550f 100644 --- a/src/runtime/api/bun/subprocess/Readable.rs +++ b/src/runtime/api/bun/subprocess/Readable.rs @@ -1,7 +1,7 @@ use core::mem; use core::ptr::NonNull; -use bun_jsc::{self as jsc, JSGlobalObject, JSValue, JsResult, event_loop::EventLoop}; +use bun_jsc::{JSGlobalObject, JSValue, JsResult, event_loop::EventLoop}; use bun_sys::{self, Fd, FdExt as _}; use crate::node::types::FdJsc as _; @@ -280,7 +280,7 @@ impl Readable { { let fd = *fd; *self = Readable::Closed; - jsc::ArrayBuffer::to_js_buffer_from_memfd(fd, global) + bun_jsc::ArrayBuffer::to_js_buffer_from_memfd(fd, global) } } Readable::Pipe(_) => { @@ -300,14 +300,7 @@ impl Readable { Err(_) => return Err(global.throw_out_of_memory()), }; - // Ownership of the mimalloc-backed buffer transfers to JSC - // (freed via `MarkedArrayBuffer_deallocator`). - jsc::MarkedArrayBuffer { - buffer: jsc::ArrayBuffer::from_owned_bytes(own, jsc::JSType::Uint8Array), - owns_buffer: true, - pinned: false, - } - .to_node_buffer(global) + JSValue::create_buffer_from_box(global, own) } _ => Ok(JSValue::UNDEFINED), } diff --git a/src/runtime/api/bun/subprocess/SubprocessPipeReader.rs b/src/runtime/api/bun/subprocess/SubprocessPipeReader.rs index 074fb5b15617..5734f4cd4ef2 100644 --- a/src/runtime/api/bun/subprocess/SubprocessPipeReader.rs +++ b/src/runtime/api/bun/subprocess/SubprocessPipeReader.rs @@ -9,7 +9,7 @@ use bun_io::max_buf::MaxBuf; #[cfg(unix)] use bun_io::pipe_reader::PosixFlags; use bun_jsc::event_loop::EventLoop; -use bun_jsc::{self as jsc, JSGlobalObject, JSValue, JsResult, MarkedArrayBuffer}; +use bun_jsc::{JSGlobalObject, JSValue, JsResult}; use bun_ptr::ScopedRef; use bun_ptr::{IntrusiveRc, ParentRef, RefCount}; use bun_sys; @@ -334,14 +334,7 @@ impl PipeReader { match &mut self.state { State::Done(bytes) => { let bytes = core::mem::take(bytes); - // `state.done` is now empty via `take()`. - // `MarkedArrayBuffer::from_bytes` takes a borrowed `&mut [u8]` - // with `owns_buffer = true` (freed via mimalloc on the JS side); leak the - // boxed slice so JS becomes the owner — same pattern as - // `MarkedArrayBuffer::from_string`. - let slice: &'static mut [u8] = Box::leak(bytes.into_boxed_slice()); - MarkedArrayBuffer::from_bytes(slice, jsc::JSType::Uint8Array) - .to_node_buffer(global_this) + JSValue::create_buffer_from_box(global_this, bytes.into_boxed_slice()) } _ => Ok(JSValue::UNDEFINED), } diff --git a/src/runtime/shell/interpreter.rs b/src/runtime/shell/interpreter.rs index f0e6da780e48..13015b055a2d 100644 --- a/src/runtime/shell/interpreter.rs +++ b/src/runtime/shell/interpreter.rs @@ -1224,7 +1224,7 @@ impl Interpreter { ], ), Err(err) if !global_this.has_pending_termination_exception() => { - let error = global_this.take_exception(err); + let error = global_this.take_error(err); if let Some(reject) = JSShellInterpreter::reject_get_cached(this_jsvalue) { diff --git a/test/js/bun/ffi/ffi.test.js b/test/js/bun/ffi/ffi.test.js index aad15470fb5b..be18b1f206bb 100644 --- a/test/js/bun/ffi/ffi.test.js +++ b/test/js/bun/ffi/ffi.test.js @@ -1458,6 +1458,44 @@ describe("toBuffer borrowed-pointer ownership (no bad-free on GC)", () => { ); }); +// toBuffer hands an arbitrary (pointer, byteLength) pair straight to the Buffer +// hand-off that spawnSync, Bun.$ and others use for their output. That makes it +// the one way to reach the hand-off with a byteLength above kMaxLength (2^32) +// without 4 GiB of real bytes; the views below never touch the memory they +// describe. Subprocess because unpatched builds abort inside JSC. +describe("toBuffer at the Buffer length limit", () => { + it.concurrent("throws a RangeError above 2^32 bytes and still creates a view of exactly 2^32 bytes", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + import { ptr, toBuffer } from "bun:ffi"; + const backing = new Uint8Array(16); + let aboveLimit; + try { + aboveLimit = { length: toBuffer(ptr(backing), 0, 2 ** 32 + 1).length }; + } catch (e) { + aboveLimit = { isRangeError: e instanceof RangeError }; + } + const atLimit = { length: toBuffer(ptr(backing), 0, 2 ** 32).length }; + console.log(JSON.stringify({ aboveLimit, atLimit })); + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ result: JSON.parse(stdout.trim() || "null"), stderr, exitCode, signalCode: proc.signalCode }).toEqual({ + result: { aboveLimit: { isRangeError: true }, atLimit: { length: 2 ** 32 } }, + stderr: "", + exitCode: 0, + signalCode: null, + }); + }); +}); + describe.skipIf(!FFI_FIXTURE_PATH)("engine-native FFI (single implementation)", () => { const lib = FFI_FIXTURE_PATH; it("linkSymbols() binds and calls symbols from raw pointers", () => { diff --git a/test/js/bun/shell/shelloutput.test.ts b/test/js/bun/shell/shelloutput.test.ts index 29be7a8808fb..58dbf486ca26 100644 --- a/test/js/bun/shell/shelloutput.test.ts +++ b/test/js/bun/shell/shelloutput.test.ts @@ -1,5 +1,7 @@ import { $, ShellError, ShellPromise } from "bun"; import { describe, expect, test } from "bun:test"; +import { bunEnv, bunExe, isPosix } from "harness"; +import { totalmem } from "os"; describe("ShellOutput + ShellError", () => { test("output", async () => { @@ -25,6 +27,41 @@ describe("ShellOutput + ShellError", () => { }); }); +// The shell hands its captured stdout to JSC as a Buffer, which holds at most +// kMaxLength (2^32) bytes. A larger capture used to abort the process at that +// hand-off, and the rejection path behind it (the interpreter's `reject` +// callback) had never carried an error. This is the only way to exercise it. The +// child holds 4 GiB of zeros (8 GiB while its buffer grows), so this runs in a +// child process, with a long timeout, and only on machines with room. +describe.skipIf(!isPosix || totalmem() < 16 * 1024 ** 3)("stdout at the Buffer length limit", () => { + test("a capture of 2^32 + 1 bytes rejects with the RangeError that new ArrayBuffer(2 ** 32 + 1) throws", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const size = String(2 ** 32 + 1); + const result = await Bun.$\`head -c \${size} /dev/zero\`.quiet().then( + out => ({ length: out.stdout.length }), + e => ({ isRangeError: e instanceof RangeError, message: e.message }), + ); + console.log(JSON.stringify(result)); + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ result: JSON.parse(stdout.trim() || "null"), stderr, exitCode, signalCode: proc.signalCode }).toEqual({ + result: { isRangeError: true, message: "Out of memory" }, + stderr: "", + exitCode: 0, + signalCode: null, + }); + }, 120_000); +}); + async function withErr(promise: ShellPromise): Promise { let err: ShellError | undefined; try { diff --git a/test/js/bun/spawn/spawnSync.test.ts b/test/js/bun/spawn/spawnSync.test.ts index 6b6094ff9044..fdfdb3b6cf20 100644 --- a/test/js/bun/spawn/spawnSync.test.ts +++ b/test/js/bun/spawn/spawnSync.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "bun:test"; import { bunEnv, bunExe, bunRun, isLinux, isMusl, isPosix, isWindows } from "harness"; +import { totalmem } from "os"; import { join } from "path"; describe("spawnSync", () => { it("should throw a RangeError if timeout is less than 0", () => { @@ -99,6 +100,61 @@ describe("spawnSync", () => { }); }); +// A Buffer holds at most kMaxLength (2^32) bytes. spawnSync hands the captured +// output to JSC without a copy, and a larger output used to kill the process at +// that hand-off instead of throwing the RangeError an allocation of that size +// throws. An output of exactly 2^32 bytes used to die in a length cast on the +// same path. Each case makes the child hold 4 GiB of zeros (the read buffer +// doubles to 8 GiB on the way), so the cases run one at a time, in a child +// process, with a long timeout, and only on machines with room. +describe.skipIf(!isPosix || totalmem() < 16 * 1024 ** 3)("spawnSync output at the Buffer length limit", () => { + async function captureZeros(size: number) { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + let result; + try { + const { stdout, exitCode } = Bun.spawnSync({ + cmd: ["head", "-c", ${JSON.stringify(String(size))}, "/dev/zero"], + stdout: "pipe", + stderr: "pipe", + }); + result = { isBuffer: Buffer.isBuffer(stdout), length: stdout.length, exitCode }; + } catch (e) { + result = { isRangeError: e instanceof RangeError, message: e.message }; + } + console.log(JSON.stringify(result)); + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + return { result: JSON.parse(stdout.trim() || "null"), stderr, exitCode, signalCode: proc.signalCode }; + } + + it("an output of 2^32 + 1 bytes throws the RangeError that new ArrayBuffer(2 ** 32 + 1) throws", async () => { + expect(await captureZeros(2 ** 32 + 1)).toEqual({ + result: { isRangeError: true, message: "Out of memory" }, + stderr: "", + exitCode: 0, + signalCode: null, + }); + }, 120_000); + + it("an output of exactly 2^32 bytes is returned whole", async () => { + expect(await captureZeros(2 ** 32)).toEqual({ + result: { isBuffer: true, length: 2 ** 32, exitCode: 0 }, + stderr: "", + exitCode: 0, + signalCode: null, + }); + }, 120_000); +}); + describe("uid/gid", () => { const isRoot = process.getuid?.() === 0;