css: cap & parent-selector expansion when compiling nesting for older targets - #31276
Conversation
…er targets When browser targets lack CSS nesting support, every `&` in a nested rule's selector is replaced with the parent selector at print time. The parent may itself contain `&`, so selectors with multiple `&` references per nesting level expand to (references per level)^depth copies of their ancestors; a few KB of deeply nested input makes the printer allocate without bound. Budget the number of parent-selector substitutions per rule prelude (65,536) and report a "Maximum nesting expansion exceeded" printer error instead of expanding forever. Preserved nesting (no targets) and ordinary nested CSS are unaffected.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughBun's CSS compiler now prevents unbounded expansion of deeply nested ChangesCSS nesting expansion limit guard
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
Updated 8:55 AM PT - May 23rd, 2026
✅ @robobun, your commit b20c268eb29ababb7f9d3fc2b41b178cc51b80a3 passed in 🧪 To try this PR locally: bunx bun-pr 31276That installs a local version of the PR into your bun-31276 --bun |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
I didn't find any correctness issues — the counter/limit mechanism is simple and well-tested — but this introduces a new hard limit (65,536) and a new user-visible error path, and per the description bun build will now emit an empty chunk for these inputs instead of hanging, so a maintainer should sign off on the limit value and that behavior tradeoff.
Extended reasoning...
Overview
The PR adds a per-rule-prelude budget for & parent-selector substitutions during CSS-nesting lowering: a u32 counter on Printer (reset in StyleRule::to_css_base, incremented in serialize_nesting), a new PrinterErrorKind::maximum_nesting_expansion, and a 65,536-substitution cap. A new test file covers the runaway case erroring out, the no-targets case still minifying, ordinary 8-level nesting still expanding fully, and bun build terminating. The only finding from the bug-hunting pass is a non-blocking nit about test.concurrent.
Security risks
This is a resource-exhaustion / DoS fix (fuzzer-found OOM hang). The change strictly reduces attack surface by bounding work; it adds no new parsing, no new inputs, and no unsafe code. I see no injection, auth, or data-exposure concerns.
Level of scrutiny
Medium. The implementation itself is mechanical (increment + compare + early-return) and mirrors the existing ParserError::maximum_nesting_depth precedent. However, it lives in the CSS printer hot path used by bun build, introduces a new hard limit whose value is a judgment call, and intentionally diverges from upstream lightningcss. The PR also notes that because the bundler currently swallows printer errors, the practical effect for bun build users on hostile input changes from "hang/OOM" to "silently empty chunk" — clearly better, but a behavior change a maintainer should be aware of and explicitly accept.
Other factors
The PR description is unusually thorough (root cause, growth measurements, interaction with #31270, full local test-suite results, clippy clean), and the tests include kill-switches so a regression fails rather than hangs CI. No CODEOWNERS entry covers src/css/. CI is still building. Given the new limit/error and the bun build empty-chunk implication, I am deferring rather than approving so a human can confirm the limit value and the error-surfacing follow-up plan.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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-expansion.test.ts`:
- Around line 78-90: The two spawn-based tests calling runMinifyTest
(destructuring { stdout, stderr, signalCode } — add exitCode to that
destructure) need an explicit assertion that the process exited successfully;
update both tests (the one expecting "ERR Maximum nesting expansion exceeded"
and the "deeply nested `&` selectors still minify..." test) to destructure
exitCode and add expect(exitCode).toBe(0) as the final assertion in each test so
the exit code is asserted last for clearer failures.
🪄 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: d0e1e144-b163-4b9b-a46b-9973b19541d0
📒 Files selected for processing (5)
src/css/error.rssrc/css/printer.rssrc/css/rules/style.rssrc/css/selectors/selector.rstest/js/bun/css/nested-selector-expansion.test.ts
|
CI status on b20c268 (build 57299): everything that ran is green — 73/73 completed checks pass, including all build lanes, the Linux/Alpine/Debian test lanes, and the x64 ASAN lane that exercises the new |
…argets (#31277) ### What does this PR do? Fixes an OOM found by CSS fuzzing (signature `oom:css:__rust_alloc|alloc::vec::spec_from_iter_nested…`): a 924-byte stylesheet of 23 unclosed nested rules, each with a two-selector list, ```css co :is(.bar), .bar :is(.baz) { co :is(.bar), .bar :is(.baz) { … ×23 … color: red; } ``` makes the CSS minifier allocate 4+ GB. The published repro calls `minifyTest(input, "")` with **no targets** and stays linear (534 bytes of output) — the blowup needs browser targets that force `:is()`/nesting to be compiled away, which is what the real entry points use: ```sh # default --target=browser targets are edge88/firefox78/chrome87/safari14 (no :is(), no native nesting) bun build input.css --outdir out --minify # 18 levels → 24 MB output; 23 levels → multi-GB RSS ``` or `minifyTest(input, "", { chrome: 80 << 16 })`. The `::part()` OOM from the same fuzzing campaign is the identical path (`::part()` can never be wrapped in `:is()`). ### Cause Two multiplicative behaviors combine in `minify_style_arm` (`src/css/rules/mod.rs`) / `StyleRule::minify` when nesting has to be compiled for the targets: 1. Selectors the targets don't support can't stay in the same rule (one unsupported selector would make browsers drop the whole list), so each incompatible selector is split into its own rule — **including a `deep_clone` of the entire, already-expanded nested-rule subtree** (`rules: sty.rules.deep_clone(...)`). That clone's `Vec::from_iter` is the allocation stack in the fuzz signature. 2. Every nesting level's selector list multiplies into the level below. With two incompatible selectors at each of 23 levels the minified tree holds ~2²³ style-rule structures before anything is printed → multi-GB RSS from a sub-KB input. Upstream lightningcss behaves the same way (inherited, like the backtracking issue in #31243), so there is no upstream fix to port. ### Fix `MinifyContext` now tracks the expansion: * `selector_expansion_multiplier` — product of the enclosing style rules' selector-list lengths, bumped only when the targets force nesting to be compiled (or the rule's selectors to be split for compatibility), * `selector_expansion_total` — running total of selectors that expansion will produce. Past `MAX_SELECTOR_EXPANSION` (65,536) minify stops with a new `MinifyErrorKind::selector_expansion_limit_exceeded` error instead of materializing the explosion: ``` error: Nested CSS rules expand to more than 65536 selectors when compiled for the configured browser targets. Reduce the nesting depth or the number of selectors per rule, or target browsers that support CSS nesting. ``` The check runs before descending into nested rules, so nothing exponential is allocated on the error path. Real-world stylesheets don't get anywhere near the limit (65,536 expanded selectors is already megabytes of output); stylesheets below it, stylesheets minified without browser targets, and targets that support native nesting are unaffected. Two pieces of plumbing so the error actually reaches users: * `StyleSheet::minify` previously hit `panic!("TODO: Handle")` when rule minification failed (the path was unreachable until now). It now returns the recorded `MinifyError` with its source location; the bundler (`ParseTask`) reports it as a build error and `cssInternals.minifyTest` throws it. * `css_jsc/error_jsc.rs::to_error_instance` deref'd the message string after calling `bun_string_jsc::to_error_instance`, which already consumes the caller's reference. The over-deref freed the `WTFStringImpl` while the JS error still referenced it and crashed debug builds (libpas "Alloc bit not set" / ASAN) the first time a CSS minify error was actually thrown. ### Relationship to other open fuzz fixes Same fuzzing campaign, different mechanisms, complementary fixes: * #31276 caps the **printer-side** `&` parent-selector substitution fan-out (a single selector with multiple `&` references per level — no selector-list splitting involved). It doesn't bound this report, because here the OOM happens in minify's `deep_clone` before printing starts; and this PR doesn't bound that one, because a single selector per level keeps the multiplier at 1. * #31270 removes duplicate re-serialization across vendor-prefix passes. Overlap is only textual (adjacent code in `style.rs`/`error.rs`; the test file here is named `nested-selector-list-expansion.test.ts` to avoid colliding with #31276's). ### Verification New `test/js/bun/css/nested-selector-list-expansion.test.ts`: * 17 levels of `co :is(.bar), .bar :is(.baz)` with `{ chrome: 80 << 16 }` now throws the limit error (previously ~2¹⁸ cloned rules), * same for the `::part()` shape, * `bun build --minify` with default targets reports the error and exits 1 (previously 12+ MB of output at this depth, OOM at the fuzzer's depth), * below-limit nesting still compiles for old targets, the original fuzzer repro with no targets still minifies to the same small output, and deep nesting is preserved untouched for targets that support CSS nesting. Without the fix 3/6 tests fail; with it 6/6 pass. Existing suites on the fixed debug (ASAN) build: `css.test.ts` + `nested-function-backtracking.test.ts` (1054 pass / 0 fail), `doesnt_crash.test.ts` (60 pass), `color.test.ts` + `small-list-grow.test.ts` (917 pass; the only failure is the pre-existing `fuzz ansi256` debug-ASAN timeout also noted in #31243), `test/bundler/css/` (166 pass).
2 participants
What does this PR do?
Fixes a hang with unbounded memory growth in the CSS printer, found by CSS fuzzing (signature
hang:css:…alloc::raw_vec::RawVecInner::finish_grow…— the printer keeps reallocating an ever-growing output buffer; the follow-up OOM reports are the same mechanism hitting the allocator limit).The fuzzer's ~2.8 KB input is ~21 nesting levels of
The published repro calls
minifyTest(input, "")with no targets and does not hang (nesting is preserved, 1.5 KB output). The same input hangs as soon as CSS nesting is compiled away, which is what the real entry points do:or
minifyTest(input, "", { safari: 13 << 16 }). Upstream lightningcss 1.32.0 hangs on the same input with the same targets, so there is no upstream fix to port.Cause
When targets lack nesting support,
serialize_nestingreplaces each&with the parent selector (StyleContextchain). The parent's selector may itself contain&referring to the grandparent, so a selector with k&references per level expands to ~k^depth copies of its ancestors. With this input the printed output grows ~4× per nesting level: depth 6 → 1 MB, depth 8 → 17 MB, depth 10 → 274 MB, depth 21 → effectively unbounded. The work and the output are inherently exponential — the process isn't stuck, it's printing a stylesheet that would be terabytes.Fix
Printernow tracks the number of parent-selector substitutions performed for the current rule prelude (reset inStyleRule::to_css_base, counted inserialize_nesting). Past 65,536 substitutions for a single prelude the printer reports a newPrinterErrorKind::maximum_nesting_expansion("Maximum nesting expansion exceeded when compiling CSS nesting for the configured targets") instead of allocating without bound.Real-world nesting needs at most a handful of substitutions per rule (depth ×
&-per-level), so the budget is far beyond anything legitimate; only runaway expansions hit it. Behavior is unchanged for:&:hoverstill expands fully),With the budget,
bun buildon the hostile input terminates in milliseconds-to-seconds instead of hanging. Note the bundler currently maps any CSS printer error to an empty chunk rather than a build diagnostic (pre-existing, same as the Zig implementation); surfacing printer errors as build errors is a separate follow-up.Related: #31270 fixes a different exponential blowup from the same fuzzing campaign (duplicate re-serialization across vendor-prefix passes). The two mechanisms are independent — that fix does not bound this input (no prefixed selectors here), and this budget resets per prelude so it does not bound that one — the fixes are complementary and touch adjacent code.
Verification
test/js/bun/css/nested-selector-expansion.test.ts:&:is(.bar, &.baz)with targets now errors with "Maximum nesting expansion exceeded" instead of hanging (spawned with a 20 s kill switch so a regression fails instead of hanging the runner),bun buildwith the default browser target terminates on its own.test/js/bun/css/css.test.ts(1049 pass),test/js/bun/css/nested-function-backtracking / small-list-grow / doesnt_crash / css-modules (all pass),test/bundler/css/(166 pass). The only failure seen locally is the pre-existingfuzz ansi25616.7M-iteration timeout on debug ASAN builds (color integer math, unrelated to this change).cargo clippy -p bun_cssis clean.