Skip to content

css: don't duplicate vendor-prefixed nested rules per ancestor prefix pass#31270

Open
robobun wants to merge 3 commits into
mainfrom
farm/4fba0ae2/css-nested-prefix-oom
Open

css: don't duplicate vendor-prefixed nested rules per ancestor prefix pass#31270
robobun wants to merge 3 commits into
mainfrom
farm/4fba0ae2/css-nested-prefix-oom

Conversation

@robobun
Copy link
Copy Markdown
Collaborator

@robobun robobun commented May 23, 2026

What does this PR do?

Fixes an OOM found by fuzzing in the CSS printer: a ~4.8 KB stylesheet of deeply nested :fullscreen blocks makes re-serialization allocate multiple gigabytes when browser targets are set (including the bundler's default --target=browser targets).

# before: >4 GB RSS, aborts with "memory allocation of ... failed"
# after:  1.9 KB output file
bun build app.css --target=browser --minify

where app.css is ~28 levels of nested :fullscreen { ... } rules (fuzzer signature oom:css:…serialize_selector|…serialize_nesting|…serialize_selector|…serialize_selector_list|…StyleRule::to_css_base).

Cause

When CSS nesting is compiled away for older targets, StyleRule::to_css serializes the rule once per vendor prefix (:fullscreen:-webkit-full-screen + :fullscreen for e.g. Safari 14), and each prefix pass re-serializes all nested rules. A nested rule that has its own vendor prefixes immediately overrides dest.vendor_prefix with its own prefix loop, so its output is byte-for-byte identical in every ancestor pass — each nesting level doubles the output. At 28 levels that's 2²⁷ copies of the leaf rules → OOM. (The re-serialized copies are exact duplicates; with one prefixed selector per level only two distinct rules ever exist.)

Fix

Track the vendor-prefix pass in StyleRule::to_css and, while re-serializing nested rules for a non-final pass, skip nested style rules that carry their own vendor prefixes (Printer.skip_prefixed_nested_rules). They are emitted exactly once, in the ancestor's final pass. Nested rules without their own prefixes still print in every pass, because their & expansion depends on the current prefix.

Output is semantically unchanged — the same set of rules is emitted, just without the exact duplicates — and grows linearly with nesting depth instead of exponentially.

Verification

  • New test test/js/bun/css/nested-vendor-prefix-duplication.test.ts:
    • exact minified output for 3 nested prefixed levels (no duplicates)
    • depth-16 output stays linear (was ~18 MB before, 2 rules after)
    • unprefixed nested rules still expand once per ancestor prefix pass (behavior preserved)
    • bun build --target=browser --minify on the nested input produces a small file
    • fails on the unfixed build (3/4 tests), passes with the fix
  • Original fuzzer input: minifyTest(input, "", {safari: 8<<16}) now returns 1,892 bytes (previously aborted >4 GB); the no-targets repro output is byte-identical before/after.
  • test/js/bun/css/ (2018 tests) and test/bundler/css/ + test/bundler/esbuild/css.test.ts (219 tests) pass with the fix.

… pass

When CSS nesting is compiled away for older targets, a style rule whose
selector needs vendor prefixes is serialized once per prefix, and each
pass re-serialized all of its nested rules. Nested rules that carry
their own vendor prefixes override the printer's vendor prefix, so
those re-serializations were exact duplicates, doubling the output for
every nesting level. A few kilobytes of nested prefixed selectors (e.g.
:fullscreen) made the printer allocate gigabytes.

Nested style rules with their own vendor prefixes are now only emitted
during the final prefix pass of their ancestor, so output stays linear
in nesting depth. Rules without their own prefixes still expand once
per ancestor pass, since their output depends on the current prefix.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 23, 2026

Warning

Review limit reached

@robobun, we couldn't start this review because you've used your available PR reviews for now.

Your plan currently allows 2 reviews/hour. Refill in 21 minutes and 45 seconds.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more review capacity refills, 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 trial, open-source, and free plans. In all cases, review capacity refills continuously over time.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 61757549-c016-4ddc-a2db-615dd95bfada

📥 Commits

Reviewing files that changed from the base of the PR and between ffd67d8 and 7c2907b.

📒 Files selected for processing (4)
  • src/css/printer.rs
  • src/css/rules/mod.rs
  • src/css/rules/style.rs
  • test/js/bun/css/nested-vendor-prefix-duplication.test.ts

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

@robobun
Copy link
Copy Markdown
Collaborator Author

robobun commented May 23, 2026

Updated 5:05 AM PT - May 23rd, 2026

@robobun, your commit 7c2907b has some failures in Build #57254 (All Failures)


🧪   To try this PR locally:

bunx bun-pr 31270

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

bun-31270 --bun

Comment thread src/css/rules/style.rs
Comment thread test/js/bun/css/nested-vendor-prefix-duplication.test.ts Outdated
A non-final vendor prefix pass can now emit nothing when the rule has no
declarations and all of its nested rules are deferred to the final pass;
don't leave a dangling blank-line separator in pretty output for it.

Also drop the explicit test-level timeout from the new regression test
and pin the non-minified output.
Comment thread src/css/rules/style.rs Outdated
…rred

When a rule has its own declarations and all of its nested rules are
deferred to the final vendor prefix pass, the non-final pass wrote the
separator between the declaration block and the nested rules with
nothing following it. Check whether any nested rule will be emitted in
the current pass before writing that separator, and share the deferral
predicate with the rule-list printer.
@Jarred-Sumner
Copy link
Copy Markdown
Collaborator

@robobun compare with lightningcss

@robobun
Copy link
Copy Markdown
Collaborator Author

robobun commented May 23, 2026

Compared against lightningcss 1.32.0 (npm, same inputs, same targets). Short version: before this PR Bun's output here is byte-for-byte identical to lightningcss — including the exponential duplication — and lightningcss itself OOMs on the same 4.8 KB fuzzer input. The only thing this PR changes is that byte-identical duplicate rules are emitted once instead of once per ancestor prefix pass; every rule we emit is still a rule lightningcss emits.

Growth of minified output for :fullscreen { :fullscreen { … { color: red } … } } nested depth levels, targets { safari: 8<<16 }:

depth lightningcss 1.32.0 Bun before (1.4 canary, and 1.3.14) Bun after this PR
1 53 B 53 B 53 B
2 172 B 172 B 86 B
3 476 B 476 B 119 B
6 6,976 B 6,976 B 218 B
9 81,152 B 81,152 B 317 B
12 851,968 B 851,968 B 416 B
28 (fuzz input, 4.8 KB) aborts, memory allocation of … failed under a 3 GB cap same (1.4 canary: >4 GB RSS; 1.3.14: printer allocation failure) 1,892 B

lightningcss/Bun-before double per level (each prefixed ancestor re-serializes all nested rules per prefix pass); after the PR it's +33 B per level.

Depth 2, what the duplication looks like — lightningcss (and Bun before):

:-webkit-full-screen :-webkit-full-screen{color:red}:fullscreen :fullscreen{color:red}:-webkit-full-screen :-webkit-full-screen{color:red}:fullscreen :fullscreen{color:red}

Bun after (the same two rules, once):

:-webkit-full-screen :-webkit-full-screen{color:red}:fullscreen :fullscreen{color:red}

When the parent has its own declarations (:fullscreen { color: green; :fullscreen { color: red } }) lightningcss emits the nested pair inside each parent pass; we emit it once after the last parent variant — same rule set, cascade-equivalent order (nested rules still come after every parent rule they can override):

/* lightningcss / Bun before */
:-webkit-full-screen{color:green}:-webkit-full-screen :-webkit-full-screen{color:red}:fullscreen :fullscreen{color:red}:fullscreen{color:green}:-webkit-full-screen :-webkit-full-screen{color:red}:fullscreen :fullscreen{color:red}
/* Bun after */
:-webkit-full-screen{color:green}:fullscreen{color:green}:-webkit-full-screen :-webkit-full-screen{color:red}:fullscreen :fullscreen{color:red}

Everything else stays byte-identical to lightningcss. The dedup only applies when nesting is being compiled away and both the ancestor and the nested rule need vendor prefixes. Cases with only one of the two are unchanged, e.g. :fullscreen { div { color: red } }:-webkit-full-screen div{color:red}:fullscreen div{color:red} (identical in all three columns), and the lightningcss-derived suite in test/js/bun/css/css.test.ts (1032 tests) passes unchanged.

For the record, Bun 1.3.14 (the Zig CSS implementation) produces the same byte counts as lightningcss on all of the above, so the blowup wasn't introduced by the Rust port — it's faithful lightningcss behavior that both ports inherited; current lightningcss still does it.

Comment thread src/css/rules/style.rs
Comment thread src/css/rules/mod.rs
@robobun
Copy link
Copy Markdown
Collaborator Author

robobun commented May 23, 2026

CI status on 7c2907b (build #57254): 74 checks pass — every lane that exercises this change is green on every platform: linux x64/aarch64 (incl. the ASAN test lane), alpine/debian/ubuntu, windows x64/aarch64, darwin-14-x64, darwin-26-aarch64 (passed on retry), all build jobs, format/clippy/lint.

The single remaining red lane is darwin-14-aarch64-test-bun, which has now Expired twice (the original job and its retry never got picked up by a macOS 14 aarch64 agent — nothing was built or tested there). The same suite already passes on macOS 14 x64 and on macOS 26 aarch64, so this is purely agent availability: it needs one more Retry on the Buildkite build when an agent frees up. Not pushing an empty commit for this since it would restart the entire otherwise-green build.

@robobun
Copy link
Copy Markdown
Collaborator Author

robobun commented May 23, 2026

The fuzzer also hit this same blowup as a hang signature (hang:css:MemcmpInterceptorCommon(…)): a 314-byte input of ~21 nested co.foo:is(a){ rules (only the innermost block closed, the rest implicitly closed at EOF) with browser targets set. :is() only downlevels to two or three variants per level, so the per-level duplication shows up as minutes of selector re-serialization rather than gigabytes of allocation — but it's the same StyleRule::to_css prefix-pass × nested-rules multiplication.

Verified this PR also resolves that signature by cherry-picking the three commits onto current main (3c92be8):

  • cssInternals.minifyTest(<314-byte input>, "", { chrome, safari, firefox, edge, ios_saf, samsung, opera, android, ie }): hung indefinitely before (killed after 5+ minutes on a debug build), returns a 1,257-byte result in ~4 ms after.
  • bun build --minify on 18 levels of .x:is(a){ (a 189-byte file, default targets): previously never finished; now a 787-byte output in ~0.3 s.
  • The commits apply cleanly to current main and test/js/bun/css/nested-vendor-prefix-duplication.test.ts passes there (6/6).

For comparison I pushed a smaller alternative take on the same root cause to farm/0810174e/fix-css-nested-prefix-print: nested style rules simply inherit the ancestor's active prefix pass instead of starting their own pass loop (no printer flag / deferral needed), plus a regression test covering the :is() nesting shape. Output matches this PR byte-for-byte on every case I tested, so it's only there in case the simpler shape is preferred — no competing PR opened.

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.

2 participants