Fix stack overflow when inspecting JSX elements with circular references - #29750
Fix stack overflow when inspecting JSX elements with circular references#29750robobun wants to merge 1 commit into
Conversation
|
Updated 2:38 AM PT - Apr 26th, 2026
❌ @robobun, your commit 902d52a has 1 failures in
🧪 To try this PR locally: bunx bun-pr 29750That installs a local version of the PR into your bun-29750 --bun |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThe changes extend the console formatter's circular-reference detection to handle JSX values alongside other container-like types. Three test cases were added to validate circular-reference behavior in JSX elements, including self-referencing keys, circular props, and circular children. Changes
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
Found 1 issue this PR may fix:
🤖 Generated with Claude Code |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| 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, |
There was a problem hiding this comment.
🔴 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
keyviathis.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();toMatchSnapshotcallsjestSnapshotPrettyFormat(el)(expect.zig:819) →JestPrettyFormat.format(el).Tag.get(el)sees$$typeof === react.element→ returns.JSX(pretty_format.zig:416-423).format()checkscanHaveCircularReferences(.JSX)→false→ skips visited-map insert.printAs(.JSX, ...)iterates props, hitsfoo, callsthis.format(el)at line 1580.- 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.
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
|
Duplicate of #29709 which covers this plus the parallel |
1 participant
What does this PR do?
Bun.inspect()/console.log()would crash with a stack overflow when formatting a React element that contained a circular reference to itself viakey,props, orchildren.The JSX formatter recurses into
key,props, andchildren, but.JSXwas not included incanHaveCircularReferences(), so the visited-map tracking and stack-overflow guard were never applied. This PR adds.JSXto that set so circular JSX elements print[Circular]instead of recursing forever.How did you verify your code works?
Added regression tests for circular references via
key,props, andchildren. All existinginspect.test.jstests continue to pass.Found by Fuzzilli (fingerprint
bb8715595a137556).