-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Fix panic when bundling CSS that contains invalid UTF-8 #32795
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
Closed
Closed
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
071911c
css: replace invalid UTF-8 with U+FFFD before tokenizing
robobun 49991ef
css: allocate the U+FFFD-replaced source in the parse arena
robobun eed729b
test: report the child's stderr when the Bun.build fixture fails
robobun 683a93f
css: decode invalid UTF-8 with the existing string converters
robobun 86bc438
css: move invalid UTF-8 replacement into bun_core::strings
robobun 6cb6873
test: drain stdout in the escape-decoder case
robobun 038075c
css: tighten two comments
robobun 9789336
css: size the replacement buffer exactly and assert exit codes last
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
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,157 @@ | ||
| import { describe, expect, test } from "bun:test"; | ||
| import { bunEnv, bunExe, tempDir } from "harness"; | ||
| import { writeFileSync } from "node:fs"; | ||
| import { join } from "node:path"; | ||
|
|
||
| // CSS sources whose bytes are not valid UTF-8. The bundler must decode them | ||
| // (invalid sequences become U+FFFD) instead of tokenizing the raw bytes, | ||
| // which used to crash with `panic: unreachable` once an unresolvable | ||
| // `@import` specifier containing the raw byte reached the error formatter. | ||
| // | ||
| // 0xE2 is a three-byte UTF-8 lead with no continuation bytes after it. | ||
| const importWithInvalidByte = Buffer.concat([ | ||
| Buffer.from('@import url("./x'), | ||
| Buffer.from([0xe2]), | ||
| Buffer.from('y.css");\n'), | ||
| ]); | ||
|
|
||
| describe("css with invalid utf-8", () => { | ||
| test.concurrent("unresolvable @import reports a resolve error", async () => { | ||
| using dir = tempDir("css-invalid-utf8-import", {}); | ||
| writeFileSync(join(String(dir), "in.css"), importWithInvalidByte); | ||
|
|
||
| await using proc = Bun.spawn({ | ||
| cmd: [bunExe(), "build", "./in.css", "--outdir=out"], | ||
| env: bunEnv, | ||
| cwd: String(dir), | ||
| stdout: "pipe", | ||
| stderr: "pipe", | ||
| }); | ||
| const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); | ||
|
|
||
| // The invalid byte is replaced with U+FFFD, exactly as a browser decodes | ||
| // the stylesheet, so the import fails to resolve like any other typo. | ||
| expect(stderr).toContain('Could not resolve: "./x\uFFFDy.css"'); | ||
| expect(stdout).not.toContain("Bundled"); | ||
| expect(exitCode).toBe(1); | ||
| }); | ||
|
|
||
| test.concurrent("Bun.build reports the failure on a ResolveMessage", async () => { | ||
| using dir = tempDir("css-invalid-utf8-api", { | ||
| "build.js": ` | ||
| const result = await Bun.build({ entrypoints: ["./in.css"], throw: false }); | ||
| const log = result.logs[0]; | ||
| console.log(JSON.stringify({ success: result.success, message: log.message, specifier: log.specifier })); | ||
| `, | ||
| }); | ||
| writeFileSync(join(String(dir), "in.css"), importWithInvalidByte); | ||
|
|
||
| await using proc = Bun.spawn({ | ||
| cmd: [bunExe(), "run", "./build.js"], | ||
| env: bunEnv, | ||
| cwd: String(dir), | ||
| stdout: "pipe", | ||
| stderr: "pipe", | ||
| }); | ||
| const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); | ||
|
|
||
| // Checked before JSON.parse so a crashed child reports its stderr | ||
| // instead of a JSON syntax error on the empty stdout. | ||
| expect({ stdout, stderr, exitCode }).toMatchObject({ exitCode: 0 }); | ||
|
|
||
| // Only the ASCII affixes of the specifier are asserted here: the | ||
| // ResolveMessage getters mis-decode non-ASCII text today (reproducible | ||
| // on its own with `import "./café.js"`), which is a separate issue. | ||
| const log = JSON.parse(stdout); | ||
| expect(log.success).toBe(false); | ||
| expect(log.message).toStartWith('Could not resolve: "./x'); | ||
| expect(log.specifier).toStartWith("./x"); | ||
| expect(log.specifier).toEndWith("y.css"); | ||
| }); | ||
|
|
||
| test.concurrent("url() token in an at-rule prelude reports a resolve error", async () => { | ||
| using dir = tempDir("css-invalid-utf8-url", {}); | ||
| writeFileSync( | ||
| join(String(dir), "in.css"), | ||
| Buffer.concat([Buffer.from("@-x url(a"), Buffer.from([0xe2]), Buffer.from("b) tok;\n")]), | ||
| ); | ||
|
|
||
| await using proc = Bun.spawn({ | ||
| cmd: [bunExe(), "build", "./in.css", "--outdir=out"], | ||
| env: bunEnv, | ||
| cwd: String(dir), | ||
| stdout: "pipe", | ||
| stderr: "pipe", | ||
| }); | ||
| const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); | ||
|
|
||
| expect(stderr).toContain('Could not resolve: "a\uFFFDb"'); | ||
| expect(exitCode).toBe(1); | ||
| }); | ||
|
|
||
| test.concurrent("invalid bytes outside an import become U+FFFD in the output", async () => { | ||
| using dir = tempDir("css-invalid-utf8-content", {}); | ||
| // `content: "caf<0xE9>"` (latin-1 "café"): the build succeeds and the | ||
| // emitted stylesheet is well-formed UTF-8, matching how a browser would | ||
| // have decoded the input. | ||
| writeFileSync( | ||
| join(String(dir), "in.css"), | ||
| Buffer.concat([Buffer.from('a { content: "caf'), Buffer.from([0xe9]), Buffer.from('"; }\n')]), | ||
| ); | ||
|
|
||
| await using proc = Bun.spawn({ | ||
| cmd: [bunExe(), "build", "./in.css", "--outdir=out"], | ||
| env: bunEnv, | ||
| cwd: String(dir), | ||
| stdout: "pipe", | ||
| stderr: "pipe", | ||
| }); | ||
| const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); | ||
| expect(stderr).not.toContain("error:"); | ||
|
|
||
| const out = new Uint8Array(await Bun.file(join(String(dir), "out", "in.css")).arrayBuffer()); | ||
| expect(Buffer.from(out).includes(Buffer.from('content: "caf\uFFFD"'))).toBe(true); | ||
| expect(Buffer.from(out).includes(0xe9)).toBe(false); | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| expect(exitCode).toBe(0); | ||
| }); | ||
|
|
||
| test.concurrent("escaped invalid byte does not swallow the bytes after it", async () => { | ||
| using dir = tempDir("css-invalid-utf8-escape", {}); | ||
| // `\` followed by a lone 0xC3 lead byte. The escape must decode to U+FFFD | ||
| // and consume exactly that byte; it used to consume three (the UTF-8 | ||
| // length of U+FFFD), eating ident characters, string content, or the | ||
| // closing quote. | ||
| writeFileSync( | ||
| join(String(dir), "in.css"), | ||
| Buffer.concat([ | ||
| Buffer.from(".x\\"), | ||
| Buffer.from([0xc3]), | ||
| Buffer.from('yz { content: "\\'), | ||
| Buffer.from([0xc3]), | ||
| Buffer.from('x AFTER"; }\n.a { content: "\\'), | ||
| Buffer.from([0xc3]), | ||
| Buffer.from('"; color: green; }\n'), | ||
| ]), | ||
| ); | ||
|
|
||
| await using proc = Bun.spawn({ | ||
| cmd: [bunExe(), "build", "./in.css", "--outdir=out"], | ||
| env: bunEnv, | ||
| cwd: String(dir), | ||
| stdout: "pipe", | ||
| stderr: "pipe", | ||
| }); | ||
| const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); | ||
| expect(stderr).not.toContain("error:"); | ||
|
|
||
| const out = await Bun.file(join(String(dir), "out", "in.css")).text(); | ||
| // Selector: `yz` used to be eaten out of the class name. | ||
| expect(out).toContain(".x\uFFFDyz"); | ||
| // String content: the `x ` after the escape used to be eaten. | ||
| expect(out).toContain('content: "\uFFFDx AFTER"'); | ||
| // The closing quote used to be eaten, absorbing the rest of the rule. | ||
| expect(out).toContain('content: "\uFFFD";'); | ||
| expect(out).toContain("color: green;"); | ||
| expect(exitCode).toBe(0); | ||
| }); | ||
| }); | ||
|
robobun marked this conversation as resolved.
|
||
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.