Skip to content

css: bound compiled-nesting selector-prelude bytes and widen indent counter - #32454

Closed
robobun wants to merge 6 commits into
mainfrom
farm/23c6bd7d/css-nesting-prelude-byte-cap
Closed

robobun wants to merge 6 commits into
mainfrom
farm/23c6bd7d/css-nesting-prelude-byte-cap

Conversation

@robobun

@robobun robobun commented Jun 17, 2026 •

Copy link
Copy Markdown
Collaborator

What does this PR do?

Fixes a CSS minifier OOM found by fuzzing (signature oom:css:_RNvC7___rustc12___rust_alloc|_RNvXs_NtNtC5alloc3vec21spec_from_iter_nestedINtB6_3VecINtNtC7bun_css5rules7C10css, on 1.4.0-canary.1+55f6c899f): a ~10 KB input drove the CSS printer to ~200 MB of output and ~1.6 GB peak RSS when compiling nesting for Safari 13, without tripping any existing limit.

Root cause

When the browser targets don't support CSS nesting, every nested rule's selector prelude inlines the full parent-selector chain (serialize_nesting walks the StyleContext up to the root). The existing MAX_SELECTOR_EXPANSION cap (65,536) bounds how many rules that expansion produces, but not how long each rule's prelude is; the per-prelude MAX_NESTING_EXPANSIONS cap resets at every rule. Nesting depth is bounded only by the parser's 512-level MAX_NESTING_DEPTH.

By placing many single-selector levels above a handful of two-selector levels whose selectors are incompatible with the targets (so they are partitioned into one rule per selector instead of collapsed into :is()), the selector-expansion total stays well under 65,536 while each of the tens of thousands of resulting rules prints a prelude hundreds of ancestors long. On current main (all existing caps intact, release build):

// ~8.5 KB in -> ~230 MB out, no error
const c = require("bun:internal-for-testing").cssInternals;
let css = ".padding-selector {\n".repeat(400)
  + ".a:-webkitx .a, .b:-webkitx .b {\n".repeat(15) + "color: red;";
c._test(css, "", { safari: (13 << 16) | (2 << 8) });

The minimized 10 KB fuzzer input (283 nesting levels, ~14 of which are two-selector with custom/vendor-prefixed pseudo-classes) reaches the same path: selector_expansion_total tops out at 50,320 (under 65,536), output is ~200 MB, RSS peaks at ~1.6 GB including the returned JS string.

Reachable via bun build --minify with default browser targets.

Fix

  1. Track the selector-prelude bytes written while a StyleContext parent chain is being inlined (i.e. the rule is being de-nested) and cap them at MAX_NESTING_EXPANSION_BYTES = 64 MB across the whole stylesheet, mirroring the existing MAX_PREFIX_EXPANSION_BYTES. Past the cap the printer reports the existing maximum_nesting_expansion error. Top-level rules (no parent context) are not charged, so large flat stylesheets are unaffected.

  2. Widen Printer.indent_amt from u8 to u16. The parser accepts up to 512 nesting levels and each level indents by 2, so preserving nesting past depth 128 overflowed the counter: a panic (attempt to add with overflow) in debug builds and wrong indentation in release builds.

Relationship to open PRs

This PR is the minimal standalone fix that handles every pass of this specific fuzzer reproduction; textual conflicts in printer.rs/style.rs/scope.rs are expected with whichever of the above lands first.

Verification

  • The exact fuzzer repro (all four passes: minifyTest, _test, prefixTest, round-trip minifyTest) now either completes or throws Maximum nesting expansion exceeded instead of allocating >1 GB.
  • New tests in test/js/bun/css/nested-selector-list-expansion.test.ts. On an unfixed build: the padded-then-forked test returns the ~230 MB output instead of throwing, and the depth-500 indentation test panics with the overflow in debug builds. On this branch all 29 tests in the file pass.
  • A 4,000-rule × 3-level realistic nested stylesheet and shallow padded-then-forked nesting stay well under the budget and compile unchanged.
  • test/js/bun/css/css.test.ts (1093 pass), nested-selector-expansion.test.ts, nested-vendor-prefix-duplication.test.ts, test/bundler/css/ (167 pass). The css-fuzz.test.ts debug-build timeouts are pre-existing on main.

…indent counter

When compiling CSS nesting for targets that don't support it, every nested
rule's selector prelude inlines the full parent-selector chain. The existing
MAX_SELECTOR_EXPANSION cap bounds how many rules that expansion produces, but
not how long each rule's prelude is (nesting depth is capped at 512 by the
parser). A stylesheet that stacks many single-selector levels above a handful
of multi-selector levels stays under the 65,536-rule cap while each of those
rules prints a prelude hundreds of levels long: a 10 KB input reached ~200 MB
of output and ~1.6 GB RSS (fuzzer signature
oom:css:...spec_from_iter_nested...bun_css5rules).

