Skip to content
1 change: 1 addition & 0 deletions src/css/css_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2595,6 +2595,7 @@ mod stylesheet_impl {
err: None,
selector_expansion_multiplier: 1,
selector_expansion_total: 0,
token_expansion_total: 0,
};

if self.rules.minify(&mut minify_ctx, false).is_err() {
Expand Down
19 changes: 19 additions & 0 deletions src/css/declaration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,25 @@ impl<'bump> DeclarationBlock<'bump> {
self.declarations.len() + self.important_declarations.len()
}

/// Recursive `TokenOrValue` count across every unparsed / custom
/// property in this block. Other property kinds are fixed-size values
/// whose clone cost is already bounded by the selector-expansion cap.
/// See [`css_rules::MAX_TOKEN_EXPANSION`](crate::css_rules::MAX_TOKEN_EXPANSION).
pub fn token_weight(&self) -> usize {
fn one(p: &css::Property) -> usize {
match p {
css::Property::Unparsed(u) => u.value.token_weight(),
css::Property::Custom(c) => c.value.token_weight(),
_ => 0,
}
}
Comment thread
robobun marked this conversation as resolved.
self.declarations
.iter()
.chain(self.important_declarations.iter())
.map(one)
.sum()
}

pub fn new_in(bump: &'bump Bump) -> Self {
Self {
important_declarations: DeclarationList::new_in(bump),
Expand Down
9 changes: 9 additions & 0 deletions src/css/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,10 @@ pub enum MinifyErrorKind {
/// Compiling nested rules for the configured browser targets would expand to
/// more than [`crate::css_rules::MAX_SELECTOR_EXPANSION`] selectors.
selector_expansion_limit_exceeded,
/// Compiling nested rules for the configured browser targets would clone
/// unparsed property values totalling more than
/// [`crate::css_rules::MAX_TOKEN_EXPANSION`] raw tokens.
token_expansion_limit_exceeded,
/// Rule minification failed without recording a more specific diagnostic on
/// `MinifyContext::err`. Defensive fallback — every failing path is expected
/// to record one before returning an error.
Expand All @@ -540,6 +544,11 @@ impl fmt::Display for MinifyErrorKind {
"Nested CSS rules expand to more than {} 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.",
crate::css_rules::MAX_SELECTOR_EXPANSION,
),
Self::token_expansion_limit_exceeded => write!(
f,
"Nested CSS rules expand to more than {} raw property tokens when compiled for the configured browser targets. Reduce the nesting depth, the number of selectors per rule, or the size of unparsed property values, or target browsers that support CSS nesting.",
crate::css_rules::MAX_TOKEN_EXPANSION,
),
Self::unknown => write!(f, "CSS minification failed"),
}
}
Expand Down
34 changes: 34 additions & 0 deletions src/css/properties/custom.rs
Original file line number Diff line number Diff line change
Expand Up @@ -815,6 +815,40 @@
res
}

/// Number of `TokenOrValue` nodes in this list, counting through nested
/// `Function`/`var()`/`env()`/`light-dark()` etc. so the result reflects
/// the allocation a `deep_clone` of this list performs. Used by the
/// minify-time token-expansion budget (see
/// [`css_rules::MAX_TOKEN_EXPANSION`](crate::css_rules::MAX_TOKEN_EXPANSION)).
pub fn token_weight(&self) -> usize {
let mut n = self.v.len();
for t in self.v.iter() {
match t {
TokenOrValue::Function(f) => n += f.arguments.token_weight(),
TokenOrValue::Var(v) => {
if let Some(fallback) = &v.fallback {
n += fallback.token_weight();
}
}
TokenOrValue::Env(e) => {
if let Some(fallback) = &e.fallback {
n += fallback.token_weight();
}
}

Check warning on line 837 in src/css/properties/custom.rs

View check run for this annotation

Claude / Claude Code Review

token_weight() doesn't count EnvironmentVariable::indices

The `Env` arm only counts `e.fallback` but not `e.indices`, even though `EnvironmentVariable::indices: Vec<CSSInteger>` is parsed by an unbounded loop and heap-allocated by every `deep_clone`. `env(x 1 1 ... 1)` with 60K integers therefore reports `token_weight() == 1`, so at depth 14 (selector total ≈49K < 65,536; token charge = 16,384 < 1M) the same selector-split path produces ~8K live clones × ~240 KB `Vec<i32>` ≈ ~2 GB before either cap fires. One-line fix in this match arm: `n += e.indices
Comment thread
robobun marked this conversation as resolved.
TokenOrValue::UnresolvedColor(c) => match c {
UnresolvedColor::RGB { alpha, .. } | UnresolvedColor::HSL { alpha, .. } => {
n += alpha.token_weight();
}
UnresolvedColor::LightDark { light, dark } => {
n += light.token_weight() + dark.token_weight();
}
},
_ => {}
}
}
n
}

pub fn get_necessary_fallbacks(&self, targets: &css::targets::Targets) -> ColorFallbackKind {
let mut fallbacks = ColorFallbackKind::empty();
for token_or_value in self.v.iter() {
Expand Down
18 changes: 18 additions & 0 deletions src/css/rules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1281,6 +1281,21 @@ pub struct StyleContext<'a> {
/// instead.
pub const MAX_SELECTOR_EXPANSION: u32 = 65_536;

/// Upper bound on the number of raw `TokenOrValue` nodes that compiling
/// nested rules for the configured targets may clone across a stylesheet.
///
/// Companion to [`MAX_SELECTOR_EXPANSION`]: that cap counts expanded rules,
/// this one counts the raw-token payload those rules carry. A rule split
/// for an incompatible selector deep-clones its declarations (and the
/// already-expanded nested tree), so a large unparsed property value under
/// a handful of split levels is duplicated once per expanded rule. The
/// selector cap alone permits up to 65,536 copies, which for a
/// multi-thousand-token value is gigabytes of `TokenOrValue` allocations
/// long before that cap is reached. One million tokens is on the order of
/// 100 MB of in-memory `TokenOrValue` and a few MB of printed output;
/// real stylesheets stay far below it.
pub const MAX_TOKEN_EXPANSION: usize = 1 << 20;

/// Per-stylesheet minification state threaded through `CssRuleList::minify`
/// and every leaf rule's `minify`.
///
Expand Down Expand Up @@ -1316,4 +1331,7 @@ pub struct MinifyContext<'a, 'bump> {
/// Running total of selectors that compiling nested rules for the targets
/// will expand to, checked against [`MAX_SELECTOR_EXPANSION`].
pub selector_expansion_total: u32,
/// Running total of raw `TokenOrValue` nodes that compiling nested rules
/// for the targets will clone, checked against [`MAX_TOKEN_EXPANSION`].
pub token_expansion_total: usize,
}
27 changes: 22 additions & 5 deletions src/css/rules/style.rs
Original file line number Diff line number Diff line change
Expand Up @@ -390,18 +390,35 @@
context: &mut MinifyContext<'_, '_>,
) -> Result<(), MinifyErr> {
if context.selector_expansion_multiplier > 1 {
Comment thread
robobun marked this conversation as resolved.
context.selector_expansion_total = context.selector_expansion_total.saturating_add(
context
.selector_expansion_multiplier
.saturating_mul(self.selectors.v.len().max(1)),
);
let copies = context
.selector_expansion_multiplier
.saturating_mul(self.selectors.v.len().max(1));
context.selector_expansion_total =
context.selector_expansion_total.saturating_add(copies);
if context.selector_expansion_total > super::MAX_SELECTOR_EXPANSION {
context.err = Some(crate::error::MinifyError {
kind: crate::error::MinifyErrorKind::selector_expansion_limit_exceeded,
loc: self.loc,
});
return Err(MinifyErr::minify_err);
}
// Same expansion multiplies this rule's unparsed/custom property
// token lists. A large raw value under the selector cap still
// deep-clones into gigabytes of `TokenOrValue`, so budget the
// token payload separately.
let weight = self.declarations.token_weight();
if weight > 0 {
context.token_expansion_total = context
.token_expansion_total

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.

🟡 Beyond CssRule::Unknown (above), self.declarations.token_weight() also misses two more TokenList carriers in the same sty.rules.deep_clone() subtree that the Unknown-specific fix wouldn't cover: (1) @container style(--foo: x x …)StyleQuery::Feature(Box<Property::Custom>) is deep-cloned via ContainerRule::deep_clone but the Container arm of CssRuleList::minify only recurses into cont.rules and never charges the condition (likewise MediaFeatureValue::Env fallbacks in @media/@container size queries); (2) nested rules' selectorsPseudoElement::CustomFunction { arguments: TokenList } / PseudoClass::CustomFunction (e.g. .inner::-foo(x x …)), cloned via self.selectors.deep_clone(). Both reproduce the same depth-12 ~0.9 GB OOM with token_expansion_total == 0. Rather than patching each carrier, a recursive StyleRule::token_weight() (declarations + selector CustomFunction args + nested at-rule preludes/blocks/conditions) charged once at the split level would close all of these — including the Unknown gap — together.

Extended reasoning...

What this adds to the comment above

The 🔴 comment above establishes that charge_selector_expansion only weighs self.declarations.token_weight() and so misses TokenLists carried elsewhere in the cloned subtree, citing CssRule::Unknown as the example. Both fixes that comment proposes are scoped to CssRule::Unknown specifically (charge prelude + block in the _ => {} arm of CssRuleList::minify, or walk self.rules for Unknown children). This comment names two further carriers that an author implementing either of those fixes literally would still leave open — same multiplier, same deep_clone sink, same crash class, but different containers in different code paths — so the fuzzer would surface them next under the same signature.

Carrier 1: @container style() conditions

@container is in allowed_in_style_rule() (css_parser.rs:1041), so it nests inside style rules. @container style(--foo: x x x …) parses via ContainerCondition::parse (with ALLOW_STYLE, container.rs:322) → parse_style_queryStyleQuery::parse_feature (container.rs:147–156) → Property::parse on PropertyId::Custom(--foo)CustomProperty::parseTokenList::parse, producing a StyleQuery::Feature(Box<Property::Custom>) whose value is an arbitrarily large TokenList. ContainerRule::deep_clone (container.rs:364–374) deep-clones conditionStyleQuery::deep_clone (container.rs:180–194) → dc::propertyCustomProperty::deep_cloneTokenList::deep_clone. The Container arm of CssRuleList::minify (rules/mod.rs) only does cont.rules.minify(...) and never charges the condition; every enclosing style rule has empty declarations (only a nested @container), so token_expansion_total stays at 0. The same applies to MediaFeatureValue::Env(EnvironmentVariable) fallbacks reachable via @media (width: env(--x, <large fallback>)) and @container (width: env(--x, …)) size queries — QueryFeature::deep_clone deep-clones the fallback TokenList and nothing charges it.

This is not covered by the Unknown fix: @container has its own CssRuleList::minify arm (Container(cont)), not the catch-all _ => {}, and "walk self.rules for Unknown children" doesn't match a Container child.

Carrier 2: selector CustomFunction arguments

PseudoElement::CustomFunction { arguments: TokenList } and PseudoClass::CustomFunction { arguments: TokenList } (parser.rs:1040–1045, 3150–3155) are parsed via TokenList::parse_raw (parser.rs:1312–1317, 1406–1410), so .inner::-foo(x x …) produces a multi-thousand-entry Vec<TokenOrValue> on the inner rule's selector. The partition path's sty.rules.deep_clone(context.arena) reaches StyleRule::deep_cloneself.selectors.deep_clone() (style.rs) → per-component deep_clone()PseudoElement::deep_clone() = self.clone() (parser.rs:3177–3179) → TokenList::clone()Vec<TokenOrValue>::clone(), allocating a fresh vec per clone. The leaf rule's declarations (color: red) parse as a typed Property::Color, so self.declarations.token_weight() == 0 and token_expansion_total again stays at 0.

This is not covered by either Unknown fix or any "walk self.rules for at-rule TokenLists" fix: the carrier lives in the selector tree (StyleRule.selectors), not in StyleRule.rules.

Step-by-step proof (concrete numbers, both carriers)

Same skeleton as the PR's repro — only the payload location changes:

const payload = "x ".repeat(3000);   // ~6,000 raw tokens

// Carrier 1: @container style() condition
const css1 = "x::part(a), y::part(b) {\n".repeat(12)
           + "@container style(--foo: " + payload + ") { .inner { color: red } }";

// Carrier 2: selector CustomFunction
const css2 = "x::part(a), y::part(b) {\n".repeat(12)
           + ".inner::-foo(" + payload + ") { color: red }";

cssInternals._test(css1 /* or css2 */, "", { chrome: 80 << 16 });
  • Selector cap. At depth 12 with two-selector lists, selector_expansion_total ≈ 8K–12K, far under 65,536.
  • Token cap. Every style rule on the path has declarations.token_weight() == 0 (empty blocks for the 12 wrapper levels; parsed color: red for .inner), so token_expansion_total stays at 0.
  • Split-clone. Levels 2..12 carry implicit Component::Nesting → incompatible with chrome 80; ::part() is a pseudo-element so no :is() collapse. minify_style_arm partitions both selectors at every level, executing sty.rules.deep_clone(context.arena) per partition, doubling the leaf subtree at each level → ~2^11 ≈ 2,048 clones bottom-up.
  • Allocation. 2,048 × ~6,000 tokens × 72 B/TokenOrValue0.88 GB of Vec<TokenOrValue> allocations before either cap fires — same magnitude and crash class this PR targets. (Carrier 2 goes through Vec::clone() rather than deep_clone_with(), but the allocation is identical.)

Suggested fix

Rather than patching three more arms individually (the catch-all for Unknown, the Container/Media arms for conditions, and a separate selector walk), replace the per-rule declaration-only weight with a recursive StyleRule::token_weight() that sums self.declarations.token_weight() + Σ arguments.token_weight() over each Component::PseudoElement(CustomFunction{..}) / Component::NonTsPseudoClass(CustomFunction{..}) in self.selectors + a walk over self.rules collecting TokenList weights from Unknown.{prelude,block}, Container.condition, Media.query env fallbacks, and recursing into nested style rules. Charged once at the outermost split level (where copies is computed), this closes all four carriers under the new cap's documented contract ("raw TokenOrValue nodes that compiling nested rules … may clone") in one place.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified all three carriers (@container style(--foo: ...), ::-foo(...), :-foo(...)) reproduce at D=12. These, along with @media env fallbacks and selector-list arguments, are exactly what #31913's clone_weight module walks: it recursively weighs Container.condition via style_query -> property -> token_list, selector Component::PseudoElement(CustomFunction) / PseudoClass(CustomFunction) arguments, media/supports conditions, and the full nested rule_list, charging the total against MAX_SELECTOR_SPLIT_CLONE_WEIGHT in the while incompatible.len() > 0 loop before the clone. Reimplementing that walk here at the charge_selector_expansion layer would duplicate ~200 lines of #31913 and conflict on merge.

This PR's scope is the nesting-multiplier vector the fuzzer reported (Vec<TokenOrValue>::deep_clone_with under declarations + the immediately adjacent Unknown and env() carriers surfaced in this review round); #31913 is listed under Related as the complementary cap that bounds every split-clone payload carrier. Leaving this open for a maintainer to weigh in on scope; happy to fold a comprehensive walk in if preferred over landing #31913.

.saturating_add((copies as usize).saturating_mul(weight));
if context.token_expansion_total > super::MAX_TOKEN_EXPANSION {
context.err = Some(crate::error::MinifyError {
kind: crate::error::MinifyErrorKind::token_expansion_limit_exceeded,
loc: self.loc,
});
return Err(MinifyErr::minify_err);
}
}

Check failure on line 421 in src/css/rules/style.rs

View check run for this annotation

Claude / Claude Code Review

Token-expansion cap bypassed by nested unknown at-rule body

The new token cap only weighs `self.declarations.token_weight()`, so it misses the raw `TokenList`s carried by nested `CssRule::Unknown` at-rules — which the same partition path deep-clones via `sty.rules.deep_clone()`. Moving the PR's repro payload from `color: f(x x …)` into a nested `@foo { x x … }` body reproduces the same `Vec<TokenOrValue>::deep_clone` OOM at depth 12 (~0.9 GB) while `token_expansion_total` stays at 0. Charging `multiplier × (prelude + block).token_weight()` for `CssRule::
Comment thread
robobun marked this conversation as resolved.
Outdated
}
Ok(())
}
Expand Down
129 changes: 128 additions & 1 deletion test/js/bun/css/nested-selector-list-expansion.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,10 @@ import { bunEnv, bunExe, tempDir } from "harness";
// plain `bun build` since the default bundler targets predate `:is()` and
// native nesting. The minifier now bounds the expansion and reports an error.

const { minifyTest, prefixTest } = cssInternals;
const { minifyTest, prefixTest, _test } = cssInternals;

const LIMIT_ERROR = "Nested CSS rules expand to more than";
const TOKEN_LIMIT_ERROR = "raw property tokens when compiled for the configured browser targets";

// `outer` plain-nested two-selector rules, then `atRule`'s block, then `inner`
// more nested two-selector rules. The blocks are left unclosed (the CSS parser
Expand Down Expand Up @@ -263,3 +264,129 @@ test("bun build does not hang on deeply nested multi-selector css spanning @star
expect(exitCode).toBe(1);
expect(await Bun.file(`${dir}/out/input.css`).exists()).toBe(false);
});

// Regression test for unbounded token-list cloning when compiling CSS nesting
// for browser targets that don't support it.
//
// The selector-expansion cap counts the number of rules the expansion
// produces, but each split rule also deep-clones its declarations. An
// unparsed property value (any value the property-specific parser couldn't
// read) is stored as a raw TokenList and copied in full for every clone, so
// a few thousand tokens under ten ::part()-selector nesting levels expanded
// into gigabytes of in-memory tokens while the selector count stayed well
// under its 65,536 cap. Found by fuzzing. The minifier now bounds the total
// cloned-token count and reports an error.

/** `depth` nested ::part() rules with a large unparsed `color:` value at the
* bottom. `::part()` is a pseudo-element so the selector list can never be
* collapsed into `:is()`; each level is split into one cloned rule per
* selector, and the clone carries a full copy of the inner value. */
function nestedWithLargeUnparsedValue(depth: number, tokens: number): string {
// `x ` parses to two tokens (ident + whitespace); the unknown function
// `f(...)` around it keeps the whole thing one raw TokenList.
const payload = Buffer.alloc(tokens * 2, "x ").toString();
return (
"x::part(a), y::part(b) {\n".repeat(depth) +
".inner { color: f(" +
payload +
"var(--x)) }\n" +
"}\n".repeat(depth)
);
}

test("nested selector splits with a large unparsed value error instead of exploding (minify)", () => {
// 8 two-selector levels = 256 copies of a ~6000-token value = ~1.5M tokens,
// past the 1M cap. Before the fix this emitted ~800 KB of output (and at
// slightly larger depths allocated gigabytes before the selector cap was
// reached).
const src = nestedWithLargeUnparsedValue(8, 3000);
expect(() => minifyTest(src, "", OLD_TARGETS)).toThrow(TOKEN_LIMIT_ERROR);
});

test("nested selector splits with a large unparsed value error instead of exploding (prefix)", () => {
const src = nestedWithLargeUnparsedValue(8, 3000);
expect(() => prefixTest(src, "", OLD_TARGETS)).toThrow(TOKEN_LIMIT_ERROR);
});

test("nested selector splits with a large unparsed value error instead of exploding (_test)", () => {
// The fuzzer entrypoint.
const src = nestedWithLargeUnparsedValue(8, 3000);
expect(() => _test(src, "", OLD_TARGETS)).toThrow(TOKEN_LIMIT_ERROR);
});

test("nested selector splits with a large unparsed value below the token limit still compile for old targets", () => {
// 6 levels = 64 copies of ~6000 tokens = ~384K tokens, under the 1M cap.
const src = nestedWithLargeUnparsedValue(6, 3000);
const out = minifyTest(src, "", OLD_TARGETS);
expect(out).toContain("var(--x)");
expect(out.length).toBeLessThan(1_000_000);
});

test("unparsed-value output below the token limit is unchanged by the cap", () => {
// Shallow enough that neither cap applies: the cap must not affect what
// valid expansions emit.
const src = nestedWithLargeUnparsedValue(2, 20);
expect(minifyTest(src, "", OLD_TARGETS)).toMatchInlineSnapshot(
`":is(x::part(a),y::part(b)) x::part(a) .inner{color:f(x x x x x x x x x x x x x x x x x x x x var(--x))}:is(x::part(a),y::part(b)) y::part(b) .inner{color:f(x x x x x x x x x x x x x x x x x x x x var(--x))}"`,
);
});

test("large unparsed values are preserved as-is for targets that support CSS nesting", () => {
// No split, no clone: the input passes through with native nesting intact
// regardless of value size.
const src = nestedWithLargeUnparsedValue(12, 3000);
const out = minifyTest(src, "", MODERN_TARGETS);
expect(out).toContain("var(--x)");
expect(out.length).toBeLessThan(20_000);
});

test("large unparsed values are preserved as-is when no targets are configured", () => {
const src = nestedWithLargeUnparsedValue(12, 3000);
const out = minifyTest(src, "");
expect(out).toContain("var(--x)");
expect(out.length).toBeLessThan(20_000);
});

test("token limit still applies when the large unparsed value sits inside a context-preserving at-rule", () => {
// Same `@starting-style` hiding mechanism as the selector-cap tests above:
// the token charge must follow the multiplier through the at-rule.
const payload = Buffer.alloc(6000, "x ").toString();
const src =
"x::part(a), y::part(b) {\n".repeat(4) +
"@starting-style {\n" +
"x::part(a), y::part(b) {\n".repeat(4) +
".inner { color: f(" +
payload +
"var(--x)) }";
expect(() => minifyTest(src, "", OLD_TARGETS)).toThrow(TOKEN_LIMIT_ERROR);
});

test("bun build reports an error instead of OOMing on deeply nested selectors with a large unparsed value", async () => {
using dir = tempDir("css-token-expansion", {
// 12 levels and a ~6000-token value: before the fix this allocated on the
// order of a gigabyte of cloned TokenOrValue before reaching the selector
// cap.
"input.css": nestedWithLargeUnparsedValue(12, 3000),
});
await using proc = Bun.spawn({
cmd: [bunExe(), "build", "input.css", "--outdir", "out", "--minify"],
env: bunEnv,
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
// Kill switch for a regression: before the fix this allocated past the
// container's memory budget, so let the child terminate itself instead of
// hanging the runner.
timeout: 20_000,
killSignal: "SIGKILL",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
// Must terminate on its own (reporting the limit error), not be SIGKILLed
// by the timeout or OOM-killed by the OS.
expect({ signalCode: proc.signalCode, stderr, stdout, exitCode }).toMatchObject({
signalCode: null,
stderr: expect.stringContaining(LIMIT_ERROR),
exitCode: 1,
Comment thread
coderabbitai[bot] marked this conversation as resolved.
});
expect(await Bun.file(`${dir}/out/input.css`).exists()).toBe(false);
});
Loading