Skip to content

Fix stack overflow when inspecting JSX elements with circular references #29750

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
robobun wants to merge 1 commit into from
Closed

Fix stack overflow when inspecting JSX elements with circular references #29750

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion src/bun.js/ConsoleObject.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1136,7 +1136,7 @@

pub fn canHaveCircularReferences(tag: Tag) bool {
return switch (tag) {
.Function, .Array, .Object, .Map, .Set, .Error, .Class, .Event => true,
.Function, .Array, .Object, .Map, .Set, .Error, .Class, .Event, .JSX => true,

Check failure on line 1139 in src/bun.js/ConsoleObject.zig

View check run for this annotation

Claude / Claude Code Review

Same JSX circular-reference stack overflow remains in JestPrettyFormat 

The same crash still exists in the parallel jest pretty-formatter: `src/bun.js/test/pretty_format.zig:328-330` has its own `canHaveCircularReferences()` that still omits `.JSX`, and its `.JSX` branch recurses into `key`/`props`/`children` via `this.format()` with no stack-check fallback. After this PR, `expect(circularJsxEl).toMatchSnapshot()` or a failing `toEqual` diff (via `diff_format.zig`) will still segfault — the fuzzer crash is only half-fixed. The same one-line change should be mirrored

@claude claude Bot Apr 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 The same crash still exists in the parallel jest pretty-formatter: src/bun.js/test/pretty_format.zig:328-330 has its own canHaveCircularReferences() that still omits .JSX, and its .JSX branch recurses into key/props/children via this.format() with no stack-check fallback. After this PR, expect(circularJsxEl).toMatchSnapshot() or a failing toEqual diff (via diff_format.zig) will still segfault — the fuzzer crash is only half-fixed. The same one-line change should be mirrored at pretty_format.zig:329.

Extended reasoning...

What the bug is

This PR correctly adds .JSX to canHaveCircularReferences() in src/bun.js/ConsoleObject.zig:1139 so that Bun.inspect() / console.log() no longer stack-overflow on circular JSX elements. However, Bun has a second, parallel formatter in src/bun.js/test/pretty_format.zig (JestPrettyFormat) with its own copy of canHaveCircularReferences() at lines 328-330:

pub fn canHaveCircularReferences(tag: Tag) bool {
    return switch (tag) {
        .Array, .Object, .Map, .Set => true,
        else => false,
    };
}

This copy still does not include .JSX, and unlike ConsoleObject.zig it has no stack_check.isSafeToRecurse() fallback either.

The code path that triggers it

JestPrettyFormat's .JSX branch in printAs() (line 1477+) is structurally identical to the one this PR fixes:

  • line 1531: recurses into key via this.format(...)
  • line 1580: recurses into each prop value via this.format(...)
  • line 1643: recurses into JSX children via this.format(...)

In format() (line ~876), the visited-map circular-reference tracking is gated on comptime canHaveCircularReferences(tag). Since .JSX returns false, no visited-set entry is recorded and the recursion is unbounded.

This formatter is reachable from user code via:

  • expect(...).toMatchSnapshot() → expect.zig:819 → jestSnapshotPrettyFormat → JestPrettyFormat.format
  • any failing expect(...).toEqual(...) / .toBe(...) diff → diff_format.zig:39,48 → JestPrettyFormat.format

Why existing code doesn't prevent it

Tag.get() in pretty_format.zig (lines 416-423) detects 25618typeof === Symbol.for('react.element') and returns .JSX exactly like ConsoleObject.zig does, so the value is routed into the unprotected .JSX branch rather than the protected .Object branch. Grep confirms there is no isSafeToRecurse / StackCheck guard anywhere in pretty_format.zig, so there is no secondary safety net.

Step-by-step proof

const el = { $$typeof: Symbol.for('react.element'), type: 'div', key: null, ref: null, props: {} };
el.props.foo = el;
expect(el).toMatchSnapshot();
  1. toMatchSnapshot calls jestSnapshotPrettyFormat(el) (expect.zig:819) → JestPrettyFormat.format(el).
  2. Tag.get(el) sees $$typeof === react.element → returns .JSX (pretty_format.zig:416-423).
  3. format() checks canHaveCircularReferences(.JSX) → false → skips visited-map insert.
  4. printAs(.JSX, ...) iterates props, hits foo, calls this.format(el) at line 1580.
  5. Goto step 2. Stack grows until segfault.

The same applies with el.key = el (line 1531) or el.props.children = [el] (line 1643).

Impact

The fuzzer crash (fingerprint bb8715595a137556) that this PR targets is only half-fixed: Bun.inspect() is safe, but the test-runner pretty-formatter for the same value still crashes the process. Any user who snapshot-tests or diff-compares a React tree containing a back-reference (e.g., a parent ref stashed on a child) will hit a hard segfault in bun test.

Fix

Mirror the change at src/bun.js/test/pretty_format.zig:329 — add .JSX to the switch (and optionally .Error/.Event/.Function/.Class for parity with ConsoleObject.zig). It's the same one-token diff.

else => false,
};
}
Expand Down
18 changes: 18 additions & 0 deletions test/js/bun/util/inspect.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,24 @@ it("jsx with fragment", () => {
expect(input).toBe(output);
});

it("jsx with circular reference in key", () => {
const el = { $$typeof: Symbol.for("react.element"), type: "div", key: null, ref: null, props: {} };
el.key = el;
expect(Bun.inspect(el)).toContain("[Circular]");
});

it("jsx with circular reference in props", () => {
const el = { $$typeof: Symbol.for("react.element"), type: "div", key: null, ref: null, props: {} };
el.props.foo = el;
expect(Bun.inspect(el)).toBe("<div foo=[Circular] />");
});

it("jsx with circular reference in children", () => {
const el = { $$typeof: Symbol.for("react.element"), type: "div", key: null, ref: null, props: {} };
el.props.children = [el];
expect(Bun.inspect(el)).toBe("<div>\n [Circular]\n</div>");
});

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