Fix crash when inspecting circular or malformed JSX elements - #30383
Fix crash when inspecting circular or malformed JSX elements#30383robobun wants to merge 1 commit into
Conversation
Bun.inspect() on a React element (object with $$typeof set to
Symbol.for('react.element')) would segfault in two cases:
- If the element contained a circular reference through key or
props.children, the formatter recursed without cycle detection
until stack overflow. The .JSX tag was missing from
canHaveCircularReferences().
- If props was not an object, getObject().? unwrapped null and
panicked.
Apply the same fixes to the jest pretty-format path.
|
Updated 8:57 PM PT - May 7th, 2026
❌ @robobun, your commit d5d510d has 1 failures in
🧪 To try this PR locally: bunx bun-pr 30383That installs a local version of the PR into your bun-30383 --bun |
WalkthroughThis PR extends JSX handling in Bun's console formatting system. It adds JSX values to circular-reference detection eligibility and refactors JSX props retrieval to safely convert props to objects via ChangesJSX Circular Reference and Props Handling
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/test_runner/pretty_format.zig`:
- Around line 1537-1538: The JSX formatter currently only recognizes
"react.element" and "react.fragment" and therefore skips React 19 transitional
elements; update the tag-name check that matches "react.element" and
"react.fragment" to also accept "react.transitional.element" (the same place
that branches into the hardened JSX path) so that when you extract props (the
block using value.get(this.globalThis, "props") -> props_obj ->
props_obj.toJS()) transitional elements follow the JSX formatting path just like
the other React element types.
In `@test/js/bun/util/inspect.test.js`:
- Around line 777-810: Replace the three separate it(...) cases ("handles
circular key", "handles circular children", "handles non-object props") with a
single parameterized it.each(...) table that supplies a description, a function
or factory to build the elem fixture (so you can create circular references
inside the factory for the first two cases), and the expected assertion
type/value; call Bun.inspect on the produced elem in the test body and branch
the assertion: expect(...).toContain("[Circular]") for the first two fixtures
and expect(...).toBe("<div />") for the non-object props fixture. Ensure you
reference the same symbols used now (elem, props, $$typeof, Bun.inspect) so the
factory builds identical shapes and the descriptions match the original test
names.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: fbe36a92-f047-4378-9bf5-5b64b0cf9af1
📒 Files selected for processing (3)
src/jsc/ConsoleObject.zigsrc/test_runner/pretty_format.zigtest/js/bun/util/inspect.test.js
| if (if (try value.get(this.globalThis, "props")) |props| props.getObject() else null) |props_obj| { | ||
| const props = props_obj.toJS(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Compare React element symbol coverage between the two formatter implementations.
rg -n -C2 'react\.(element|transitional\.element|fragment)' \
src/test_runner/pretty_format.zig \
src/jsc/ConsoleObject.zigRepository: oven-sh/bun
Length of output: 1629
React 19 transitional elements still bypass the JSX formatter.
This file detects only "react.element" and "react.fragment" (lines 419-422) but not "react.transitional.element", unlike src/jsc/ConsoleObject.zig (line 1307). React 19 elements will skip the hardened JSX path added in this PR and fall back to generic object formatting.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/test_runner/pretty_format.zig` around lines 1537 - 1538, The JSX
formatter currently only recognizes "react.element" and "react.fragment" and
therefore skips React 19 transitional elements; update the tag-name check that
matches "react.element" and "react.fragment" to also accept
"react.transitional.element" (the same place that branches into the hardened JSX
path) so that when you extract props (the block using value.get(this.globalThis,
"props") -> props_obj -> props_obj.toJS()) transitional elements follow the JSX
formatting path just like the other React element types.
| it("handles circular key", () => { | ||
| const elem = { | ||
| $$typeof: Symbol.for("react.element"), | ||
| type: "div", | ||
| key: null, | ||
| ref: null, | ||
| props: { children: [] }, | ||
| }; | ||
| elem.key = elem; | ||
| expect(Bun.inspect(elem)).toContain("[Circular]"); | ||
| }); | ||
|
|
||
| it("handles circular children", () => { | ||
| const elem = { | ||
| $$typeof: Symbol.for("react.element"), | ||
| type: "div", | ||
| key: null, | ||
| ref: null, | ||
| props: {}, | ||
| }; | ||
| elem.props.children = elem; | ||
| expect(Bun.inspect(elem)).toContain("[Circular]"); | ||
| }); | ||
|
|
||
| it("handles non-object props", () => { | ||
| const elem = { | ||
| $$typeof: Symbol.for("react.element"), | ||
| type: "div", | ||
| key: null, | ||
| ref: null, | ||
| props: 123, | ||
| }; | ||
| expect(Bun.inspect(elem)).toBe("<div />"); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
cat -n test/js/bun/util/inspect.test.js | sed -n '770,820p'Repository: oven-sh/bun
Length of output: 1236
🏁 Script executed:
rg "describe\.each" test/js/bun/util/inspect.test.js -A 5 -B 2Repository: oven-sh/bun
Length of output: 37
🏁 Script executed:
rg "describe\.each" test/js/bun/ --type js --type ts -l | head -20Repository: oven-sh/bun
Length of output: 627
🏁 Script executed:
cat test/js/bun/test/jest-each.test.ts | head -100Repository: oven-sh/bun
Length of output: 1609
🏁 Script executed:
rg "describe\.each|it\.each" test/js/bun/test/jest-each.test.ts -A 8 -B 1 | head -80Repository: oven-sh/bun
Length of output: 1455
🏁 Script executed:
rg "it\.each" test/js/bun/test/jest-each.test.ts -A 5 -B 1 | head -40Repository: oven-sh/bun
Length of output: 1126
🏁 Script executed:
# Check if there are examples of describe.each with complex objects in the test suite
rg "describe\.each.*\(" test/js/bun/ -A 10 -B 1 --type ts --type js | head -60Repository: oven-sh/bun
Length of output: 4859
🏁 Script executed:
# Let's verify the exact syntax expected by checking how describe.each callback should work
cat test/js/bun/udp/udp_socket.test.ts | sed -n '1,50p'Repository: oven-sh/bun
Length of output: 2056
🏁 Script executed:
# Check the jest-each test more carefully to understand the callback structure
rg "describe\.each.*\{" test/js/bun/typescript/type-export.test.ts -A 8 -B 2 | head -50Repository: oven-sh/bun
Length of output: 1323
🏁 Script executed:
# Let's confirm the exact callback signature expected for describe.each
rg "describe\.each.*\)" test/js/bun/udp/udp_socket.test.ts -A 5 -B 1Repository: oven-sh/bun
Length of output: 358
🏁 Script executed:
# Check if there's any example that directly executes logic in describe.each callback
rg "describe\.each" test/js/bun/ --type ts --type js -A 15 | grep -A 15 "describe\.each.*hostname" | head -25Repository: oven-sh/bun
Length of output: 1780
🏁 Script executed:
# Let me verify the exact structure needed - check if describe.each can work with verify functions or if we need it.each
cat test/js/bun/dns/resolve-dns.test.ts | grep -A 20 "describe.each(validHostnames)"Repository: oven-sh/bun
Length of output: 546
Use it.each() for these parameterized test cases.
Lines 777–810 contain repeated test structure with only fixture/assertion differences. Using it.each() will consolidate these tests and align with repo conventions per the coding guidelines.
♻️ Suggested refactor
describe("JSX element", () => {
- it("handles circular key", () => {
- const elem = {
- $$typeof: Symbol.for("react.element"),
- type: "div",
- key: null,
- ref: null,
- props: { children: [] },
- };
- elem.key = elem;
- expect(Bun.inspect(elem)).toContain("[Circular]");
- });
-
- it("handles circular children", () => {
- const elem = {
- $$typeof: Symbol.for("react.element"),
- type: "div",
- key: null,
- ref: null,
- props: {},
- };
- elem.props.children = elem;
- expect(Bun.inspect(elem)).toContain("[Circular]");
- });
-
- it("handles non-object props", () => {
- const elem = {
- $$typeof: Symbol.for("react.element"),
- type: "div",
- key: null,
- ref: null,
- props: 123,
- };
- expect(Bun.inspect(elem)).toBe("<div />");
- });
+ it.each([
+ {
+ name: "handles circular key",
+ setup: () => {
+ const elem = {
+ $$typeof: Symbol.for("react.element"),
+ type: "div",
+ key: null,
+ ref: null,
+ props: { children: [] },
+ };
+ elem.key = elem;
+ return elem;
+ },
+ assertion: (output) => expect(output).toContain("[Circular]"),
+ },
+ {
+ name: "handles circular children",
+ setup: () => {
+ const elem = {
+ $$typeof: Symbol.for("react.element"),
+ type: "div",
+ key: null,
+ ref: null,
+ props: {},
+ };
+ elem.props.children = elem;
+ return elem;
+ },
+ assertion: (output) => expect(output).toContain("[Circular]"),
+ },
+ {
+ name: "handles non-object props",
+ setup: () => ({
+ $$typeof: Symbol.for("react.element"),
+ type: "div",
+ key: null,
+ ref: null,
+ props: 123,
+ }),
+ assertion: (output) => expect(output).toBe("<div />"),
+ },
+ ])("$name", ({ setup, assertion }) => {
+ assertion(Bun.inspect(setup()));
+ });
});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@test/js/bun/util/inspect.test.js` around lines 777 - 810, Replace the three
separate it(...) cases ("handles circular key", "handles circular children",
"handles non-object props") with a single parameterized it.each(...) table that
supplies a description, a function or factory to build the elem fixture (so you
can create circular references inside the factory for the first two cases), and
the expected assertion type/value; call Bun.inspect on the produced elem in the
test body and branch the assertion: expect(...).toContain("[Circular]") for the
first two fixtures and expect(...).toBe("<div />") for the non-object props
fixture. Ensure you reference the same symbols used now (elem, props, $$typeof,
Bun.inspect) so the factory builds identical shapes and the descriptions match
the original test names.
|
Found 2 issues this PR may fix:
🤖 Generated with Claude Code |
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
|
Duplicate of #29709 which already covers both the circular JSX fix and the non-object props fix. |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
LGTM — straightforward crash fix with tests.
Extended reasoning...
Overview
This PR fixes two crashes in Bun.inspect() / jest pretty-format when handling malformed JSX-shaped objects (objects with $$typeof: Symbol.for("react.element")). It touches src/jsc/ConsoleObject.zig, src/test_runner/pretty_format.zig, and adds three regression tests in test/js/bun/util/inspect.test.js.
The two changes are:
- Add
.JSXtocanHaveCircularReferences()so the existing visited-map cycle detection inprintAs()kicks in for JSX elements, preventing unbounded recursion → stack overflow whenkeyorprops.childrenform a cycle. - Replace
props.getObject().?(which panics on null) with an upfrontif (... props.getObject() else null) |props_obj|guard, so non-objectpropsvalues are simply skipped and the element renders as<div />.
Security risks
None. This is purely a defensive hardening of debug-output formatting against fuzzer-generated inputs. No new attack surface, no auth/crypto/permissions involvement; if anything it removes a DoS-via-crash vector for code that logs untrusted objects.
Level of scrutiny
Low. The diff is ~10 lines of logic across two near-identical formatters, both following the established pattern already used for .Array, .Object, .Map, .Set. The getObject() null-check is the idiomatic way this codebase guards optional object coercion. Behavior for well-formed JSX elements is unchanged (props_obj.toJS() round-trips to the same JSValue that was previously bound as props).
Other factors
Three new tests directly exercise each crash path (circular key, circular children, non-object props) and assert the expected [Circular] / <div /> output. The bug-hunting system found no issues. No outstanding reviewer comments on the PR.
1 participant
Bun.inspect()on a React element (object with$$typeofset toSymbol.for("react.element")) would segfault in two cases:If the element contained a circular reference through
keyorprops.children, the formatter recursed without cycle detection until stack overflow. The.JSXtag was missing fromcanHaveCircularReferences().If
propswas not an object,getObject().?unwrapped null and panicked.The same fixes are applied to the jest pretty-format path in
src/test_runner/pretty_format.zig.Found by Fuzzilli (fingerprint
c3da574162c76702).