css: bound nested-prelude substitution output across the stylesheet - #32451
css: bound nested-prelude substitution output across the stylesheet#32451robobun wants to merge 6 commits into
Conversation
A selector like `& > &` is a single entry in its rule's selector list, so the minify-time selector-expansion multiplier (which multiplies by the list length) does not see it as fan-out. At print time every `&` expands the parent, so each nesting level still fans out by its `&` count. The per-prelude substitution counter (MAX_NESTING_EXPANSIONS) bounds one rule's prelude but is reset between sibling rules, so many sibling leaf rules under a `& > &` chain each stay under the per-prelude cap while the total output grows by (leaf count) x 2^depth. A ~7 KB stylesheet expanded into ~1.7 GB of output this way (found by CSS fuzzing, stack sampled inside write_fmt on the growing Vec<u8>). Measure the bytes each nested rule's prelude emits while compiling nesting away and accumulate them across the whole stylesheet. Past 64 MB (matching MAX_PREFIX_EXPANSION_BYTES), report the existing maximum_nesting_expansion error instead of serializing without bound. Only preludes with a parent context are metered, so large flat stylesheets are unaffected.
|
Updated 11:17 PM PT - Jun 16th, 2026
❌ @robobun, your commit f6f7665 has 3 failures in
🧪 To try this PR locally: bunx bun-pr 32451That installs a local version of the PR into your bun-32451 --bun |
|
This PR may be a duplicate of:
🤖 Generated with Claude Code |
|
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 (2)
WalkthroughAdds a stylesheet-wide byte budget ( ChangesCSS Nesting Expansion Byte Budget
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![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 `@src/css/rules/scope.rs`:
- Around line 33-38: The byte metering gate at the location of
prelude_bytes_before only activates when dest.ctx is already set, but the
scope-end expansion can create a temporary context at lines 59-66 that bypasses
this gate. Move the byte metering initialization to occur before the temporary
context creation from scope_start so that all expansion paths, including
top-level `@scope` rules, are properly accounted against the
nesting_expansion_bytes budget. Additionally, add a regression test case that
verifies top-level `@scope` rules with repeated & fan-out patterns respect the
nesting_expansion_bytes limit and do not allow runaway output growth.
🪄 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: da8406aa-ce37-424d-a51d-4c91e6cff465
📒 Files selected for processing (4)
src/css/printer.rssrc/css/rules/scope.rssrc/css/rules/style.rstest/js/bun/css/nested-selector-expansion.test.ts
…pe-start> <scope-end> is serialized with <scope-start> as a temporary parent context even when there is no outer style-rule context, so top-level @scope (huge) to (& & ... &) rules fan out each & in <scope-end> into a copy of <scope-start>. Gate the byte metering on that path too so many such sibling rules are charged against the stylesheet-wide nesting-expansion budget.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Additional findings (outside current diff — PR may have been updated during review):
-
🔴
src/css/rules/scope.rs:74-83— The new metering block at lines 74–83 is placed after the pre-existing earlyreturn serialize_selector_list(...)at line 69, so whenscope_endisSomeandscope_startisNone(i.e.@scope to (...) { }) the prelude's&-substitution bytes are emitted but never charged tonesting_expansion_bytes. Many sibling@scope to (& ...) {}rules nested under a deep& > &chain therefore reproduce the same unbounded blow-up this PR fixes forStyleRule. The metering should run before this early return (or the early return should be dropped so control falls through — it already looks like a pre-existing bug since it also skips),{, the body, and}).Extended reasoning...
What the bug is
ScopeRule::to_csscapturesprelude_bytes_beforeat line 38 (whendest.ctxisSome) and then, after both scope preludes have been serialized, accumulates the emitted bytes intodest.nesting_expansion_bytesand checks them againstMAX_NESTING_EXPANSION_BYTESat lines 74–83. But between those two points there is a pre-existing early return at line 69:} else { let ctx = dest.ctx; return serialize_selector_list(scope_end.v.slice(), dest, ctx, false); }
This branch is taken when
scope_endisSomeandscope_startisNone— i.e. for@scope to (<scope-end>) { ... }with no<scope-start>. Thereturnexitsto_cssbefore reaching the new metering block, soprelude_bytes_beforeis captured but never used: the bytesserialize_selector_listwrites are not added tonesting_expansion_bytesand never compared against the 64 MB budget.Why it is reachable
The parser (
css_parser.rs) parsesscope_startandscope_endindependently, so@scope to (...)without a scope-start is accepted and producesscope_start = None, scope_end = Some(...). When such an@scoperule is nested inside a style rule and nesting is being compiled away,StyleRule::to_css_baseprints its nested rules viadest.with_context(&self.selectors, ...), sodest.ctxisSomewhenScopeRule::to_cssruns. That makesprelude_bytes_beforeSomeat line 38, and line 69 passesdest.ctxasctx, so&in the scope-end selector is substituted with the full parent chain — exactly the expansion the PR is metering. Line 32 also resetsdest.nesting_expansions = 0, so each sibling@scopeprelude gets a fresh per-prelude substitution budget.Why existing guards don't catch it
The per-prelude
MAX_NESTING_EXPANSIONScap (65 536) is reset at line 32 for each@scoperule, and the minify-timeselector_expansion_multiplierdoesn't see& > &as fan-out (it's one selector in its list) — these are precisely the gaps the PR's new stylesheet-wide byte budget is meant to close. The PR description says "The same metering is applied around@scope's preludes", but this code path is the exception.Step-by-step proof
- Root:
.aaa…::part(x), .bbb…::part(y)(~1 KB). - Wrap 13 times in
& > & { … }(so each leaf prelude's&does 2^13 = 8192 substitutions — under the 65 536 per-prelude cap). - Innermost body: 200 sibling
@scope to (& .leaf-i) {}rules. - Compile with a target that lacks native nesting (e.g.
{ firefox: 100 << 16 }).
For each sibling
@scope:dest.ctxisSome(set by the enclosingStyleRule::to_css_base→with_context), soprelude_bytes_before = Some(n)at line 38.scope_startisNone,scope_endisSome, so control reaches theelseat line 67.- Line 69 calls
serialize_selector_list(scope_end, dest, dest.ctx, false):& .leaf-iexpands the 13-deep& > &chain into ~8192 copies of the ~1 KB root, emitting ~8 MB. - The function then returns. Lines 74–83 never execute;
nesting_expansion_bytesis unchanged.
200 siblings × ~8 MB ≈ ~1.6 GB of output with the new byte budget never checked on this path — the same DoS shape the PR fixes for
StyleRule, reachable via a trivially different leaf rule.Impact
This is an incomplete fix for the DoS the PR targets: the same fuzzer that found the
StyleRulevariant could find this one by swapping.lN { … }leaves for@scope to (& .lN) {}leaves. The unbounded output is written byserialize_selector_listbefore the early return, so the (separate, pre-existing) fact that the early return also skips)/{/body/}does not mitigate it.How to fix
Either move/duplicate the metering check so it also runs on the line-69 path (e.g. compute and check
nesting_expansion_bytesimmediately before thereturn), or — probably better — drop thereturnso control falls through to the existing metering block and the rest of the rule body. The early return itself looks like a pre-existing bug (it leaves the prelude unclosed and never prints the rule body), so removing it would fix both issues at once. - Root:
ScopeRule::to_css early-returned after serializing <scope-end> when <scope-start> was absent, so @scope to (...) { } emitted an unclosed prelude with no body. The early return also bypassed the nesting-expansion byte budget check, letting many sibling @scope to (& .lN) {} rules under a `& > &` chain fan out each & in <scope-end> against the outer parent context without bound (the same ~1.7 GB blow-up this PR fixes for style-rule leaves). Drop the early return so control falls through to close the prelude, charge its bytes against the budget, and emit the rule body.
|
CI on build 63092 is red on one lane (
None of these touch The diff is green; these are unrelated infra/external-service flakes on the darwin 26 aarch64 runner. |
1 participant
What does this PR do?
Fixes a CSS serializer hang found by fuzzing: a ~7 KB stylesheet expands into ~1.7 GB of output (scales to an unbounded hang with more sibling rules) when compiling nesting away for older targets.
Fuzzer signature:
hang:css:_RNvXNvNtNtNtC8bun_core4util2io5Write9write_fmtINtB2_6BridgeINtNtC5alloc3vec3VechEENtNtC4core3fmt5Write9write_s(i.e.<Write::write_fmt::Bridge<Vec<u8>> as fmt::Write>::write_str: the printer spinning while growing its output buffer).Cause
A selector like
& > &is a single entry in its rule's selector list, so the minify-timeselector_expansion_multiplier(which multiplies by the list length, #31277) does not see it as fan-out and stays at 1. At print time every&expands the parent, so each such nesting level still fans out by the number of&references it holds.The per-prelude substitution counter$2^{13} = 8192$ substitutions (under the per-prelude cap); 200 sibling leaves then emit $200 \times 8192$ copies of the root selector list with none of the existing caps firing. The total output grows by (leaf count) x (product of per-level
MAX_NESTING_EXPANSIONS(#31276) bounds one rule's prelude to 65,536 substitutions, but it is reset between sibling rules inStyleRule::to_css_base. With a depth-13& > &chain, each leaf rule's prelude expands to&counts); at ~15 KB of input this was ~2 GB of output.Fix
Measure the bytes each nested rule's prelude emits while compiling nesting away and accumulate them into a new stylesheet-wide
Printer::nesting_expansion_bytes(never reset, likeprefix_expansion_bytesfrom #31642). PastMAX_NESTING_EXPANSION_BYTES(64 MB, matchingMAX_PREFIX_EXPANSION_BYTES), report the existingmaximum_nesting_expansionerror instead of serializing without bound. Only preludes with a parent context are metered, so large flat stylesheets are unaffected and output for non-pathological input is byte-for-byte unchanged.The same metering is applied around
@scope's preludes, which serialize with the parent context the same way.Related: #31916 adds a byte budget on both the minify side (estimated selector weight) and the printer side (measured per-substitution) for a different trigger (fat selectors x moderate count). This PR is the narrower printer-side fix for the many-siblings trigger; it measures per-prelude rather than per-substitution, so it needs no recursion-depth tracking.
Verification
New tests in
test/js/bun/css/nested-selector-expansion.test.ts:many sibling rules under a \& > &` chain error out instead of serializing gigabytesfails on the unfixed build (outputsOK 1691652252`) and passes on the fixed build (throws after ~64 MB).a few sibling rules under a \& > &` chain still serialize without hitting the byte budget` confirms the same shape at a non-pathological scale (3 leaves, ~25 MB) is unchanged.The existing nesting-expansion, selector-list-expansion, vendor-prefix-duplication, and
css.test.tssuites all pass unchanged.