Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
24 changes: 13 additions & 11 deletions src/jsc/ConsoleObject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1954,6 +1954,7 @@ pub mod formatter {
| Tag::Error
| Tag::Class
| Tag::Event
| Tag::JSX
)
}
}
Expand Down Expand Up @@ -3289,17 +3290,16 @@ 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;
if self.can_throw_stack_overflow {
return Err(self.global_this.throw_stack_overflow());
}
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())
Expand Down Expand Up @@ -3554,18 +3554,20 @@ pub mod formatter {
writer_: &mut dyn bun_io::Write,
value: JSValue,
) -> JsResult<()> {
let target = value.get_proxy_internal_field(jsc::ProxyField::Target);
let mut target = value.get_proxy_internal_field(jsc::ProxyField::Target);
// Proxy does not allow non-objects here.
debug_assert!(target.is_cell());
// TODO: if (options.showProxy), print like
// `Proxy { target: ..., handlers: ... }` — this is default off so
// it is not used.
self.format::<C>(
Tag::get(target, self.global_this)?,
writer_,
target,
self.global_this,
)
loop {
let tag = Tag::get(target, self.global_this)?;
if !matches!(tag.tag.tag(), Tag::Proxy) {
return self.format::<C>(tag, writer_, target, self.global_this);
}
target = target.get_proxy_internal_field(jsc::ProxyField::Target);
debug_assert!(target.is_cell());
}
}

#[inline(never)]
Expand Down
6 changes: 5 additions & 1 deletion src/runtime/test_runner/pretty_format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,10 @@ impl Tag {

#[inline]
pub(crate) 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 | Tag::Event
)
Comment thread
claude[bot] marked this conversation as resolved.
}
}