Track the selector-prelude bytes written under a compiled-nesting
StyleContext and cap them at 64 MB (same as MAX_PREFIX_EXPANSION_BYTES),
reporting the existing maximum_nesting_expansion printer error.

Also widen Printer.indent_amt from u8 to u16: the parser accepts up to 512
nesting levels and each level indents by 2, so preserving nesting past depth
128 overflowed the counter (a panic in debug builds, wrong indentation in
release).
@coderabbitai

coderabbitai Bot commented Jun 17, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@autofix-ci[bot], we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 9 minutes and 56 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 64472fec-c90e-4e72-b374-c2a2ad43ba3a

📥 Commits

Reviewing files that changed from the base of the PR and between 2699c7e and 35cc6d0.

📒 Files selected for processing (1)
  • test/js/bun/css/nested-selector-list-expansion.test.ts

Walkthrough

The change widens Printer::indent_amt from u8 to u16, adds a nesting_expansion_bytes: usize accumulator to Printer, and introduces a 64 MiB MAX_NESTING_EXPANSION_BYTES constant. Both StyleRule::to_css_base and ScopeRule::to_css now measure selector-prelude bytes emitted during compiled nesting, accumulate the delta into the printer's counter, and return a maximum_nesting_expansion error when the budget is exceeded. Regression tests cover error triggering, safe cases, and indent-depth overflow.

Changes

CSS Nesting Expansion Budget

Layer / File(s) Summary
Printer field additions: indent_amt widening and nesting_expansion_bytes
src/css/printer.rs
indent_amt is widened from u8 to u16 with updated docs; nesting_expansion_bytes: usize field is added to accumulate selector-prelude byte growth; Printer::new initializes the new field to 0.
StyleRule nesting expansion measurement and budget enforcement
src/css/rules/style.rs
MAX_NESTING_EXPANSION_BYTES (64 MiB) constant is defined; to_css_base records bytes_written() before and after serialize_selector_list for non-top-level compiled nesting, accumulates the delta into dest.nesting_expansion_bytes, and returns maximum_nesting_expansion when exceeded.
ScopeRule nesting expansion metering
src/css/rules/scope.rs
ScopeRule::to_css detects whether prelude expansion may occur, measures bytes emitted during prelude serialization, increments dest.nesting_expansion_bytes by the delta, and returns maximum_nesting_expansion error when the budget is exceeded.
Regression tests: byte limit errors, safe cases, and indent overflow
test/js/bun/css/nested-selector-list-expansion.test.ts
NESTING_LIMIT_ERROR constant and deepPadThenFork helper are introduced; subprocess test asserts deep forked nesting triggers the limit error; no-target and shallow cases verify safe paths; @scope deep subprocess tests confirm limit enforcement; @scope shallow cases verify correct serialization; a realistic stylesheet test passes without hitting the limit; a subprocess test confirms depth-500 indentation does not overflow.

Possibly related PRs

  • oven-sh/bun#31276: Implements the original maximum_nesting_expansion error mechanism and caps runaway & nesting expansion — the same error path that this PR's MAX_NESTING_EXPANSION_BYTES enforcement triggers.
  • oven-sh/bun#31482: Mitigates nested-selector expansion runaway caused by nesting-holding at-rules (notably @scope/@nest) by ensuring those at-rules recurse into nested rules during minify so selector-expansion caps apply to the full depth.
  • oven-sh/bun#31642: Adds expansion byte counters (prefix_expansion_bytes for vendor-prefix fan-out) to src/css/printer.rs and src/css/rules/style.rs, conceptually and structurally parallel to this PR's nesting_expansion_bytes budgeting approach.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the two main technical changes: bounding compiled-nesting selector-prelude bytes and widening the indent counter from u8 to u16.
Description check ✅ Passed The description includes detailed explanations of what the PR does, the root cause of the OOM issue, the fixes implemented, and comprehensive verification details. It matches the repository template structure with 'What does this PR do?' and 'How did you verify your code works?' sections.
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.

@robobun

robobun commented Jun 17, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 11:31 PM PT - Jun 16th, 2026

❌ @autofix-ci[bot], your commit 35cc6d0 has 2 failures in Build #63096 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 32454

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

bun-32454 --bun

@github-actions

Copy link
Copy Markdown
Contributor

This PR may be a duplicate of:

  1. css: budget selector expansion and nesting substitution by bytes #31916 - Adds the same Printer.nesting_expansion_bytes counter bounded by MAX_NESTING_EXPANSION_BYTES (64 MB) on the printer side, plus a minify-side selector-weight byte budget; superset of this PR's fix
  2. css: bound nested-prelude substitution output across the stylesheet #32451 - Adds the same Printer::nesting_expansion_bytes metering bounded by MAX_NESTING_EXPANSION_BYTES (64 MB) to cap nested-prelude serialization output; same mechanism and same constant

