Skip to content
Open
Show file tree
Hide file tree
Changes from all 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);
},
);
});
62 changes: 62 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,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);
Comment thread
robobun marked this conversation as resolved.
});

// 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"');
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