Expand Down Expand Up @@ -1870,6 +1873,7 @@ impl<'a> Formatter<'a> {
{
evt @ (EventType::MessageEvent | EventType::ErrorEvent) => evt,
_ => {
let _ = self.map.remove(&value);
return self.print_as::<W, { Tag::Object }, ENABLE_ANSI_COLORS>(
writer.ctx, value, JSType::Event,
);
Expand Down
167 changes: 166 additions & 1 deletion test/js/bun/util/inspect.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { describe, expect, it } from "bun:test";
import { bunEnv, bunExe, isASAN, isWindows, normalizeBunSnapshot, tmpdirSync } from "harness";
import { bunEnv, bunExe, isASAN, isWindows, normalizeBunSnapshot, tempDir, tmpdirSync } from "harness";
import { join } from "path";
import util from "util";
it("prototype", () => {
Expand Down Expand Up @@ -316,6 +316,171 @@
expect(input).toBe(output);
});

it("jsx with circular references does not crash", () => {
// Run in a subprocess: without the fix this overflows the native stack and segfaults,
// which would otherwise kill the test runner before it can record a failure.
Comment thread
claude[bot] marked this conversation as resolved.
Outdated
const { exitCode, stdout } = Bun.spawnSync({
cmd: [
bunExe(),
"-e",
`
const a = { $$typeof: Symbol.for("react.element"), type: "div", props: null, key: null };
a.props = a;
console.log(Bun.inspect(a));

const b = { $$typeof: Symbol.for("react.element"), type: "div", key: null };
b.props = { children: b };
console.log(Bun.inspect(b));

const c = { $$typeof: Symbol.for("react.element"), type: "span", key: null };
c.props = { children: [c, c] };
console.log(Bun.inspect(c));

const d = { $$typeof: Symbol.for("react.element"), type: "div", props: {} };
d.key = d;
console.log(Bun.inspect(d));
`,
],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});

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

View check run for this annotation

Claude / Claude Code Review

New subprocess tests use spawnSync serially instead of async spawn / test.concurrent

Nit: the six new subprocess tests use `Bun.spawnSync` and run serially, diverging from REVIEW.md's "async spawns over `spawnSync`" / "`test.concurrent` for independent subprocess suites" and from this file's own convention — the two pre-existing subprocess tests here ("huge sparse array", "object mutated while being formatted") both use `await using proc = Bun.spawn(...)` + `Promise.all([stdout.text(), stderr.text(), exited])`. Six independent full-`bun` spawns (two of which build 100k / 20k obj
Comment thread
claude[bot] marked this conversation as resolved.
Outdated
const out = stdout.toString();
expect(out).toContain("[Circular]");
expect(out).toContain("<div>\n [Circular]\n</div>");
expect(out).toContain("<span>\n [Circular]\n [Circular]\n</span>");
expect(out).toContain("<div key=[Circular] />");
expect(exitCode).toBe(0);
});

it("jsx with non-object props does not crash", () => {
const { exitCode, stdout } = Bun.spawnSync({
cmd: [
bunExe(),
"-e",
`
const el = { $$typeof: Symbol.for("react.element"), type: "div", props: 42, key: null };
console.log(Bun.inspect(el));
`,
],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
expect(stdout.toString().trim()).toBe("<div />");
expect(exitCode).toBe(0);
});

it("jsx with circular props in test diff formatter", () => {
using dir = tempDir("jsx-circular-diff", {
"diff.test.js": `
import { test, expect } from "bun:test";
test("circular", () => {
const el = { $$typeof: Symbol.for("react.element"), type: "div", props: null, key: null };
el.props = el;
expect(() => expect(el).toEqual({})).toThrow();
});
test("non-object props", () => {
const el = { $$typeof: Symbol.for("react.element"), type: "div", props: 42, key: null };
expect(() => expect(el).toEqual({})).toThrow();
});
`,
});
const { exitCode, stderr } = Bun.spawnSync({
cmd: [bunExe(), "test", "diff.test.js"],
cwd: String(dir),
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
expect(stderr.toString()).toContain("2 pass");
expect(exitCode).toBe(0);
});

it("Event in test diff formatter is not spuriously [Circular]", () => {
using dir = tempDir("event-diff", {
"diff.test.js": `
import { test, expect } from "bun:test";
test("close event", () => {
expect(() => expect(new CloseEvent("close", { code: 1000 })).toEqual({})).toThrow(/CloseEvent/);
});
test("custom event", () => {
expect(() => expect(new CustomEvent("foo")).toEqual({})).toThrow(/CustomEvent/);
});
test("circular message event still detected", () => {
const ev = new MessageEvent("message");
Object.defineProperty(ev, "data", { value: ev, configurable: true });
expect(() => expect(ev).toEqual({})).toThrow(/\\[Circular\\]/);
});
`,
});
const { exitCode, stderr } = Bun.spawnSync({
cmd: [bunExe(), "test", "diff.test.js"],
cwd: String(dir),
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
expect(stderr.toString()).toContain("3 pass");
expect(exitCode).toBe(0);
});

it("deeply nested Proxy chain does not crash", () => {
// Without the fix, print_proxy recurses on the target without a stack-safety
// check and segfaults; run in a subprocess so a regression fails the suite
// instead of killing the runner.
const { exitCode, stdout, signalCode } = Bun.spawnSync({
cmd: [
bunExe(),
"-e",
`
let p = { ok: 1 };
for (let i = 0; i < 100000; i++) p = new Proxy(p, {});
console.log(Bun.inspect(p));
console.log(p);
`,
],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
expect(normalizeBunSnapshot(stdout.toString())).toMatchInlineSnapshot(`
"{
ok: 1,
}
{
ok: 1,
}"
`);
expect(signalCode).toBeFalsy();
expect(exitCode).toBe(0);
});

it("deeply nested non-cyclic jsx does not segfault", () => {
// Stack size and release-build frame size vary by platform, so a fixed depth
// may or may not overflow: accept either a clean RangeError or successful
// completion. The regression was SIGSEGV.
const { exitCode, stdout, signalCode } = Bun.spawnSync({
cmd: [
bunExe(),
"-e",
`
let el = { $$typeof: Symbol.for("react.element"), type: "div", props: {}, key: null };
for (let i = 0; i < 20000; i++)
el = { $$typeof: Symbol.for("react.element"), type: "div", props: { children: el }, key: null };
try { Bun.inspect(el); console.log("ok"); }
catch (e) { console.log(e.constructor.name); }
`,
],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
expect(["RangeError", "ok"]).toContain(stdout.toString().trim());
expect(signalCode).toBeFalsy();
expect(exitCode).toBe(0);
});

it("inspect", () => {
expect(Bun.inspect(new TypeError("what")).includes("TypeError: what")).toBe(true);
expect(Bun.inspect("hi")).toBe('"hi"');
Expand Down
Loading