diff --git a/src/jsc/ConsoleObject.rs b/src/jsc/ConsoleObject.rs index 96dae5137792..5390b784e665 100644 --- a/src/jsc/ConsoleObject.rs +++ b/src/jsc/ConsoleObject.rs @@ -1729,10 +1729,7 @@ pub mod formatter { ordered_properties: false, custom_formatted_object: CustomFormattedObject::default(), disable_inspect_custom: false, - // `StackCheck::default()` has `cached_stack_end = 0` ⇒ the - // check always passes; callers that want a real bound - // overwrite with `StackCheck::init()` explicitly. - stack_check: StackCheck::default(), + stack_check: StackCheck::init(), can_throw_stack_overflow: false, error_display_level: ErrorDisplayLevel::Full, format_buffer_as_text: false, @@ -2010,6 +2007,7 @@ pub mod formatter { | Tag::Error | Tag::Class | Tag::Event + | Tag::JSX ) } } @@ -3387,9 +3385,6 @@ pub mod formatter { if self.global_this.has_exception() { return Err(jsc::JsError::Thrown); } - if !can_circ { - return Ok(true); - } if !self.stack_check.is_safe_to_recurse() { self.failed = true; @@ -3399,6 +3394,10 @@ pub mod formatter { return Ok(false); } + if !can_circ { + return Ok(true); + } + if self.map_node.is_none() { let mut node = core::ptr::NonNull::new(visited::Pool::get_node()) .expect("ObjectPool::get_node always returns a valid heap node"); diff --git a/src/jsc/VirtualMachine.rs b/src/jsc/VirtualMachine.rs index 5d715e84b68f..9a9b5d7df85f 100644 --- a/src/jsc/VirtualMachine.rs +++ b/src/jsc/VirtualMachine.rs @@ -5600,8 +5600,7 @@ impl VirtualMachine { // re-check here with an extra `MAX_PATH_BYTES * 3` of slack on Windows // to cover the transpiler's nested path buffers — same parity-level // protection the Object path gets from C++ `forEachProperty`'s - // `vm.isSafeToRecurse()`. The formatter's `stack_check` was seated by - // the caller (`format2` / `Bun.inspect`). + // `vm.isSafeToRecurse()`. let extra_headroom: usize = if cfg!(windows) { // 3× PathBuffer ≈ 288 KB — empirically enough for the // `remap_zig_exception` → `transpile_source_code` chain on the diff --git a/src/jsc/ipc.rs b/src/jsc/ipc.rs index 63514dd4b71f..4d77026d3016 100644 --- a/src/jsc/ipc.rs +++ b/src/jsc/ipc.rs @@ -372,7 +372,14 @@ mod advanced { } let message = &data[HEADER_LENGTH..][..message_len as usize]; - let deserialized = JSValue::deserialize(message, global)?; + let deserialized = match JSValue::deserialize(message, global) { + Ok(v) => v, + Err(JsError::Thrown) => { + global.clear_exception(); + return Err(IPCDecodeError::InvalidFormat); + } + Err(e) => return Err(e.into()), + }; Ok(DecodeIPCMessageResult { bytes_consumed: HEADER_LENGTH_U32 + message_len, diff --git a/src/runtime/api/YAMLObject.rs b/src/runtime/api/YAMLObject.rs index 5a72dcfbf3c1..5b3179bd8b7c 100644 --- a/src/runtime/api/YAMLObject.rs +++ b/src/runtime/api/YAMLObject.rs @@ -132,8 +132,10 @@ impl AnchorAlias { name: match origin { ValueOrigin::Root => AnchorAliasName::Root, ValueOrigin::ArrayItem => AnchorAliasName::ArrayItem(0), + // `prop_name` is a borrow of the iterator's `PropertyNameArray` + // entry; take a ref so it outlives that iterator. ValueOrigin::PropValue(prop_name) => AnchorAliasName::PropValue { - prop_name, + prop_name: OwnedString::new(prop_name.dupe_ref()), counter: 0, }, }, @@ -146,7 +148,7 @@ pub(crate) enum AnchorAliasName { Root, ArrayItem(usize), PropValue { - prop_name: BunString, + prop_name: OwnedString, // added after the name counter: usize, }, @@ -408,7 +410,7 @@ impl Stringifier { self.builder.append_latin1(b"value"); self.builder.append_usize(*counter); } else { - self.builder.append_string(*prop_name); + self.builder.append_string(prop_name.get()); if *counter != 0 { self.builder.append_usize(*counter); } diff --git a/src/runtime/test_runner/pretty_format.rs b/src/runtime/test_runner/pretty_format.rs index 0c59618aa3d4..e1b9d765a77e 100644 --- a/src/runtime/test_runner/pretty_format.rs +++ b/src/runtime/test_runner/pretty_format.rs @@ -3,7 +3,7 @@ use crate::test_runner::expect::JSValueTestExt; use core::ffi::c_void; use bun_collections::HashMap; -use bun_core::{fmt as bun_fmt, Output}; +use bun_core::{fmt as bun_fmt, Output, StackCheck}; use bun_jsc::{ self as jsc, ComptimeStringMapExt as _, JSGlobalObject, JSObject, JSPropertyIterator, JSType, JSValue, JsError, JsResult, VM, @@ -342,6 +342,7 @@ pub struct Formatter<'a> { pub failed: bool, pub estimated_line_length: usize, pub always_newline_scope: bool, + pub stack_check: StackCheck, } impl<'a> Formatter<'a> { @@ -357,6 +358,7 @@ impl<'a> Formatter<'a> { failed: false, estimated_line_length: 0, always_newline_scope: false, + stack_check: StackCheck::init(), } } @@ -504,7 +506,7 @@ impl Tag { #[inline] pub const fn can_have_circular_references(self) -> bool { - matches!(self, Tag::Array | Tag::Object | Tag::Map | Tag::Set) + matches!(self, Tag::Array | Tag::Object | Tag::Map | Tag::Set | Tag::JSX) } } @@ -1156,6 +1158,11 @@ impl<'a> Formatter<'a> { if self.failed { return Ok(()); } + if !self.stack_check.is_safe_to_recurse() { + self.failed = true; + let _ = writer_.write_all(b"..."); + return Ok(()); + } // reshaped for borrowck — `WrappedWriter` borrows both writer_ // and &mut self.estimated_line_length; we use a local wrapper and sync // `failed` at scope exit. estimated_line_length is unused by WrappedWriter diff --git a/test/js/bun/spawn/spawn.ipc.test.ts b/test/js/bun/spawn/spawn.ipc.test.ts index ab186a0a87a7..d73e17c2913e 100644 --- a/test/js/bun/spawn/spawn.ipc.test.ts +++ b/test/js/bun/spawn/spawn.ipc.test.ts @@ -157,6 +157,40 @@ describe("ipc mode advanced", () => { expect(exitCode).toBe(0); }, ); + + it("a malformed SerializedScriptValue payload closes the channel without leaving an uncaught TypeError", async () => { + // type=SerializedMessage (0x02), len=4, payload = SSV version 0xFFFFFFFF (> CurrentVersion) + // so CloneDeserializer throws TypeError("Unable to deserialize data."). The decoder must + // clear that exception and treat the frame as InvalidFormat; previously it left the + // exception pending and the parent saw it as an uncaught error. + const parent = ` + const child = Bun.spawn({ + cmd: [ + process.execPath, "-e", + 'require("fs").writeSync(3, Buffer.from([0x02, 0x04, 0x00, 0x00, 0x00, 0xff, 0xff, 0xff, 0xff]))', + ], + stdio: ["ignore", "inherit", "inherit"], + serialization: "advanced", + ipc(msg) { console.error("UNEXPECTED_IPC_MESSAGE", msg); }, + }); + await child.exited; + console.log("PARENT_OK"); + `; + + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", parent], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + + expect(stdout.trim()).toBe("PARENT_OK"); + expect(stderr).not.toContain("Unable to deserialize data"); + expect(stderr).not.toContain("UNEXPECTED_IPC_MESSAGE"); + expect(exitCode).toBe(0); + }); }); // getIPCInstance error path: on Windows, windowsConfigureClient can open the diff --git a/test/js/bun/test/pretty-format-overflow.test.ts b/test/js/bun/test/pretty-format-overflow.test.ts index 4acdf03b5110..c25805e40d02 100644 --- a/test/js/bun/test/pretty-format-overflow.test.ts +++ b/test/js/bun/test/pretty-format-overflow.test.ts @@ -37,7 +37,7 @@ test("deep nesting", () => { env: bunEnv, cwd: dir, stderr: "pipe", - stdout: "pipe", + stdout: "ignore", }); const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); @@ -50,4 +50,40 @@ test("deep nesting", () => { // Verify it actually formatted and showed the diff (not just crashed) expect(stderr).toContain("expect(received).toEqual(expected)"); }, 30000); + + test.each(["array", "object"])( + "toEqual diff of a %s nested past the native stack limit does not crash", + async kind => { + // `b` is shallow so toEqual returns a mismatch immediately; the crash was in + // the diff formatter rendering `a` afterwards. + const build = + kind === "array" + ? "let a = []; for (let i = 0; i < 30000; i++) a = [a];" + : "let a = {}; for (let i = 0; i < 30000; i++) a = {x: a};"; + const dir = tempDirWithFiles("pretty-format-stack", { + "deep.test.js": ` + import { test, expect } from "bun:test"; + test("deep", () => { + ${build} + expect(a).toEqual(${kind === "array" ? '["leaf"]' : '{ x: "leaf" }'}); + }); + `, + }); + + await using proc = Bun.spawn({ + cmd: [bunExe(), "test", "deep.test.js"], + env: bunEnv, + cwd: dir, + stderr: "pipe", + stdout: "ignore", + }); + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + + // The diff formatter stopped at the stack limit and the matcher threw its + // normal mismatch error, so the runner reports a failing test. + expect(stderr).toContain("expect(received).toEqual(expected)"); + expect(stderr).toContain("1 fail"); + expect(exitCode).toBe(1); + }, + ); }); diff --git a/test/js/bun/util/inspect.test.js b/test/js/bun/util/inspect.test.js index e76766e7f42f..a95dc341101e 100644 --- a/test/js/bun/util/inspect.test.js +++ b/test/js/bun/util/inspect.test.js @@ -316,6 +316,68 @@ it("jsx with fragment", () => { expect(input).toBe(output); }); +// Without the fix these overflow the native stack and segfault, so they run in a +// subprocess and assert on exit code + output rather than bringing down the runner. +describe("deep / self-referencing values do not overflow the formatter stack", () => { + it("self-referencing JSX element prints [Circular]", async () => { + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const a = { $$typeof: Symbol.for("react.element"), type: "div", props: null, key: null }; + a.props = { children: a }; + console.log(Bun.inspect(a)); + `, + ], + env: bunEnv, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect(stdout).toContain("[Circular]"); + expect(exitCode).toBe(0); + }); + + it.each([ + [ + "a deep JSX tree", + `let x = "leaf"; for (let i = 0; i < 100000; i++) x = { $$typeof: Symbol.for("react.element"), type: "div", props: { children: x }, key: null };`, + ], + ["a deep Proxy chain", `let x = {}; for (let i = 0; i < 100000; i++) x = new Proxy(x, {});`], + ])("console.log of %s throws RangeError instead of crashing", async (_, setup) => { + // print_jsx writes indent per level before recursing, so the partial console.log + // output before the stack check fires can be large; discard it and read the + // caught error name from stderr. + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", `${setup} try { console.log(x); } catch (e) { process.stderr.write("CAUGHT:" + e.name); }`], + env: bunEnv, + stdout: "ignore", + stderr: "pipe", + }); + const [stderr, exitCode] = await Promise.all([proc.stderr.text(), proc.exited]); + expect(stderr).toEndWith("CAUGHT:RangeError"); + expect(exitCode).toBe(0); + }); + + // print_array writes `[\n` + 2*indent spaces before recursing, so the partial + // output before the stack check fires is O(N^2); ignore stderr and assert on + // exitCode/signalCode (a segfault would be signalCode SIGSEGV, not exit 1). + it.each([ + ["throwing a deeply nested array as an uncaught exception", "throw a;"], + ["rejecting a deeply nested array as an unhandled rejection", "Promise.reject(a);"], + ])("%s does not crash the printer", async (_, stmt) => { + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", `let a = []; for (let i = 0; i < 60000; i++) a = [a]; ${stmt}`], + env: bunEnv, + stdout: "ignore", + stderr: "ignore", + }); + await proc.exited; + expect({ signalCode: proc.signalCode, exitCode: proc.exitCode }).toEqual({ signalCode: null, exitCode: 1 }); + }); +}); + it("inspect", () => { expect(Bun.inspect(new TypeError("what")).includes("TypeError: what")).toBe(true); expect(Bun.inspect("hi")).toBe('"hi"'); diff --git a/test/js/bun/yaml/yaml.test.ts b/test/js/bun/yaml/yaml.test.ts index 9273f512328a..0d83f7b2abab 100644 --- a/test/js/bun/yaml/yaml.test.ts +++ b/test/js/bun/yaml/yaml.test.ts @@ -2516,6 +2516,54 @@ config: }); describe("stringify", () => { + test.skipIf(!isASAN)( + "anchor name derived from a property name survives a GC between the anchor scan and emit", + async () => { + // The anchor name for a shared value is the first property name it was seen under, + // stored as a BunString that borrowed the scan iterator's PropertyNameArray entry + // without a ref. A Proxy whose ownKeys returns fresh strings is the one case where + // nothing else holds that StringImpl; once the inner iterator drops and a later + // getter GCs, the emit pass reads freed bytes. Malloc=1 routes WTF allocations + // through system malloc so ASAN sees the free. + await using proc = Bun.spawn({ + cmd: [ + bunExe(), + "-e", + ` + const shared = { x: 1 }; + function keys() { + const out = []; + for (let i = 0; i < 8; i++) out.push("freshkey_padding_to_avoid_small_string_" + i); + return out; + } + const inner = new Proxy({}, { + ownKeys: keys, + getOwnPropertyDescriptor() { return { enumerable: true, configurable: true, value: shared }; }, + has() { return true; }, + get() { return shared; }, + }); + const outer = { inner }; + Object.defineProperty(outer, "zz", { + enumerable: true, + get() { Bun.gc(true); Bun.gc(true); Bun.gc(true); return shared; }, + }); + const anchor = keys()[0]; + const out = Bun.YAML.stringify(outer); + if (!out.includes("&" + anchor)) throw new Error("anchor name corrupted"); + if (!out.includes("*" + anchor)) throw new Error("alias name corrupted"); + process.stdout.write("ok"); + `, + ], + env: { ...bunEnv, Malloc: "1" }, + stdout: "pipe", + stderr: "pipe", + }); + const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + expect({ stdout, exitCode }).toEqual({ stdout: "ok", exitCode: 0 }); + expect(stderr).not.toContain("AddressSanitizer"); + }, + ); + // Basic data type tests test("stringifies null", () => { expect(YAML.stringify(null)).toBe("null");