-
Notifications
You must be signed in to change notification settings - Fork 5.1k
css: reject stylesheets of 2 GiB or more instead of aborting on an int cast #39113
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
robobun
wants to merge
7
commits into
main
Choose a base branch
from
farm/52257f14/css-input-length-bound
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+229
−3
Open
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
446c2f7
css: reject stylesheets of 2 GiB or more instead of aborting on an in…
robobun 9d638b7
css: shorten the MAX_INPUT_LEN doc comment
robobun 98e9dc3
css: one-line doc comment for MAX_INPUT_LEN
robobun 07467b0
css: store the source span of a url() import record, test the bound a…
robobun cb12478
css: one-line comment on the import record span
robobun e0c560a
css test: gate the attribute case on 16 GiB, its transcode peaks at 8…
robobun 6a971b3
css: bound the Bun.color parser input too
robobun File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,200 @@ | ||
| import { describe, expect, test } from "bun:test"; | ||
| import { bunEnv, bunExe, isWindows, tempDir } from "harness"; | ||
| import { closeSync, openSync, statSync, writeSync } from "node:fs"; | ||
| import os from "node:os"; | ||
| import { join } from "node:path"; | ||
|
|
||
| // Every byte offset the CSS parser hands to the rest of bun (import records, | ||
| // CSS module symbols, `composes`) and every line/column in its diagnostics is | ||
| // an i32, so once the tokenizer got past byte 2**31 of a stylesheet the process | ||
| // aborted with `panic: int cast: TryFromIntError(PosOverflow)`. The parser now | ||
| // refuses input longer than MAX_INPUT_LEN up front with an ordinary error, | ||
| // before reading it. | ||
| // | ||
| // Each stylesheet here is one comment covering almost all of it followed by a | ||
| // `composes` declaration, a cast site that every entry point reaches (a url() | ||
| // only becomes an import record when bundling). The comment body is all zero | ||
| // bytes or spaces, so it is cheap to produce: untouched pages of a Uint8Array, | ||
| // a hole in a sparse file, or one Buffer.alloc. Handing it to bun still costs | ||
| // the child 2 GiB of memory (8.3 GiB peak for the attribute case, which | ||
| // transcodes), hence the memory gates (10 GiB is the one fs-oom.test.ts uses | ||
| // for its 2 GiB reads) and the timeout: a child takes 3 to 11 s in a debug | ||
| // build. The tests are deliberately not concurrent, so at most one such child | ||
| // exists at a time. | ||
| const MAX_INPUT_LEN = 2 ** 31 - 2; | ||
| const TAIL = "*/.a{composes:b}\n"; | ||
| const MESSAGE = "CSS file is too large to parse (2 GiB maximum)"; | ||
| const CHILD_TIMEOUT = 30_000; | ||
|
|
||
| // Inside a container os.totalmem() reports the host's RAM; | ||
| // process.constrainedMemory() reports the cgroup limit there. | ||
| const memory = Math.min(os.totalmem(), process.constrainedMemory() || Infinity); | ||
|
|
||
| // Builds an in-memory stylesheet of `length` bytes with `Bun.build` and prints | ||
| // the outcome. The comment closes `TAIL.length` bytes before the end. | ||
| function bunBuildScript(length: number): string { | ||
| return ` | ||
| const tail = new TextEncoder().encode(${JSON.stringify(TAIL)}); | ||
| const bytes = new Uint8Array(${length}); | ||
| bytes.set([0x2f, 0x2a]); // "/*" | ||
| bytes.set(tail, bytes.length - tail.length); | ||
| const result = await Bun.build({ | ||
| entrypoints: ["/app/big.css"], | ||
| files: { "/app/big.css": bytes }, | ||
| throw: false, | ||
| }); | ||
| console.log(JSON.stringify({ | ||
| success: result.success, | ||
| logs: result.logs.map(log => ({ level: log.level, message: log.message, file: log.position?.file })), | ||
| })); | ||
| `; | ||
| } | ||
|
|
||
| describe.skipIf(memory < 10 * 1024 ** 3)("stylesheet of 2 GiB or more", () => { | ||
| // The `composes` sits past byte 2**31, where its offset no longer fits an | ||
| // i32. This is the input that aborted before. | ||
| test( | ||
| "Bun.build reports an error naming the file", | ||
| async () => { | ||
| await using proc = Bun.spawn({ | ||
| cmd: [bunExe(), "-e", bunBuildScript(2 ** 31 + TAIL.length)], | ||
| env: bunEnv, | ||
| stdout: "pipe", | ||
| stderr: "pipe", | ||
| }); | ||
| const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); | ||
| expect(stderr).toBe(""); | ||
| expect(JSON.parse(stdout)).toEqual({ | ||
| success: false, | ||
| logs: [{ level: "error", message: MESSAGE, file: "/app/big.css" }], | ||
| }); | ||
| expect(exitCode).toBe(0); | ||
| }, | ||
| CHILD_TIMEOUT, | ||
| ); | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| // One byte past the limit is rejected too, even though every offset in it | ||
| // still fits an i32. (At the limit the stylesheet parses, but a 2 GiB | ||
| // tokenize takes minutes in a debug build, so that side is not tested.) | ||
| test( | ||
| "Bun.build rejects MAX_INPUT_LEN + 1 bytes", | ||
| async () => { | ||
| await using proc = Bun.spawn({ | ||
| cmd: [bunExe(), "-e", bunBuildScript(MAX_INPUT_LEN + 1)], | ||
| env: bunEnv, | ||
| stdout: "pipe", | ||
| stderr: "pipe", | ||
| }); | ||
| const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); | ||
| expect(stderr).toBe(""); | ||
| expect(JSON.parse(stdout)).toEqual({ | ||
| success: false, | ||
| logs: [{ level: "error", message: MESSAGE, file: "/app/big.css" }], | ||
| }); | ||
| expect(exitCode).toBe(0); | ||
| }, | ||
| CHILD_TIMEOUT, | ||
| ); | ||
|
|
||
| // Windows only makes a file sparse on request, so seeking past 2 GiB there | ||
| // would really write 2 GiB of zeros. The bound is the same code on every | ||
| // platform and the tests above already run there. | ||
| test.skipIf(isWindows)( | ||
| "bun build --no-bundle reports an error", | ||
| async () => { | ||
| using dir = tempDir("css-too-large", {}); | ||
| const css = join(String(dir), "big.css"); | ||
| const tail = Buffer.from(TAIL); | ||
| const fd = openSync(css, "w"); | ||
| try { | ||
| writeSync(fd, Buffer.from("/*"), 0, 2, 0); | ||
| writeSync(fd, tail, 0, tail.length, 2 ** 31); | ||
| } finally { | ||
| closeSync(fd); | ||
| } | ||
| expect(statSync(css).size).toBe(2 ** 31 + tail.length); | ||
|
|
||
| await using proc = Bun.spawn({ | ||
| cmd: [bunExe(), "build", "--no-bundle", css], | ||
| env: bunEnv, | ||
| stdout: "pipe", | ||
| stderr: "pipe", | ||
| }); | ||
| const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); | ||
| expect(stderr).toBe(`error: ${MESSAGE} parsing\n`); | ||
| expect(stdout).toBe(""); | ||
|
claude[bot] marked this conversation as resolved.
|
||
| expect(exitCode).toBe(1); | ||
| }, | ||
| CHILD_TIMEOUT, | ||
| ); | ||
|
|
||
| // A style attribute goes through `StyleAttribute::parse`, the other entry | ||
| // point with the bound. A JS string holds at most 2**31 - 1 code units, so | ||
| // the tail is Latin-1 text that grows when encoded as UTF-8: that puts the | ||
| // `composes` past byte 2**31 and its offset out of i32 range. The transcode | ||
| // holds the 2 GiB Buffer, the string and two UTF-8 buffers at once: 8.3 GiB | ||
| // peak (VmHWM) in a debug build. | ||
| test.skipIf(memory < 16 * 1024 ** 3)( | ||
| "StyleAttribute::parse reports an error", | ||
| async () => { | ||
| await using proc = Bun.spawn({ | ||
| cmd: [ | ||
| bunExe(), | ||
| "-e", | ||
| ` | ||
| const { cssInternals } = require("bun:internal-for-testing"); | ||
| const length = 2 ** 31 - 1; | ||
| const buf = Buffer.alloc(length, 0x20); | ||
| const tail = Buffer.from("/*" + "\\u00e9".repeat(40) + "*/composes:b", "latin1"); | ||
| tail.copy(buf, length - tail.length); | ||
| try { | ||
| cssInternals.attrTest(buf.toString("latin1"), "", false); | ||
|
robobun marked this conversation as resolved.
|
||
| console.log("parsed"); | ||
| } catch (error) { | ||
| console.log(error.message); | ||
| } | ||
| `, | ||
| ], | ||
| env: bunEnv, | ||
| stdout: "pipe", | ||
| stderr: "pipe", | ||
| }); | ||
| const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); | ||
| expect(stderr).toBe(""); | ||
| expect(stdout).toBe(`parsing failed: ${MESSAGE}\n`); | ||
| expect(exitCode).toBe(0); | ||
| }, | ||
| CHILD_TIMEOUT, | ||
| ); | ||
|
|
||
| // `Bun.color` builds its own parser over the UTF-8 of the string. The longest | ||
| // JS string, 2**31 - 1 ASCII chars, is MAX_INPUT_LEN + 1 bytes. Before the | ||
| // bound this input parsed as an invalid color and returned null. | ||
| test( | ||
| "Bun.color throws", | ||
| async () => { | ||
| await using proc = Bun.spawn({ | ||
| cmd: [ | ||
| bunExe(), | ||
| "-e", | ||
| ` | ||
| const input = Buffer.alloc(2 ** 31 - 1, 0x20).toString("latin1"); | ||
| try { | ||
| console.log(JSON.stringify(Bun.color(input, "css"))); | ||
| } catch (error) { | ||
| console.log(error.message); | ||
| } | ||
| `, | ||
| ], | ||
| env: bunEnv, | ||
| stdout: "pipe", | ||
| stderr: "pipe", | ||
| }); | ||
| const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); | ||
| expect(stderr).toBe(""); | ||
| expect(stdout).toBe("color() input is too large to parse (2 GiB maximum)\n"); | ||
| expect(exitCode).toBe(0); | ||
| }, | ||
| CHILD_TIMEOUT, | ||
| ); | ||
| }); | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.