Fix stack overflow when inspecting JSX elements with circular references#29170
Fix stack overflow when inspecting JSX elements with circular references#29170robobun wants to merge 1 commit into
Conversation
Bun.inspect() on a React element whose props, key, or children point back to the element itself would recurse until the stack overflowed because the .JSX tag was not included in canHaveCircularReferences(). Add it so the existing cycle detection and stack guard apply. Also guard against props being a non-object value instead of unwrapping getObject() with .?, which panicked on primitives.
|
Updated 10:31 AM PT - Apr 11th, 2026
❌ @robobun, your commit 96c3fb1 has 1 failures in
🧪 To try this PR locally: bunx bun-pr 29170That installs a local version of the PR into your bun-29170 --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 (3)
WalkthroughThis pull request enhances JSX value handling in the console formatting system by enabling circular-reference detection for JSX values and improving props extraction to safely handle non-object props. Regression tests validate the new behavior for circular references and non-object props scenarios. Changes
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| } | ||
|
|
||
| pub inline fn canHaveCircularReferences(tag: Tag) bool { | ||
| return tag == .Array or tag == .Object or tag == .Map or tag == .Set; | ||
| return tag == .Array or tag == .Object or tag == .Map or tag == .Set or tag == .JSX; | ||
| } |
There was a problem hiding this comment.
🔴 The fix in pretty_format.zig is incomplete compared to ConsoleObject.zig in two ways: (1) canHaveCircularReferences() adds .JSX but still omits .Event, leaving circular MessageEvent/ErrorEvent (where data/error points back to the event) able to trigger infinite recursion via any Jest matcher; (2) pretty_format.zig has no stack-depth guard (no stack_check field, no isSafeToRecurse() call) unlike ConsoleObject.zig which guards all recursive types against deep-but-non-circular trees. Both gaps should be closed before merging.
Extended reasoning...
Missing .Event in canHaveCircularReferences (pretty_format.zig)
After this PR, pretty_format.zig canHaveCircularReferences() returns true for .Array, .Object, .Map, .Set, and .JSX — but not .Event. ConsoleObject.zig correctly includes .Event in the same list. The .Event branch in printAs() recurses for both MessageEvent (formats the .data property) and ErrorEvent (formats the .error property). If data/error holds a back-reference to the event itself, Tag.get() returns .Event, and printAs(.Event) is called again with no visited-map check, producing unbounded recursion and a stack overflow. This is directly triggerable from any Jest matcher:
const e = new MessageEvent("message");
Object.defineProperty(e, "data", { value: e });
expect(e).toMatchSnapshot(); // infinite recursion -> crash
Execution path: printAs(.Event, e) -> fastGet(.data) returns e -> Tag.get(e) -> .Event -> format(.Event, e) -> printAs(.Event, e) -> ... with no canHaveCircularReferences guard to break the cycle.
No stack-depth guard in pretty_format.zig
ConsoleObject.zig carries two additional fields on its Formatter struct: stack_check: bun.StackCheck and can_throw_stack_overflow: bool. Inside printAs(), before processing any type in canHaveCircularReferences(), it calls isSafeToRecurse() and sets this.failed = true if the stack is too deep.
pretty_format.zig has no such guard at all — grep returns zero hits for stack_check, isSafeToRecurse, or can_throw_stack_overflow in that file. The visited-map only prevents cycles (same object identity); it does nothing for deeply-nested but non-circular trees. A JSX tree thousands of levels deep, or a deeply-nested Array/Object/Map/Set, will recurse unboundedly and overflow the Zig stack when processed through any Jest matcher (toMatchSnapshot, toEqual, etc.).
Why existing code does not prevent this
The visited-map is only consulted when canHaveCircularReferences() returns true for the compile-time Format tag. Because .Event returns false, printAs(.Event) never inserts the value into the map and never checks for an existing entry. The stack-depth guard is simply absent — it was never ported to pretty_format.zig from ConsoleObject.zig.
Impact
Any Jest test that calls expect() on (a) a MessageEvent or ErrorEvent with a circular data/error property, or (b) a deeply-nested React element tree or Object/Array/Map/Set, can crash the test process with a Zig stack overflow. Since this formatter is used by all Jest matchers, the crash surface is wide.
How to fix
- Add .Event to canHaveCircularReferences() in pretty_format.zig alongside .JSX.
- Add a stack_check: bun.StackCheck field to the Formatter struct in pretty_format.zig and call isSafeToRecurse() at the top of the canHaveCircularReferences() guard block in printAs(), mirroring the pattern already present in ConsoleObject.zig.
There was a problem hiding this comment.
Both of these are pre-existing gaps on main that predate this PR — neither .Event nor a stack_check guard exist in pretty_format.zig today. This PR targets the specific fuzzer crash (2b73926abbed374a) in Bun.inspect() on circular JSX, and mirrors the same minimal JSX fix to pretty_format.zig since the identical code pattern lives there.
Widening the scope to also port .Event cycle detection and the full StackCheck mechanism into the Jest formatter is a separate change (the stack guard in particular needs new struct fields + init plumbing). Keeping this one focused on the JSX crash; happy to track those as a follow-up.
|
Superseded by #29709 which has the same fix plus additional test coverage for the diff formatter path. |
1 participant
Fuzzer found a segfault when calling
Bun.inspect()on a React element whoseprops(orkey, orprops.children) contains a reference back to the element itself:The
.JSXformatter tag was missing fromcanHaveCircularReferences(), so neither the visited-map cycle detection nor the stack guard was applied when formatting JSX elements. The formatter would recurse intoprops→ format the same JSX element → recurse intoprops→ … until the stack blew.While in the area, also fixed a related panic:
props.getObject().?assumedpropsis always an object, but a user-constructed element can haveprops: 42and would hitattempt to use null value. Now it just skips props iteration and renders<div />.Same fixes applied to both
ConsoleObject.zig(Bun.inspect/console.log) andtest/pretty_format.zig(test matcher formatting).Fuzzer fingerprint:
2b73926abbed374a