🤖 Generated with Claude Code

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🤖 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 `@test/js/bun/css/nested-selector-list-expansion.test.ts`:
- Around line 300-304: The strict assertion checking stderr equality in the
nested-selector-list-expansion test can flake in debug/ASAN builds due to benign
startup noise. Replace the exact stderr equality check with conditional stderr
normalization that filters out non-actionable debug/ASAN noise while still
detecting actual panic signatures. Keep the existing assertions for exitCode,
signalCode, and stdout validation as these provide sufficient crash detection
without being prone to environmental flakes.
🪄 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: cdb4fa99-2827-468a-8ee2-e42dfe1c5607

📥 Commits

Reviewing files that changed from the base of the PR and between 0c537fe and ef9db52.

📒 Files selected for processing (3)
  • src/css/printer.rs
  • src/css/rules/style.rs
  • test/js/bun/css/nested-selector-list-expansion.test.ts

Comment thread test/js/bun/css/nested-selector-list-expansion.test.ts Outdated
…assertion

@scope preludes serialize with a parent context the same way style-rule
preludes do (scope-end with scope-start as a temporary parent, or an outer
StyleContext), so charge them against MAX_NESTING_EXPANSION_BYTES too.

Replace the exact stderr equality in the subprocess test with a
combined-object assertion (stderr checked only for panic signatures), per
the repo convention for subprocess output.
@robobun

robobun commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator Author

Re the duplicate flag: all three PRs (#31913, #31916, #32451) address overlapping pieces of the same CSS nesting-expansion bug family, for different fuzzer signatures. I verified against this PR's exact fuzzer input (signature oom:css:...spec_from_iter_nested...bun_css5rules, which runs minifyTest/_test/prefixTest/round-trip minifyTest):

This PR is the only standalone change that handles all four passes of this specific reproduction. The PR body's "Relationship to open PRs" section has the same summary; 6881223 adds the @scope prelude metering from #32451 so the byte-budget side is equivalent.

Comment thread src/css/rules/style.rs
Comment thread test/js/bun/css/nested-selector-list-expansion.test.ts Outdated
Adds the bypass scenario from review (an @scope prelude with & nested
under the deep pad + fork chain) as an explicit regression test, plus a
shallow positive case proving @scope preludes under compiled nesting
still serialize below the budget.
Comment thread src/css/rules/scope.rs
Comment thread src/css/printer.rs Outdated
robobun and others added 2 commits June 17, 2026 05:52
…ring

When @scope has a <scope-end> but no <scope-start>, ScopeRule::to_css
returned immediately after serializing <scope-end>, skipping the closing
paren, the rule body, the closing brace, and the nesting-expansion byte
charge added in 6881223. @scope to (& .x) nested under the
padded-then-forked chain therefore still inlined the full ancestor chain
per cloned @scope without the byte budget ever firing (~121 MB output
from an ~8.5 KB input), and the emitted CSS was truncated to
'@scope to (<scope-end>' regardless of nesting.

Drop the early return so the scope-end-only branch falls through to the
existing close-paren, byte-metering, and body serialization. Add
regression tests for both the byte-budget bypass and the truncated
output, and update the nesting_expansion_bytes doc comment to list
ScopeRule::to_css alongside StyleRule::to_css_base.
@robobun

robobun commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator Author

CI on 35cc6d0: 284/286 jobs passed. The two failures are unrelated to this diff:

  • test/js/bun/test/parallel/test-docker-build-debian.ts on darwin 26 aarch64: Docker Hub 429 Too Many Requests pulling debian:trixie-slim ("You have reached your unauthenticated pull rate limit").
  • test/js/web/fetch/fetch-leak.test.ts on darwin 14 aarch64: should not leak using readable stream RSS-threshold assertion off by ~2.4 MB (expected ≥54.3, got 51.9).

test/js/bun/css/nested-selector-list-expansion.test.ts (the file this PR modifies) passed on every lane. Ready for review.

@robobun

robobun commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator Author

Stale PR review: closing.

No human has commented on or reviewed this PR since it opened on 2026-06-17, and it conflicts with main. Each source change in it is merged or in another open PR. The indent counter is already u32 with saturating arithmetic on main (src/css/printer.rs:119, from #36165), so the u16 here would narrow it. #31916 adds the same Printer::nesting_expansion_bytes counter, the same MAX_NESTING_EXPANSION_BYTES = 64 << 20 and the same maximum_nesting_expansion error, and it checks the total inside each & substitution (src/css/selectors/selector.rs). The @scope to (...) fix in src/css/rules/scope.rs is byte-identical in #33679.

The bug is still on main and stays tracked by #31916. With bun 1.4.3, bun build turns the 8,921-byte stylesheet from the description (400 padding levels above 15 two-selector levels) into a 243,925,004-byte asset and exits 0. #31916 has no test with this padded-then-forked shape, so the deepPadThenFork tests in test/js/bun/css/nested-selector-list-expansion.test.ts are worth moving there.

Reopen if this evidence is wrong.

@robobun robobun closed this Sep 11, 2026
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