Fix segfault in Bun.inspect for circular JSX elements#30382
Conversation
When a React element's key, props, or children contained a reference back to the element itself, Bun.inspect would recurse infinitely and crash with a stack overflow. The JSX formatter tag was missing from canHaveCircularReferences(), so the visited-map and stack checks that protect Object/Array/Map/Set formatting never ran for JSX.
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughThis PR extends circular reference detection to Event and JSX types across the Console and Jest formatting systems. Changes to ChangesCircular Reference Detection for Event and JSX
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
|
Duplicate of #29709, which has the same fix plus the |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — minimal crash fix that just adds .JSX to the existing circular-reference guard, with a regression test.
Extended reasoning...
Overview
This PR fixes a SIGSEGV (native stack overflow) in Bun.inspect() / console.log() when formatting a self-referential React element. The change is two one-line additions: .JSX is appended to the canHaveCircularReferences() switch arm in both src/jsc/ConsoleObject.zig:1139 and src/test_runner/pretty_format.zig:329. A new it("jsx with circular reference") block in test/js/bun/util/inspect.test.js covers circular key, circular arbitrary prop, and circular children.
Security risks
None. This is purely an output-formatting change that gates JSX values behind the same visited-map / StackCheck guard already applied to .Object, .Array, .Map, .Set, .Event, etc. No new code paths, no auth/crypto/permissions involvement, and the worst-case behavioral change is printing [Circular] instead of crashing.
Level of scrutiny
Low. The fix is mechanical — extending an existing, well-exercised protection to one more enum tag — and was found by a fuzzer (471aa8ea6d631cd1). The .JSX branch of printAs clearly recurses via this.format(...) on key, props, and children, so it obviously belongs in the circular-reference set. The visited-map insert/remove is symmetric (getOrPut on entry, map.remove in defer), so adding a tag here cannot leak entries or change behavior for non-circular JSX.
Other factors
No CODEOWNERS cover the touched paths. The one inline comment is a pre-existing nit about .Event parity in pretty_format.zig and is explicitly deferrable. The added regression test asserts exact output for all three circular shapes and the full inspect.test.js suite passes per the PR description.
| 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.
🟣 Pre-existing nit: while you're touching this line, consider also adding .Event for parity with ConsoleObject.zig:1139. The .Event branch in this file's printAs recursively calls this.format() on the event's data/error sub-values, so a self-referential MessageEvent/ErrorEvent (e.g. via Object.defineProperty(ev, 'data', {value: ev})) would still recurse unboundedly in the Jest snapshot path. Not introduced by this PR — feel free to defer.
Extended reasoning...
What the gap is
This PR adds .JSX to canHaveCircularReferences() in src/test_runner/pretty_format.zig:329, mirroring the same fix applied to src/jsc/ConsoleObject.zig:1139. However, the ConsoleObject.zig version of this function already lists .Event (alongside .Function, .Error, .Class), while the pretty_format.zig version still does not. After this PR the two switch arms read:
- ConsoleObject.zig:
.Function, .Array, .Object, .Map, .Set, .Error, .Class, .Event, .JSX => true - pretty_format.zig:
tag == .Array or tag == .Object or tag == .Map or tag == .Set or tag == .JSX
Of the tags that differ, .Event is the only one that actually recurses in pretty_format.zig's printAs implementation — .Function, .Class, and .Error just print a one-line label here, so they don't need the visited-map guard. .Event does.
Code path
In pretty_format.zig, Tag.get() maps JSType.Event to Tag.Event via the final switch (.Event => .Event). MessageEvent and ErrorEvent instances carry JSType.Event (0b11101111), distinct from .DOMWrapper (0b11101110), so they reach the .Event arm of printAs. That arm then does:
.MessageEvent => {
const data: JSValue = (try value.fastGet(this.globalThis, .data)) orelse .js_undefined;
const tag = try Tag.get(data, this.globalThis);
try this.format(tag, Writer, writer_, data, this.globalThis, enable_ansi_colors);
}
.ErrorEvent => {
if (try value.fastGet(this.globalThis, .@"error")) |data| {
const tag = try Tag.get(data, this.globalThis);
try this.format(tag, Writer, writer_, data, this.globalThis, enable_ansi_colors);
}
}Because .Event is absent from canHaveCircularReferences(), the visited-map check at the top of printAs is compiled out for this branch, and there is no StackCheck guard either.
Why nothing else catches it
The only circular-reference protection in JestPrettyFormat.Formatter.printAs is gated on comptime Format.canHaveCircularReferences(). With .Event excluded, the getOrPut/found_existing check never runs for the outer Event frame, so when data resolves back to the same Event the recursion is unbounded — exactly the same failure mode this PR fixes for .JSX.
Step-by-step repro
const ev = new MessageEvent('message');Object.defineProperty(ev, 'data', { value: ev });— the existinginspect.test.jstest "MessageEvent with deleted data" already demonstrates that overridingdataviadefinePropertyis observed by the formatter.expect(ev).toMatchSnapshot();(or anything that goes throughJestPrettyFormat).Tag.get(ev)→.Event.printAs(.Event, …)runs without insertingevinto the visited map.- It reads
data, getsevback, callsthis.format(Tag.get(ev) = .Event, …, ev, …), which re-entersprintAs(.Event, …)with the same value. - Loop until native stack overflow → SIGSEGV.
Impact
Same impact class as the JSX bug being fixed: a hard crash (stack overflow) of the test runner when snapshotting a self-referential Event. The trigger is admittedly contrived — data/error are read-only getters by default, so the user has to deliberately shadow them with defineProperty — which is why this is a nit rather than a blocking issue.
Fix
One-character-class change on the line already being edited:
return tag == .Array or tag == .Object or tag == .Map or tag == .Set or tag == .JSX or tag == .Event;This brings pretty_format.zig in line with ConsoleObject.zig for the one remaining tag that actually recurses here. This is pre-existing and not introduced by the PR, so it's entirely reasonable to defer to a follow-up.
1 participant
What does this PR do?
Fixes a segfault in
Bun.inspect()/console.log()when formatting a React element that contains a circular reference to itself viakey,props.children, or any other prop.The
.JSXformatter tag was missing fromcanHaveCircularReferences(), so the visited-map check and theStackCheckguard that protectObject,Array,Map,Set, etc. never ran for JSX, and formatting recursed until the native stack was exhausted.How did you verify your code works?
Added a regression test in
test/js/bun/util/inspect.test.jscovering circularkey, circular arbitrary prop, and circularchildren. The new test crashes the process on current canary and passes with this change. Fullinspect.test.jssuite (73 tests) passes.Fuzzer fingerprint:
471aa8ea6d631cd1