Skip to content

fix(inspect): handle circular refs and non-object props in JSX formatter#30362

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

fix(inspect): handle circular refs and non-object props in JSX formatter#30362
robobun wants to merge 1 commit into
mainfrom
farm/273f11fe/fix-jsx-inspect-circular

Conversation

@robobun

@robobun robobun commented May 7, 2026

Copy link
Copy Markdown
Collaborator

What does this PR do?

Bun.inspect() / console.log() crashed with a stack overflow when formatting a React element whose props (or a prop value, or children) referenced the element itself:

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

The .JSX tag was missing from canHaveCircularReferences(), so the visited-map check and the StackCheck guard that protect other recursive formatters (.Object, .Array, .Map, …) never ran for JSX elements, allowing unbounded recursion into props/children.

Also removes the props.getObject().? force-unwrap — since $$typeof detection works on any object, props is user-controlled and can be a primitive, which previously panicked with "attempt to use null value".

Now prints [Circular] like other circular structures:

<div foo=[Circular] />

Found by Fuzzilli. Fingerprint: ae1fb4ec8882b8cf

How did you verify your code works?

Added tests in test/js/bun/util/inspect.test.js covering circular props, circular prop values, circular children, and non-object props. All 76 inspect tests pass with bun bd test; the new tests crash the system bun.

The JSX/React element formatter in Bun.inspect/console.log did not
participate in circular reference detection, so inspecting an element
whose props (or children) referenced the element itself would recurse
until the stack overflowed and crash.

Add .JSX to canHaveCircularReferences so it gets the same visited-map
and stack-check protection as Object/Array/etc, and guard against
non-object props values instead of force-unwrapping.
@coderabbitai

coderabbitai Bot commented May 7, 2026

Copy link
Copy Markdown
Contributor

Warning

Rate limit exceeded

@robobun has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 1 minute and 33 seconds before requesting another review.

To continue reviewing without waiting, purchase usage credits in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: a94b0e9c-528f-4c35-b929-b0d6f28f513f

📥 Commits

Reviewing files that changed from the base of the PR and between c5a2f8f and 37e2701.

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

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

@robobun

robobun commented May 7, 2026

Copy link
Copy Markdown
Collaborator Author
Updated 1:15 AM PT - May 7th, 2026

@robobun, your commit 37e2701 has 1 failures in Build #52492 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 30362

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

bun-30362 --bun

@github-actions github-actions Bot added the claude label May 7, 2026
@github-actions

github-actions Bot commented May 7, 2026

Copy link
Copy Markdown
Contributor

This PR may be a duplicate of:

  1. Fix stack overflow in Bun.inspect for circular JSX elements #30126 - Also adds .JSX to canHaveCircularReferences() in ConsoleObject.zig to fix stack overflow on circular JSX elements
  2. Fix stack overflow in Bun.inspect() on circular JSX elements #29709 - Also adds .JSX to canHaveCircularReferences() and guards getObject() on props, with fixes in both ConsoleObject.zig and pretty_format.zig

🤖 Generated with Claude Code

@robobun

robobun commented May 7, 2026

Copy link
Copy Markdown
Collaborator Author

Duplicate of #29709 (which also covers pretty_format.zig). Closing in favor of that.

@robobun robobun closed this May 7, 2026
@robobun robobun deleted the farm/273f11fe/fix-jsx-inspect-circular branch May 7, 2026 08:14

@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, well-tested fix for a fuzzer-found crash.

Extended reasoning...

Overview

This PR fixes two crashes in Bun.inspect()'s JSX formatter (src/jsc/ConsoleObject.zig): (1) adds .JSX to canHaveCircularReferences() so the existing visited-map and StackCheck guard at the top of printAs now protects JSX elements from infinite recursion through circular props/children, and (2) replaces the props.getObject().? force-unwrap with a safe if (props.getObject()) |props_obj| so a primitive props value no longer panics. Four regression tests are added to test/js/bun/util/inspect.test.js.

Security risks

None. This is purely output-formatting code for Bun.inspect/console.log. The change strictly hardens against malformed input (circular refs, non-object props) found by Fuzzilli; no auth, crypto, or permission paths are touched.

Level of scrutiny

Low. The first change is a one-token addition to an existing switch arm, reusing the exact circular-reference machinery already applied to .Object, .Array, .Map, etc. — the guard wraps the whole switch (comptime Format) block, so .JSX slots in with no behavioral surprises beyond printing [Circular] instead of overflowing the stack. The second change replaces an incorrect SAFETY: assumption (props are not guaranteed to be objects since $$typeof detection works on any user object) with a null-safe optional unwrap; non-object props now render as <div />, which is reasonable.

Other factors

The new tests cover all four scenarios (circular props object, circular prop value, circular children, primitive props) and the description confirms all 76 inspect tests pass while the new tests crash the unpatched binary. No CODEOWNERS apply to these paths and the bug-hunting system found no issues.

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