Skip to content
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 6 additions & 7 deletions src/jsc/ConsoleObject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -2010,6 +2007,7 @@ pub mod formatter {
| Tag::Error
| Tag::Class
| Tag::Event
| Tag::JSX
)
}
}
Expand Down Expand Up @@ -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;
Expand All @@ -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");
Expand Down
3 changes: 1 addition & 2 deletions src/jsc/VirtualMachine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 8 additions & 1 deletion src/jsc/ipc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
8 changes: 5 additions & 3 deletions src/runtime/api/YAMLObject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
},
Expand All @@ -146,7 +148,7 @@ pub(crate) enum AnchorAliasName {
Root,
ArrayItem(usize),
PropValue {
prop_name: BunString,
prop_name: OwnedString,
// added after the name
counter: usize,
},
Expand Down Expand Up @@ -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);
}
Expand Down
11 changes: 9 additions & 2 deletions src/runtime/test_runner/pretty_format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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> {
Expand All @@ -357,6 +358,7 @@ impl<'a> Formatter<'a> {
failed: false,
estimated_line_length: 0,
always_newline_scope: false,
stack_check: StackCheck::init(),
}
}

Expand Down Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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
Expand Down
34 changes: 34 additions & 0 deletions test/js/bun/spawn/spawn.ipc.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
38 changes: 37 additions & 1 deletion test/js/bun/test/pretty-format-overflow.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand All @@ -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);
},
);
});
67 changes: 67 additions & 0 deletions test/js/bun/util/inspect.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,73 @@
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) => {
await using proc = Bun.spawn({
cmd: [bunExe(), "-e", `${setup} try { console.log(x); } catch (e) { process.stdout.write("CAUGHT:" + e.name); }`],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stdout).toEndWith("CAUGHT:RangeError");
expect(exitCode).toBe(0);

Check warning on line 357 in test/js/bun/util/inspect.test.js

View check run for this annotation

Claude / Claude Code Review

Deep-JSX console.log test may emit O(N²) bytes to stdout

The "deep JSX tree" case here can push O(N²) bytes to the pipe before the RangeError fires: `print_jsx` writes `">\n"` + `write_indent_n(self.indent)` (2×indent spaces) at every level *before* recursing and never increments `self.depth`, so nothing bounds it except the stack check — plausibly thousands of levels in a release build ⇒ tens of MB flushed to stdout and then materialised as one JS string in the parent via `proc.stdout.text()` before `toEndWith` runs. The Proxy variant sharing this `i
Comment thread
robobun marked this conversation as resolved.
});

it("throwing a deeply nested array as an uncaught exception does not crash the printer", async () => {
await using proc = Bun.spawn({
cmd: [bunExe(), "-e", `let a = []; for (let i = 0; i < 60000; i++) a = [a]; throw a;`],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
// Uncaught throw exits 1 (not a segfault's 139/11) and the error printer ran.
expect(stderr).toContain("error");
expect(exitCode).toBe(1);
});

it("rejecting a deeply nested array as an unhandled rejection does not crash the printer", async () => {
await using proc = Bun.spawn({
cmd: [bunExe(), "-e", `let a = []; for (let i = 0; i < 60000; i++) a = [a]; Promise.reject(a);`],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).toContain("error");
expect(exitCode).toBe(1);
});
Comment thread
claude[bot] marked this conversation as resolved.
Outdated
});

it("inspect", () => {
expect(Bun.inspect(new TypeError("what")).includes("TypeError: what")).toBe(true);
expect(Bun.inspect("hi")).toBe('"hi"');
Expand Down
48 changes: 48 additions & 0 deletions test/js/bun/yaml/yaml.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down
Loading