Skip to content

inspect: handle circular references in JSX elements#29583

Closed
robobun wants to merge 1 commit into
mainfrom
farm/267e2b52/fix-jsx-circular-inspect
Closed

inspect: handle circular references in JSX elements#29583
robobun wants to merge 1 commit into
mainfrom
farm/267e2b52/fix-jsx-circular-inspect

Conversation

@robobun

@robobun robobun commented Apr 22, 2026

Copy link
Copy Markdown
Collaborator

What does this PR do?

Bun.inspect() on a React element whose key, props, or children referenced itself (directly or indirectly) would recurse until the stack overflowed and the process segfaulted.

The .JSX tag was missing from canHaveCircularReferences(), so the visited-set check and stack guard at the top of printAs were skipped for React elements. Formatting the key (or any prop/child) would re-enter the .JSX branch with the same value forever.

Repro

const el = {
  $$typeof: Symbol.for("react.element"),
  type: "div",
  key: null,
  props: {},
};
el.key = el;
Bun.inspect(el); // segfault (stack overflow)

Now prints <div key=[Circular] />.

How did you verify your code works?

  • Fuzzer repro no longer crashes
  • Added regression test in test/js/bun/util/inspect.test.js covering circular key, prop value, and children
  • Existing JSX inspect tests still pass

Found by Fuzzilli (fingerprint 6e718886f45af13a).

Bun.inspect() on a React element whose key, props, or children referenced
itself would recurse until the stack overflowed. The .JSX tag was missing
from canHaveCircularReferences(), so the visited-set check and stack guard
in printAs were skipped.
@robobun

robobun commented Apr 22, 2026

Copy link
Copy Markdown
Collaborator Author
Updated 5:05 AM PT - Apr 22nd, 2026

@robobun, your commit 3041c69 has 2 failures in Build #47159 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 29583

That installs a local version of the PR into your bun-29583 executable, so you can run:

bun-29583 --bun

@coderabbitai

coderabbitai Bot commented Apr 22, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 6d0c3641-3f9a-4dbd-bfb0-ad6a93f6edb7

📥 Commits

Reviewing files that changed from the base of the PR and between 7a4e667 and 3041c69.

📒 Files selected for processing (2)
  • src/bun.js/ConsoleObject.zig
  • test/js/bun/util/inspect.test.js

Walkthrough

The changes extend the circular-reference detection mechanism to include JSX values and add test coverage verifying that self-referential React-element-like objects are properly rendered with circular-reference markers.

Changes

Cohort / File(s) Summary
JSX Circular Reference Handling
src/bun.js/ConsoleObject.zig, test/js/bun/util/inspect.test.js
Updated canHaveCircularReferences logic to classify JSX values as eligible for circular-reference detection. Added comprehensive test case validating Bun.inspect output with self-referential JSX structures using both single-line and multi-line formatting.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding circular reference handling to JSX element inspection, which aligns with the code changes (updating canHaveCircularReferences for .JSX and adding corresponding tests).
Description check ✅ Passed The description covers both required template sections: 'What does this PR do?' explains the bug, root cause, and fix with clear examples, and 'How did you verify your code works?' details the verification steps including fuzzer repro, regression tests, and existing test validation.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@claude claude Bot left a comment

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.

LGTM — minimal one-line fix adding .JSX to the circular-reference guard, with regression tests.

Extended reasoning...

Overview

This PR adds .JSX to the canHaveCircularReferences() switch in src/bun.js/ConsoleObject.zig so that the visited-set / stack-safety guard at the top of printAs runs for React elements. It also adds a regression test in test/js/bun/util/inspect.test.js covering circular key, prop value, and children. Total production change is one token added to an existing switch arm.

Security risks

None. This is purely a console/inspect formatting code path with no auth, crypto, I/O, or untrusted-input parsing implications. The change strictly adds a defensive guard (cycle detection + stack check) that was already applied to every other recursive container tag.

Level of scrutiny

Low. The fix is mechanical and follows the established pattern exactly: .Function, .Array, .Object, .Map, .Set, .Error, .Class, .Event already participate in the same guard, and .JSX is the obvious missing member since its formatter recurses into arbitrary user values (key/props/children). I verified at ConsoleObject.zig:2094-2123 that the guard inserts the value into this.map, prints [Circular] on revisit, and removes it on exit via defer, so adding .JSX simply opts that branch into the same well-tested machinery.

Other factors

The PR was generated from a Fuzzilli crash repro and includes three targeted assertions that exercise each recursion entry point in the .JSX branch. No bugs were flagged by the bug-hunting system, there are no outstanding reviewer comments, and the change cannot regress non-circular JSX output (the guard is a no-op on first visit).

@github-actions

Copy link
Copy Markdown
Contributor

This PR may be a duplicate of:

  1. Fix stack overflow when inspecting JSX elements with circular references #29170 - Both PRs add .JSX to canHaveCircularReferences() in ConsoleObject.zig to fix stack overflow when inspecting JSX elements with circular references

🤖 Generated with Claude Code

@robobun

robobun commented Apr 22, 2026

Copy link
Copy Markdown
Collaborator Author

Duplicate of #29170, which already covers this fix plus the same bug in pretty_format.zig and the non-object props case.

@robobun robobun closed this Apr 22, 2026
@robobun robobun deleted the farm/267e2b52/fix-jsx-circular-inspect branch April 22, 2026 06:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant