Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/css/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,9 @@ pub enum PrinterErrorKind {
invalid_composes_selector,
/// The CSS modules pattern must end with `[local]` for use in CSS grid.
invalid_css_modules_pattern_in_grid,
/// Substituting parent selectors for `&` while compiling CSS nesting for
/// the configured targets exceeded the expansion limit.
maximum_nesting_expansion,
no_import_records,
}

Expand All @@ -255,6 +258,9 @@ impl fmt::Display for PrinterErrorKind {
Self::invalid_css_modules_pattern_in_grid => {
f.write_str("CSS modules pattern must end with '[local]' when used in CSS grid")
}
Self::maximum_nesting_expansion => f.write_str(
"Maximum nesting expansion exceeded when compiling CSS nesting for the configured targets",
),
Self::no_import_records => f.write_str("No import records found"),
}
}
Expand Down
8 changes: 8 additions & 0 deletions src/css/printer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,13 @@ pub struct Printer<'a> {
// TODO(port): lifetime — ctx is set to a stack-local during with_context() and restored
// after; `&'a StyleContext<'a>` will not borrow-check there. May need raw `*const StyleContext`.
pub ctx: Option<&'a css::StyleContext<'a>>,
/// Number of parent-selector substitutions performed for `&` while
/// serializing the current rule prelude with compiled nesting (targets
/// without CSS nesting support). Reset per prelude (in
/// `StyleRule::to_css_base` and `ScopeRule::to_css`) and bounded in
/// `serialize::serialize_nesting` so deeply nested rules with multiple
/// `&` references per level cannot expand exponentially.
pub nesting_expansions: u32,
pub scratchbuf: BumpVec<'a, u8>,
pub error_kind: Option<css::PrinterError>,
pub import_info: Option<ImportInfo<'a>>,
Expand Down Expand Up @@ -310,6 +317,7 @@ impl<'a> Printer<'a> {
in_calc: false,
css_module: None,
ctx: None,
nesting_expansions: 0,
error_kind: None,
}
}
Expand Down
4 changes: 4 additions & 0 deletions src/css/rules/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@

dest.write_str("@scope")?;
dest.whitespace()?;
// The scope preludes get their own budget for `&` substitutions when
// compiling nesting, like style rule preludes do (see
// `serialize::serialize_nesting`).
dest.nesting_expansions = 0;

Check notice on line 32 in src/css/rules/scope.rs

View check run for this annotation

Claude / Claude Code Review

Pre-existing: ScopeRule::to_css early-returns when scope_end is set without scope_start

Pre-existing (not introduced by this PR, flagging only because the PR edits this function): a few lines below the new `nesting_expansions = 0` reset, the else-branch for `scope_end` does `return serialize_selector_list(scope_end.v.slice(), dest, ctx, false);` — the early `return` exits `ScopeRule::to_css` before writing the closing `)`, the `{`, the rule body, and `}`, so `@scope to (.end) { ... }` prints as the truncated `@scope to (.end`. The same bug exists verbatim in the Zig sibling at `src
Comment thread
robobun marked this conversation as resolved.
if let Some(scope_start) = &self.scope_start {
dest.write_char(b'(')?;
// scope_start.to_css(dest)?;
Expand Down
3 changes: 3 additions & 0 deletions src/css/rules/style.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,9 @@ impl<R> StyleRule<R> {
// PORT NOTE: `dest.context()` borrows `dest`; copy the (Copy) raw
// ctx field out so it doesn't conflict with the `&mut *dest` below.
let ctx = dest.ctx;
// Each rule prelude gets its own budget for `&` substitutions when
// compiling nesting (see `serialize::serialize_nesting`).
dest.nesting_expansions = 0;
Comment thread
robobun marked this conversation as resolved.
selector::serialize::serialize_selector_list(
self.selectors.v.slice(),
dest,
Expand Down
21 changes: 21 additions & 0 deletions src/css/selectors/selector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1361,12 +1361,33 @@ pub mod serialize {
Ok(())
}

/// Maximum number of parent-selector substitutions allowed while
/// serializing a single rule prelude with compiled nesting.
///
/// When the targets don't support CSS nesting, every `&` is replaced with
/// the parent selector, which may itself contain `&` referring to the
/// grandparent, and so on. A selector with multiple `&` references per
/// nesting level therefore expands to (references per level)^depth copies
/// of its ancestors, so a few KB of deeply nested input can print
/// gigabytes of output. Real-world nesting needs at most a handful of
/// substitutions per rule; anything past this limit is a runaway
/// expansion, so bail out with an error instead of allocating without
/// bound.
const MAX_NESTING_EXPANSIONS: u32 = 65_536;

pub fn serialize_nesting(
dest: &mut Printer,
context: Option<&StyleContext>,
first: bool,
) -> Result<(), PrintErr> {
if let Some(ctx) = context {
dest.nesting_expansions += 1;
if dest.nesting_expansions > MAX_NESTING_EXPANSIONS {
return dest.new_error(
crate::error::PrinterErrorKind::maximum_nesting_expansion,
None,
);
}
// If there's only one simple selector, just serialize it directly.
// Otherwise, use an :is() pseudo class.
// Type selectors are only allowed at the start of a compound selector,
Expand Down
141 changes: 141 additions & 0 deletions test/js/bun/css/nested-selector-expansion.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
import { expect, test } from "bun:test";
import { bunEnv, bunExe, tempDir } from "harness";
import path from "node:path";

// Regression test for unbounded memory growth when compiling CSS nesting for
// targets that don't support it (found by CSS fuzzing).
//
// When the browser targets lack native nesting support, every `&` in a nested
// rule's selector is replaced with the parent selector at print time. The
// parent selector itself may contain `&` referring to the grandparent, so a
// selector with multiple `&` references per nesting level expands to
// (references per level)^depth copies of its ancestors. A ~3 KB stylesheet
// with ~20 nesting levels of `&:is(.bar, &.baz)` makes the printer allocate an
// effectively unbounded output buffer: `bun build` (whose default browser
// target predates CSS nesting) and `minifyTest` with explicit targets both
// spin forever while memory grows.
//
// The serializer now budgets the number of `&` substitutions per rule prelude
// and reports "Maximum nesting expansion exceeded" instead of expanding
// without bound. Preserving nesting (no targets) and ordinary nested CSS with
// old targets are unaffected.

/** Deeply nested rules where each level references the parent twice (`&` appears twice per selector). */
function explodingNestedCss(depth: number): string {
let css = "";
for (let i = 0; i < depth; i++) {
css += "&:is(.bar, &.baz) { color: red; }\n";
css += "&:is(.bar, &.baz) { colo\n"; // unclosed block, same shape as the fuzz input
}
css += "&:is(.bar, &.baz) { color: red; }\n";
css += "}";
return css;
}

const minifyTestScript = `
const { cssInternals } = require("bun:internal-for-testing");
const depth = parseInt(process.env.NESTED_CSS_DEPTH, 10);
const targets = process.env.NESTED_CSS_TARGETS === "1" ? { safari: 13 << 16 } : undefined;
let css = "";
for (let i = 0; i < depth; i++) {
css += "&:is(.bar, &.baz) { color: red; }\\n";
css += "&:is(.bar, &.baz) { colo\\n";
}
css += "&:is(.bar, &.baz) { color: red; }\\n";
css += "}";
try {
const out = targets ? cssInternals.minifyTest(css, "", targets) : cssInternals.minifyTest(css, "");
console.log("OK " + out.length);
} catch (err) {
console.log("ERR " + err.message);
}
`;

async function runMinifyTest(depth: number, withTargets: boolean) {
await using proc = Bun.spawn({
cmd: [bunExe(), "-e", minifyTestScript],
env: {
...bunEnv,
BUN_FEATURE_FLAG_INTERNAL_FOR_TESTING: "1",
NESTED_CSS_DEPTH: String(depth),
NESTED_CSS_TARGETS: withTargets ? "1" : "0",
},
stdout: "pipe",
stderr: "pipe",
// Kill switch: before the fix these spins were unbounded. Let the child be
// killed so a regression fails the assertions below instead of hanging the
// test runner and exhausting memory.
timeout: 20_000,
killSignal: "SIGKILL",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
return { stdout, stderr, exitCode, signalCode: proc.signalCode };
}

test.concurrent(
"deeply nested `&` selectors error out instead of expanding without bound when compiling nesting",
async () => {
const { stdout, stderr, signalCode, exitCode } = await runMinifyTest(24, true);
expect(stderr).toBe("");
expect(signalCode).toBeNull(); // not killed by the kill switch
expect(stdout).toContain("ERR Maximum nesting expansion exceeded");
expect(exitCode).toBe(0);
},
);

test.concurrent("deeply nested `&` selectors still minify when nesting is preserved (no targets)", async () => {
const { stdout, stderr, signalCode, exitCode } = await runMinifyTest(24, false);
expect(stderr).toBe("");
expect(signalCode).toBeNull();
// Without targets the nesting is preserved, so the output stays small.
expect(stdout).toStartWith("OK ");
expect(exitCode).toBe(0);
});

test.concurrent("ordinary nested CSS still compiles for older targets", async () => {
await using proc = Bun.spawn({
cmd: [
bunExe(),
"-e",
`
const { cssInternals } = require("bun:internal-for-testing");
// 8 levels deep, one parent reference per level: well within the budget.
let css = ".a { color: red; ";
for (let i = 0; i < 8; i++) css += "&:hover .b" + i + " { color: blue; ";
css += "}".repeat(9);
console.log(cssInternals.minifyTest(css, "", { safari: 13 << 16 }));
`,
],
env: { ...bunEnv, BUN_FEATURE_FLAG_INTERNAL_FOR_TESTING: "1" },
stdout: "pipe",
stderr: "pipe",
timeout: 20_000,
killSignal: "SIGKILL",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).toBe("");
// The innermost rule's `&` chain is fully expanded.
expect(stdout).toContain(".a:hover .b0:hover .b1:hover .b2:hover .b3:hover .b4:hover .b5:hover .b6:hover .b7");
expect(exitCode).toBe(0);
});

test.concurrent("bun build does not hang on deeply nested `&` selectors with the default browser target", async () => {
using dir = tempDir("css-nested-selector-expansion", {
"explode.css": explodingNestedCss(24),
});
await using proc = Bun.spawn({
cmd: [bunExe(), "build", path.join(String(dir), "explode.css"), "--outdir", path.join(String(dir), "out")],
env: bunEnv,
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
// Kill switch: before the fix this build spun forever while allocating.
timeout: 20_000,
killSignal: "SIGKILL",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
// The build must terminate on its own (the printer reports an error for the
// runaway rule) instead of being killed by the 20s kill switch.
expect(proc.signalCode).toBeNull();
expect(exitCode).not.toBeNull();
});
Loading