From d77c797570e4d80b85baac732d80ef865c381549 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 6 Sep 2026 09:45:05 +0000 Subject: [PATCH 01/11] css: keep bundler url() placeholders quoted when minifying The CSS printer does not see the final path of a copied asset. It prints a placeholder and the bundler replaces it with the output path after printing. With --minify the placeholder was printed as an unquoted url(), so a final path with a space, a quote or a parenthesis became a bad-url token and the browser dropped the declaration. Print a placeholder inside a quoted string in every mode. Real urls (external, data:) are still printed unquoted when that is shorter. --- src/css/printer.rs | 34 +++++++++++++++++++++++++++++--- src/css/properties/custom.rs | 11 +++++++---- src/css/values/url.rs | 7 ++++--- test/bundler/esbuild/css.test.ts | 31 ++++++++++++++++++++++++++++- 4 files changed, 72 insertions(+), 11 deletions(-) diff --git a/src/css/printer.rs b/src/css/printer.rs index c37427280041..c764a43ed07d 100644 --- a/src/css/printer.rs +++ b/src/css/printer.rs @@ -333,6 +333,19 @@ impl<'a> Printer<'a> { #[inline] pub(crate) fn get_import_record_url(&mut self, import_record_idx: u32) -> PrintResult<&[u8]> { + self.resolve_import_record_url(import_record_idx) + .map(|resolved| resolved.url) + } + + /// The url to print for an import record, and whether it is a bundler + /// placeholder (a unique key) that the bundler replaces with the final + /// output path after printing. A placeholder has to be printed inside a + /// quoted string: the final path is spliced in verbatim, and an unquoted + /// `url()` cannot hold a space, a quote or a parenthesis. + pub(crate) fn resolve_import_record_url( + &mut self, + import_record_idx: u32, + ) -> PrintResult> { let Some(import_info) = &self.import_info else { return Err(self.add_no_import_record_error()); }; @@ -367,17 +380,26 @@ impl<'a> Printer<'a> { Some(i) => &suffix[i as usize..], None => b"", }; - return Ok(with_suffix(urls_for_css, fragment)); + return Ok(ResolvedImportRecordUrl { + url: with_suffix(urls_for_css, fragment), + is_placeholder: false, + }); } // It is a chunk URL (copied asset): keep the full `?query#fragment`. let unique_key_for_additional_file = import_info.ast_unique_key_for_additional_file[record.source_index.get() as usize]; if !unique_key_for_additional_file.is_empty() { - return Ok(with_suffix(unique_key_for_additional_file, suffix)); + return Ok(ResolvedImportRecordUrl { + url: with_suffix(unique_key_for_additional_file, suffix), + is_placeholder: true, + }); } } // External URL stays as-is - Ok(record.path.text) + Ok(ResolvedImportRecordUrl { + url: record.path.text, + is_placeholder: false, + }) } pub(crate) fn context(&self) -> Option<&css::StyleContext<'a>> { @@ -385,6 +407,12 @@ impl<'a> Printer<'a> { } } +/// See [`Printer::resolve_import_record_url`]. +pub(crate) struct ResolvedImportRecordUrl<'u> { + pub(crate) url: &'u [u8], + pub(crate) is_placeholder: bool, +} + /// `Printer` participates in `serializer::serialize_*` so /// `Token::to_css` and friends can write through it generically. impl<'a> bun_io::Write for Printer<'a> { diff --git a/src/css/properties/custom.rs b/src/css/properties/custom.rs index ff887d55c14a..b66b99fe01de 100644 --- a/src/css/properties/custom.rs +++ b/src/css/properties/custom.rs @@ -95,13 +95,16 @@ mod ext { let is_internal = import_record.tag.is_internal(); // `get_import_record_url` reborrows &mut *dest, so capture // `is_internal` first. - let url: &'static [u8] = { - let u = dest.get_import_record_url(this.import_record_idx)?; + let (url, is_placeholder): (&'static [u8], bool) = { + let resolved = dest.resolve_import_record_url(this.import_record_idx)?; // SAFETY: import-record paths are arena/source-owned and outlive `dest`. - unsafe { &*std::ptr::from_ref::<[u8]>(u) } + ( + unsafe { &*std::ptr::from_ref::<[u8]>(resolved.url) }, + resolved.is_placeholder, + ) }; - if dest.minify && !is_internal { + if dest.minify && !is_internal && !is_placeholder { let mut buf: Vec = Vec::new(); // PERF(alloc) we could use stack fallback here? let _ = Token::UnquotedUrl(url).to_css_generic(&mut buf); diff --git a/src/css/values/url.rs b/src/css/values/url.rs index 8c228fa0af53..4bf5df27d6f6 100644 --- a/src/css/values/url.rs +++ b/src/css/values/url.rs @@ -100,13 +100,14 @@ impl Url { let is_internal = import_record .flags .contains(bun_ast::ImportRecordFlags::IS_INTERNAL); - let url = dest.get_import_record_url(self.import_record_idx)?; + let resolved = dest.resolve_import_record_url(self.import_record_idx)?; + let is_placeholder = resolved.is_placeholder; // SAFETY: `url` borrows arena-backed `import_info` data valid for the // printer's `'a`; detach so `dest` can be re-borrowed mutably below. // Printer arena, not parser source — route to the raw primitive. - let url: &[u8] = unsafe { bun_collections::detach_lifetime(url) }; + let url: &[u8] = unsafe { bun_collections::detach_lifetime(resolved.url) }; - if dest.minify && !is_internal { + if dest.minify && !is_internal && !is_placeholder { let mut buf: Vec = Vec::new(); // PERF(alloc) we could use stack fallback here? // `Token::to_css_generic(UnquotedUrl(url))` is inlined here — diff --git a/test/bundler/esbuild/css.test.ts b/test/bundler/esbuild/css.test.ts index e450a5fcd60f..b8b8f3e8a6ad 100644 --- a/test/bundler/esbuild/css.test.ts +++ b/test/bundler/esbuild/css.test.ts @@ -1259,7 +1259,36 @@ b { minifyWhitespace: true, outdir: "/out", onAfterBundle(api) { - api.expectFile("/out/entry.css").toEqualIgnoringWhitespace(/* css */ `.a{mask:url(./sprites-mrrzcz3w.svg#icon)}`); + api.expectFile("/out/entry.css").toEqualIgnoringWhitespace(/* css */ `.a{mask:url("./sprites-mrrzcz3w.svg#icon")}`); + }, + }); + + // The printer only sees a placeholder for a copied asset. The bundler splices + // the final path in afterwards, so the placeholder has to stay quoted: an + // unquoted url() with a space or a parenthesis is a bad-url token. + itBundled("css/FileLoaderURLWithSpecialCharsMinified", { + files: { + "/entry.css": /* css */ ` + .a { background: url("./my image.png") } + .b { background: url("./a(b).png") } + .c { --bg: url("./my image.png") } + .d { background: url("./plain.png") } + `, + "/my image.png": Buffer.alloc(128 * 1024 + 1, "Z").toString(), + "/a(b).png": Buffer.alloc(128 * 1024 + 1, "Y").toString(), + "/plain.png": Buffer.alloc(128 * 1024 + 1, "X").toString(), + }, + loader: { + ".png": "file", + }, + minifyWhitespace: true, + outdir: "/out", + onAfterBundle(api) { + api + .expectFile("/out/entry.css") + .toEqualIgnoringWhitespace( + '.a{background:url("./my image-mrrzcz3w.png")}.b{background:url("./a(b)-nkpagkva.png")}.c{--bg:url("./my image-mrrzcz3w.png")}.d{background:url("./plain-w49ecq2a.png")}', + ); }, }); From 5ca800dab97bb07ba9d7b1b227fc5ce18f93f17c Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Sun, 6 Sep 2026 09:49:12 +0000 Subject: [PATCH 02/11] [autofix.ci] apply automated fixes --- test/bundler/esbuild/css.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/bundler/esbuild/css.test.ts b/test/bundler/esbuild/css.test.ts index b8b8f3e8a6ad..386979d03087 100644 --- a/test/bundler/esbuild/css.test.ts +++ b/test/bundler/esbuild/css.test.ts @@ -1259,7 +1259,9 @@ b { minifyWhitespace: true, outdir: "/out", onAfterBundle(api) { - api.expectFile("/out/entry.css").toEqualIgnoringWhitespace(/* css */ `.a{mask:url("./sprites-mrrzcz3w.svg#icon")}`); + api + .expectFile("/out/entry.css") + .toEqualIgnoringWhitespace(/* css */ `.a{mask:url("./sprites-mrrzcz3w.svg#icon")}`); }, }); From ef19aded71bff5598a8ab75133123eed49ae4dd8 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 6 Sep 2026 09:50:07 +0000 Subject: [PATCH 03/11] css: shorten the resolve_import_record_url doc comment --- src/css/printer.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/css/printer.rs b/src/css/printer.rs index c764a43ed07d..b8c7ef1ba3e2 100644 --- a/src/css/printer.rs +++ b/src/css/printer.rs @@ -337,11 +337,8 @@ impl<'a> Printer<'a> { .map(|resolved| resolved.url) } - /// The url to print for an import record, and whether it is a bundler - /// placeholder (a unique key) that the bundler replaces with the final - /// output path after printing. A placeholder has to be printed inside a - /// quoted string: the final path is spliced in verbatim, and an unquoted - /// `url()` cannot hold a space, a quote or a parenthesis. + /// The url to print for an import record. `is_placeholder` means the + /// bundler replaces it with the final output path after printing. pub(crate) fn resolve_import_record_url( &mut self, import_record_idx: u32, From a61987329395c7a6b773c6db6cdc2047a3d985ac Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 6 Sep 2026 09:51:19 +0000 Subject: [PATCH 04/11] css: move the placeholder note onto the field --- src/css/printer.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/css/printer.rs b/src/css/printer.rs index b8c7ef1ba3e2..5ab97a25c3d0 100644 --- a/src/css/printer.rs +++ b/src/css/printer.rs @@ -337,8 +337,6 @@ impl<'a> Printer<'a> { .map(|resolved| resolved.url) } - /// The url to print for an import record. `is_placeholder` means the - /// bundler replaces it with the final output path after printing. pub(crate) fn resolve_import_record_url( &mut self, import_record_idx: u32, @@ -404,9 +402,9 @@ impl<'a> Printer<'a> { } } -/// See [`Printer::resolve_import_record_url`]. pub(crate) struct ResolvedImportRecordUrl<'u> { pub(crate) url: &'u [u8], + /// A unique key the bundler replaces with the final output path after printing. pub(crate) is_placeholder: bool, } From 2741d5b85bfb454b41124a2c06faada2b8826891 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 13 Sep 2026 17:33:46 +0000 Subject: [PATCH 05/11] bundler: escape the asset path spliced into a CSS chunk for the string it lands in The CSS printer writes a placeholder for a copied asset inside a quoted string, and the chunk join replaces it with publicPath + output path verbatim. The [name] part of that path is the asset's file name, so a quote or a line break in the name ended the string. A crafted name such as x");}body{color:red}a{b:url("y.png injected a whole rule. For CSS chunks, the count pass and the write pass of the join now run both parts of the path through the CSS string serializer. The count pass also applies the same separator normalization as the write pass, so the two agree on the escaped length. --- src/bundler/Chunk.rs | 58 ++++++++++++++++++++++++-------- src/css/css_parser.rs | 11 ++++-- test/bundler/esbuild/css.test.ts | 57 +++++++++++++++++++++++++++++-- 3 files changed, 108 insertions(+), 18 deletions(-) diff --git a/src/bundler/Chunk.rs b/src/bundler/Chunk.rs index d457e620f546..148caaf8b841 100644 --- a/src/bundler/Chunk.rs +++ b/src/bundler/Chunk.rs @@ -607,6 +607,22 @@ impl IntermediateOutput { dst } + /// The CSS printer writes every placeholder inside a double-quoted string, + /// so the path that replaces one in a CSS chunk is escaped for that string. + fn css_string_escaped_len(path: &[u8]) -> usize { + let mut counter = bun_io::DiscardingWriter::new(); + let _ = bun_css::css_parser::serializer::serialize_string_contents(path, &mut counter); + counter.count + } + + /// Caller must ensure `dest` has room for `css_string_escaped_len(path)` bytes. + fn memcpy_css_string_escaped(dest: &mut [u8], path: &[u8]) -> usize { + let mut stream = bun_io::FixedBufferStream::new_mut(dest); + bun_css::css_parser::serializer::serialize_string_contents(path, &mut stream) + .expect("unreachable: sized by css_string_escaped_len"); + stream.pos + } + pub(crate) fn get_size(&self) -> usize { match self { IntermediateOutput::Pieces(pieces) => { @@ -737,6 +753,7 @@ impl IntermediateOutput { graph.input_files.items_unique_key_for_additional_file(); let mut relative_platform_buf = bun_paths::path_buffer_pool::get(); let mut file_path_buf = bun_paths::path_buffer_pool::get(); + let escape_for_css = chunk.content.is_css(); match self { IntermediateOutput::Pieces(pieces) => { let entry_point_chunks_for_scb = linker_graph.files.items_entry_point_chunk_index(); @@ -852,6 +869,15 @@ impl IntermediateOutput { QueryKind::None | QueryKind::ChunkId => unreachable!(), }; + // Same `\` → `/` normalization as the write pass, so the + // escaped length below matches what is written. + let file_path: &[u8] = { + let n = file_path.len(); + let dst = &mut file_path_buf[..n]; + dst.copy_from_slice(file_path); + bun_paths::resolve_path::platform_to_posix_in_place::(dst); + dst + }; let cheap_normalizer = cheap_prefix_normalizer( import_prefix, if use_outdir_relative_path { @@ -865,7 +891,13 @@ impl IntermediateOutput { ) }, ); - count += cheap_normalizer[0].len() + cheap_normalizer[1].len(); + for part in cheap_normalizer { + count += if escape_for_css { + Self::css_string_escaped_len(part) + } else { + part.len() + }; + } } QueryKind::None => {} } @@ -1064,22 +1096,20 @@ impl IntermediateOutput { }, ); - if !cheap_normalizer[0].is_empty() { - remain[..cheap_normalizer[0].len()] - .copy_from_slice(cheap_normalizer[0]); - remain = &mut remain[cheap_normalizer[0].len()..]; - if ENABLE_SOURCE_MAP_SHIFTS { - shift.after.advance(cheap_normalizer[0]); + for part in cheap_normalizer { + if part.is_empty() { + continue; } - } - - if !cheap_normalizer[1].is_empty() { - remain[..cheap_normalizer[1].len()] - .copy_from_slice(cheap_normalizer[1]); - remain = &mut remain[cheap_normalizer[1].len()..]; + let written = if escape_for_css { + Self::memcpy_css_string_escaped(remain, part) + } else { + remain[..part.len()].copy_from_slice(part); + part.len() + }; if ENABLE_SOURCE_MAP_SHIFTS { - shift.after.advance(cheap_normalizer[1]); + shift.after.advance(&remain[..written]); } + remain = &mut remain[written..]; } if ENABLE_SOURCE_MAP_SHIFTS { diff --git a/src/css/css_parser.rs b/src/css/css_parser.rs index 5448c85586ca..c91aa54addce 100644 --- a/src/css/css_parser.rs +++ b/src/css/css_parser.rs @@ -5760,11 +5760,18 @@ pub mod serializer { writer: &mut W, ) -> bun_io::Result<()> { writer.write_all(b"\"")?; - let mut sw = CssStringWriter::new(writer); - sw.write_str(value)?; + serialize_string_contents(value, writer)?; writer.write_all(b"\"") } + /// Write the inside of a double-quoted CSS string token, without the quotes. + pub fn serialize_string_contents( + value: &[u8], + writer: &mut W, + ) -> bun_io::Result<()> { + CssStringWriter::new(writer).write_str(value) + } + pub(crate) fn serialize_dimension( value: f32, unit: &'static [u8], diff --git a/test/bundler/esbuild/css.test.ts b/test/bundler/esbuild/css.test.ts index 386979d03087..9a3c42830b33 100644 --- a/test/bundler/esbuild/css.test.ts +++ b/test/bundler/esbuild/css.test.ts @@ -1,5 +1,6 @@ -import { describe } from "bun:test"; -import { join } from "node:path"; +import { describe, expect } from "bun:test"; +import { isWindows } from "harness"; +import { basename, join } from "node:path"; import { itBundled } from "../expectBundled"; // Tests ported from: @@ -1294,6 +1295,58 @@ b { }, }); + // The final path replaces the placeholder inside a quoted string, so it is + // escaped for one. Windows cannot represent `"` or a line break in a file name. + describe.skipIf(isWindows)("asset path is escaped for the CSS string it lands in", () => { + const injection = 'x");}body{color:red}a{b:url("y'; + itBundled("css/FileLoaderURLEscapesFileName", { + files: { + // `\22 ` is `"` and `\a ` is a line feed. + "/entry.css": String.raw` + .a { background: url("./a\a b.png") } + .b { background: url("./x\22 );}body{color:red}a{b:url(\22 y.png") } + `, + "/a\nb.png": Buffer.alloc(128 * 1024 + 1, "Y").toString(), + [`/${injection}.png`]: Buffer.alloc(128 * 1024 + 1, "X").toString(), + }, + loader: { + ".png": "file", + }, + minifyWhitespace: true, + outdir: "/out", + async onAfterBundle(api) { + expect(api.readFile("/out/entry.css")).toBe( + String.raw`.a{background:url("./a\a b-nkpagkva.png")}.b{background:url("./x\");}body{color:red}a{b:url(\"y-w49ecq2a.png")}` + + "\n", + ); + // Every url() in the output has to name a file that was written next to it. + const again = await Bun.build({ entrypoints: [api.join("out/entry.css")], outdir: api.join("out2") }); + const assets = again.outputs.map(o => basename(o.path)).filter(name => name.endsWith(".png")); + expect(assets.map(name => name.replace(/-[a-z0-9]+\.png$/, "")).sort()).toEqual( + ["a\nb-nkpagkva", `${injection}-w49ecq2a`].sort(), + ); + }, + }); + + itBundled("css/FileLoaderURLEscapesPublicPath", { + files: { + "/entry.css": `.a { background: url("./plain.png") }`, + "/plain.png": Buffer.alloc(128 * 1024 + 1, "X").toString(), + }, + loader: { + ".png": "file", + }, + publicPath: '");}body{color:red}a{b:url("', + minifyWhitespace: true, + outdir: "/out", + onAfterBundle(api) { + expect(api.readFile("/out/entry.css")).toBe( + String.raw`.a{background:url("\");}body{color:red}a{b:url(\"plain-w49ecq2a.png")}` + "\n", + ); + }, + }); + }); + itBundled("css/IgnoreURLsInAtRulePrelude", { // GENERATED files: { From 52e5b97d8d1eb7a0295de795ae23caff2db2f365 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 13 Sep 2026 17:40:00 +0000 Subject: [PATCH 06/11] bundler: share one separator-normalizing helper between the count and write passes --- src/bundler/Chunk.rs | 38 +++++++++++++++----------------------- 1 file changed, 15 insertions(+), 23 deletions(-) diff --git a/src/bundler/Chunk.rs b/src/bundler/Chunk.rs index 148caaf8b841..62644e243a02 100644 --- a/src/bundler/Chunk.rs +++ b/src/bundler/Chunk.rs @@ -607,8 +607,17 @@ impl IntermediateOutput { dst } - /// The CSS printer writes every placeholder inside a double-quoted string, - /// so the path that replaces one in a CSS chunk is escaped for that string. + /// `path` with Windows separators turned into `/`, copied into `buf`. The + /// source is reachable only through `&Graph` / `&[Chunk]`, so it is never + /// normalized in place. + fn to_posix_separators<'b>(buf: &'b mut [u8], path: &[u8]) -> &'b [u8] { + let dst = &mut buf[..path.len()]; + dst.copy_from_slice(path); + bun_paths::resolve_path::platform_to_posix_in_place::(dst); + dst + } + + /// In a CSS chunk every placeholder is printed inside a double-quoted string. fn css_string_escaped_len(path: &[u8]) -> usize { let mut counter = bun_io::DiscardingWriter::new(); let _ = bun_css::css_parser::serializer::serialize_string_contents(path, &mut counter); @@ -869,15 +878,8 @@ impl IntermediateOutput { QueryKind::None | QueryKind::ChunkId => unreachable!(), }; - // Same `\` → `/` normalization as the write pass, so the - // escaped length below matches what is written. - let file_path: &[u8] = { - let n = file_path.len(); - let dst = &mut file_path_buf[..n]; - dst.copy_from_slice(file_path); - bun_paths::resolve_path::platform_to_posix_in_place::(dst); - dst - }; + let file_path = + Self::to_posix_separators(&mut file_path_buf[..], file_path); let cheap_normalizer = cheap_prefix_normalizer( import_prefix, if use_outdir_relative_path { @@ -1070,18 +1072,8 @@ impl IntermediateOutput { _ => unreachable!(), }; - // normalize windows paths to '/' - // The source slices are reachable only - // through `&Graph` / `&[Chunk]` here; materialising `&mut` from a - // shared-provenance pointer is UB regardless of whether the write - // happens. Copy into a pooled scratch buffer and normalise that. - let file_path: &[u8] = { - let n = file_path.len(); - let dst = &mut file_path_buf[..n]; - dst.copy_from_slice(file_path); - bun_paths::resolve_path::platform_to_posix_in_place::(dst); - dst - }; + let file_path = + Self::to_posix_separators(&mut file_path_buf[..], file_path); let cheap_normalizer = cheap_prefix_normalizer( import_prefix, if use_outdir_relative_path { From 76a0c7c026dd37e721f3921b76914284c5a83062 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 13 Sep 2026 17:41:18 +0000 Subject: [PATCH 07/11] bundler: trim the to_posix_separators comment --- src/bundler/Chunk.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/bundler/Chunk.rs b/src/bundler/Chunk.rs index 62644e243a02..65f3e75ef9b9 100644 --- a/src/bundler/Chunk.rs +++ b/src/bundler/Chunk.rs @@ -607,9 +607,7 @@ impl IntermediateOutput { dst } - /// `path` with Windows separators turned into `/`, copied into `buf`. The - /// source is reachable only through `&Graph` / `&[Chunk]`, so it is never - /// normalized in place. + /// `path` copied into `buf` with Windows separators turned into `/`. fn to_posix_separators<'b>(buf: &'b mut [u8], path: &[u8]) -> &'b [u8] { let dst = &mut buf[..path.len()]; dst.copy_from_slice(path); From b9de0812ba75ebec82a070434e6a7e9cdf211c8b Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 13 Sep 2026 18:10:49 +0000 Subject: [PATCH 08/11] test: say why the round-trip assertion keeps the first hash --- test/bundler/esbuild/css.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/test/bundler/esbuild/css.test.ts b/test/bundler/esbuild/css.test.ts index 9a3c42830b33..0d0c44ccf459 100644 --- a/test/bundler/esbuild/css.test.ts +++ b/test/bundler/esbuild/css.test.ts @@ -1322,6 +1322,7 @@ b { // Every url() in the output has to name a file that was written next to it. const again = await Bun.build({ entrypoints: [api.join("out/entry.css")], outdir: api.join("out2") }); const assets = again.outputs.map(o => basename(o.path)).filter(name => name.endsWith(".png")); + // The second build copies `-.png` to `--.png`. Drop `-.png`. expect(assets.map(name => name.replace(/-[a-z0-9]+\.png$/, "")).sort()).toEqual( ["a\nb-nkpagkva", `${injection}-w49ecq2a`].sort(), ); From 1b4e75f88d387bc258e7ee012de872dadb4a8842 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Sun, 13 Sep 2026 18:24:31 +0000 Subject: [PATCH 09/11] test: run the publicPath escape test on Windows too --- test/bundler/esbuild/css.test.ts | 39 ++++++++++++++++---------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/test/bundler/esbuild/css.test.ts b/test/bundler/esbuild/css.test.ts index 0d0c44ccf459..189de2994926 100644 --- a/test/bundler/esbuild/css.test.ts +++ b/test/bundler/esbuild/css.test.ts @@ -1296,7 +1296,26 @@ b { }); // The final path replaces the placeholder inside a quoted string, so it is - // escaped for one. Windows cannot represent `"` or a line break in a file name. + // escaped for one. + itBundled("css/FileLoaderURLEscapesPublicPath", { + files: { + "/entry.css": `.a { background: url("./plain.png") }`, + "/plain.png": Buffer.alloc(128 * 1024 + 1, "X").toString(), + }, + loader: { + ".png": "file", + }, + publicPath: '");}body{color:red}a{b:url("', + minifyWhitespace: true, + outdir: "/out", + onAfterBundle(api) { + expect(api.readFile("/out/entry.css")).toBe( + String.raw`.a{background:url("\");}body{color:red}a{b:url(\"plain-w49ecq2a.png")}` + "\n", + ); + }, + }); + + // Windows cannot represent `"` or a line break in a file name. describe.skipIf(isWindows)("asset path is escaped for the CSS string it lands in", () => { const injection = 'x");}body{color:red}a{b:url("y'; itBundled("css/FileLoaderURLEscapesFileName", { @@ -1328,24 +1347,6 @@ b { ); }, }); - - itBundled("css/FileLoaderURLEscapesPublicPath", { - files: { - "/entry.css": `.a { background: url("./plain.png") }`, - "/plain.png": Buffer.alloc(128 * 1024 + 1, "X").toString(), - }, - loader: { - ".png": "file", - }, - publicPath: '");}body{color:red}a{b:url("', - minifyWhitespace: true, - outdir: "/out", - onAfterBundle(api) { - expect(api.readFile("/out/entry.css")).toBe( - String.raw`.a{background:url("\");}body{color:red}a{b:url(\"plain-w49ecq2a.png")}` + "\n", - ); - }, - }); }); itBundled("css/IgnoreURLsInAtRulePrelude", { From 86f3fe3793d75808988933681039d4442807ae38 Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 19 Oct 2026 04:22:35 +0000 Subject: [PATCH 10/11] bundler: pick the splice escape per chunk kind, and keep the metafile raw The join chose the CSS escape from a boolean on the chunk it was given. The metafile is built through the same join with a stand-in chunk, so a CSS-only build ran the metafile join with the CSS escape on. The join now takes a SpliceEscape. SpliceEscape::for_chunk is an exhaustive match on the chunk kind, and the metafile calls the new code_for_metafile, which passes Raw. The sizing pass and the writing pass share spliced_path_parts and write_spliced_path, so both see the same bytes, and platform_to_posix_buf borrows the path on POSIX in place of a copy per placeholder. Tests: exact output in place of a whitespace-insensitive match, an @font-face rule, image-set(), [dir] asset naming with an ordinary publicPath, and a file name chosen by a dependency's exports. --- src/bundler/Chunk.rs | 172 +++++++++++------- src/bundler/linker_context/MetafileBuilder.rs | 18 +- src/css/printer.rs | 1 + test/bundler/esbuild/css.test.ts | 47 ++++- 4 files changed, 151 insertions(+), 87 deletions(-) diff --git a/src/bundler/Chunk.rs b/src/bundler/Chunk.rs index e0784fcdfb97..3ac3ceb651c3 100644 --- a/src/bundler/Chunk.rs +++ b/src/bundler/Chunk.rs @@ -510,6 +510,24 @@ fn additional_output_file_index(f: &AdditionalFile) -> usize { } } +/// How the path that replaces a placeholder is written into the output. +#[derive(Clone, Copy)] +enum SpliceEscape { + /// Copied as is. + Raw, + /// The placeholder is the body of a `"..."` CSS string. + CssString, +} + +impl SpliceEscape { + fn for_chunk(chunk: &Chunk) -> Self { + match chunk.content { + Content::Css(_) => Self::CssString, + Content::Javascript(_) | Content::Html => Self::Raw, + } + } +} + impl IntermediateOutput { pub(crate) fn allocator_for_size(_size: usize) -> &'static DynAlloc { // mimalloc serves large allocations via mmap already, so the global @@ -571,27 +589,43 @@ impl IntermediateOutput { dst } - /// `path` copied into `buf` with Windows separators turned into `/`. - fn to_posix_separators<'b>(buf: &'b mut [u8], path: &[u8]) -> &'b [u8] { - let dst = &mut buf[..path.len()]; - dst.copy_from_slice(path); - bun_paths::resolve_path::platform_to_posix_in_place::(dst); - dst - } - - /// In a CSS chunk every placeholder is printed inside a double-quoted string. - fn css_string_escaped_len(path: &[u8]) -> usize { - let mut counter = bun_io::DiscardingWriter::new(); - let _ = bun_css::css_parser::serializer::serialize_string_contents(path, &mut counter); - counter.count + /// The (prefix, path) pair that replaces a placeholder, shared by the sizing and writing pass. + fn spliced_path_parts<'a>( + file_path: &'a [u8], + import_prefix: &'a [u8], + from_chunk_dir: &[u8], + use_outdir_relative_path: bool, + file_path_buf: &'a mut [u8], + relative_platform_buf: &'a mut [u8], + ) -> [&'a [u8]; 2] { + let file_path = bun_paths::resolve_path::platform_to_posix_buf(file_path, file_path_buf); + cheap_prefix_normalizer( + import_prefix, + if use_outdir_relative_path { + file_path + } else { + bun_paths::resolve_path::relative_platform_buf::( + relative_platform_buf, + from_chunk_dir, + file_path, + ) + }, + ) } - /// Caller must ensure `dest` has room for `css_string_escaped_len(path)` bytes. - fn memcpy_css_string_escaped(dest: &mut [u8], path: &[u8]) -> usize { - let mut stream = bun_io::FixedBufferStream::new_mut(dest); - bun_css::css_parser::serializer::serialize_string_contents(path, &mut stream) - .expect("unreachable: sized by css_string_escaped_len"); - stream.pos + /// Writes one of the slices from `spliced_path_parts`. + fn write_spliced_path( + writer: &mut W, + path: &[u8], + escape: SpliceEscape, + ) -> bun_io::Result<()> { + match escape { + SpliceEscape::Raw => writer.write_all(path), + // esbuild copies the path as is, so a `"` in it ends the string. + SpliceEscape::CssString => { + bun_css::css_parser::serializer::serialize_string_contents(path, writer) + } + } } #[allow(clippy::too_many_arguments)] @@ -613,6 +647,7 @@ impl IntermediateOutput { shift_tracking: SourceMapShiftTracking, ) -> Result { let display_size: Option<&mut usize> = display_size.into(); + let escape = SpliceEscape::for_chunk(chunk); match shift_tracking { SourceMapShiftTracking::Enabled => self.code_with_source_map_shifts::( allocator_to_use, @@ -624,6 +659,7 @@ impl IntermediateOutput { display_size, reference_path_style, None, + escape, ), SourceMapShiftTracking::Disabled => self.code_with_source_map_shifts::( allocator_to_use, @@ -635,10 +671,33 @@ impl IntermediateOutput { display_size, reference_path_style, None, + escape, ), } } + /// `code()` for the metafile, where `chunk` only supplies the directory paths are relative to. + pub(crate) fn code_for_metafile( + &mut self, + parse_graph: &Graph, + linker_graph: &LinkerGraph<'_>, + chunk: &Chunk, + chunks: &[Chunk], + ) -> Result { + self.code_with_source_map_shifts::( + None, + parse_graph, + linker_graph, + b"", + chunk, + chunks, + None, + ReferencePathStyle::ImporterRelative, + None, + SpliceEscape::Raw, + ) + } + /// Like `code()` but with standalone HTML support. /// When `standalone_chunk_contents` is provided, chunk piece references are /// resolved to inline code content instead of file paths. Asset references @@ -661,6 +720,7 @@ impl IntermediateOutput { standalone_chunk_contents: &[Option>], ) -> Result { let display_size: Option<&mut usize> = display_size.into(); + let escape = SpliceEscape::for_chunk(chunk); match shift_tracking { SourceMapShiftTracking::Enabled => self.code_with_source_map_shifts::( allocator_to_use, @@ -672,6 +732,7 @@ impl IntermediateOutput { display_size, reference_path_style, Some(standalone_chunk_contents), + escape, ), SourceMapShiftTracking::Disabled => self.code_with_source_map_shifts::( allocator_to_use, @@ -683,12 +744,13 @@ impl IntermediateOutput { display_size, reference_path_style, Some(standalone_chunk_contents), + escape, ), } } #[allow(clippy::too_many_arguments)] - pub(crate) fn code_with_source_map_shifts( + fn code_with_source_map_shifts( &mut self, allocator_to_use: Option<&DynAlloc>, graph: &Graph, @@ -700,6 +762,7 @@ impl IntermediateOutput { display_size: Option<&mut usize>, reference_path_style: ReferencePathStyle, standalone_chunk_contents: Option<&[Option>]>, + escape: SpliceEscape, ) -> Result { // `Graph.input_files` SoA accessors live in `Graph::InputFileColumns`; // `LinkerGraph.files` SoA (`items_entry_point_chunk_index`) lands with @@ -710,7 +773,6 @@ impl IntermediateOutput { graph.input_files.items_unique_key_for_additional_file(); let mut relative_platform_buf = bun_paths::path_buffer_pool::get(); let mut file_path_buf = bun_paths::path_buffer_pool::get(); - let escape_for_css = chunk.content.is_css(); match self { IntermediateOutput::Pieces(pieces) => { let entry_point_chunks_for_scb = linker_graph.files.items_entry_point_chunk_index(); @@ -826,27 +888,18 @@ impl IntermediateOutput { QueryKind::None | QueryKind::ChunkId => unreachable!(), }; - let file_path = - Self::to_posix_separators(&mut file_path_buf[..], file_path); - let cheap_normalizer = cheap_prefix_normalizer( + for part in Self::spliced_path_parts( + file_path, import_prefix, - if use_outdir_relative_path { - file_path - } else { - bun_paths::resolve_path::relative_platform_buf::< - bun_paths::platform::Posix, - false, - >( - &mut relative_platform_buf[..], from_chunk_dir, file_path - ) - }, - ); - for part in cheap_normalizer { - count += if escape_for_css { - Self::css_string_escaped_len(part) - } else { - part.len() - }; + from_chunk_dir, + use_outdir_relative_path, + &mut file_path_buf[..], + &mut relative_platform_buf[..], + ) { + let mut counter = bun_io::DiscardingWriter::new(); + Self::write_spliced_path(&mut counter, part, escape) + .expect("unreachable"); + count += counter.count; } } QueryKind::None => {} @@ -1020,31 +1073,20 @@ impl IntermediateOutput { _ => unreachable!(), }; - let file_path = - Self::to_posix_separators(&mut file_path_buf[..], file_path); - let cheap_normalizer = cheap_prefix_normalizer( + for part in Self::spliced_path_parts( + file_path, import_prefix, - if use_outdir_relative_path { - file_path - } else { - bun_paths::resolve_path::relative_platform_buf::< - bun_paths::platform::Posix, - false, - >( - &mut relative_platform_buf[..], from_chunk_dir, file_path - ) - }, - ); - - for part in cheap_normalizer { - if part.is_empty() { - continue; - } - let written = if escape_for_css { - Self::memcpy_css_string_escaped(remain, part) - } else { - remain[..part.len()].copy_from_slice(part); - part.len() + from_chunk_dir, + use_outdir_relative_path, + &mut file_path_buf[..], + &mut relative_platform_buf[..], + ) { + let written = { + let mut stream = + bun_io::FixedBufferStream::new_mut(&mut *remain); + Self::write_spliced_path(&mut stream, part, escape) + .expect("unreachable"); + stream.pos }; if ENABLE_SOURCE_MAP_SHIFTS { shift.after.advance(&remain[..written]); diff --git a/src/bundler/linker_context/MetafileBuilder.rs b/src/bundler/linker_context/MetafileBuilder.rs index 10c528d91108..9b219b9b44ce 100644 --- a/src/bundler/linker_context/MetafileBuilder.rs +++ b/src/bundler/linker_context/MetafileBuilder.rs @@ -44,7 +44,7 @@ use bun_ast::ExportsKind; use bun_ast::ImportKind; use bun_ast::ImportRecordFlags; -use crate::chunk::{Content as ChunkContent, ReferencePathStyle, SourceMapShiftTracking}; +use crate::chunk::Content as ChunkContent; use crate::options::Loader; use crate::{Chunk, Index, LinkerContext}; @@ -454,19 +454,9 @@ pub(crate) fn generate(c: &mut LinkerContext, chunks: &mut [Chunk]) -> crate::Re )?; // Get final output with all chunk references resolved. - // `code()` takes the dummy chunk and the full slice as `&`, so pass - // `&chunks[0]` directly — overlapping shared borrows are fine. - let code_result = intermediate.code( - None, - parse_graph, - &c.graph, - b"", // no import prefix for metafile - &chunks[0], - chunks, - None, // no display size - ReferencePathStyle::ImporterRelative, - SourceMapShiftTracking::Disabled, - )?; + // `code_for_metafile()` takes the dummy chunk and the full slice as `&`, so + // pass `&chunks[0]` directly — overlapping shared borrows are fine. + let code_result = intermediate.code_for_metafile(parse_graph, &c.graph, &chunks[0], chunks)?; Ok(code_result.buffer) } diff --git a/src/css/printer.rs b/src/css/printer.rs index df0ef4a89385..1fa5877b1ac9 100644 --- a/src/css/printer.rs +++ b/src/css/printer.rs @@ -405,6 +405,7 @@ impl<'a> Printer<'a> { pub(crate) struct ResolvedImportRecordUrl<'u> { pub(crate) url: &'u [u8], /// A unique key the bundler replaces with the final output path after printing. + /// It is always printed quoted, as esbuild does since 0.19.5 (evanw/esbuild#3410). pub(crate) is_placeholder: bool, } diff --git a/test/bundler/esbuild/css.test.ts b/test/bundler/esbuild/css.test.ts index 90cf660c6267..da784838d1b6 100644 --- a/test/bundler/esbuild/css.test.ts +++ b/test/bundler/esbuild/css.test.ts @@ -1277,6 +1277,8 @@ b { .b { background: url("./a(b).png") } .c { --bg: url("./my image.png") } .d { background: url("./plain.png") } + .e { background-image: image-set("./my image.png" 1x, url("./a(b).png") 2x) } + @font-face { font-family: I; src: url("./my image.png") } `, "/my image.png": Buffer.alloc(128 * 1024 + 1, "Z").toString(), "/a(b).png": Buffer.alloc(128 * 1024 + 1, "Y").toString(), @@ -1288,11 +1290,32 @@ b { minifyWhitespace: true, outdir: "/out", onAfterBundle(api) { - api - .expectFile("/out/entry.css") - .toEqualIgnoringWhitespace( - '.a{background:url("./my image-mrrzcz3w.png")}.b{background:url("./a(b)-nkpagkva.png")}.c{--bg:url("./my image-mrrzcz3w.png")}.d{background:url("./plain-w49ecq2a.png")}', - ); + expect(api.readFile("/out/entry.css")).toBe( + '.a{background:url("./my image-mrrzcz3w.png")}.b{background:url("./a(b)-nkpagkva.png")}.c{--bg:url("./my image-mrrzcz3w.png")}.d{background:url("./plain-w49ecq2a.png")}' + + '.e{background-image:-webkit-image-set(url("./my image-mrrzcz3w.png") 1x,url("./a(b)-nkpagkva.png") 2x);background-image:image-set("./my image-mrrzcz3w.png" 1x,"./a(b)-nkpagkva.png" 2x)}' + + '@font-face{font-family:I;src:url("./my image-mrrzcz3w.png")}\n', + ); + }, + }); + + // The whole output path is copied over the placeholder, so `[dir]` carries a + // space just like a file name does. An ordinary publicPath is left as it is. + itBundled("css/FileLoaderURLAssetNamingDirAndPublicPath", { + files: { + "/entry.css": `@font-face { font-family: I; src: url("./Brand Fonts/inter.woff2") }`, + "/Brand Fonts/inter.woff2": Buffer.alloc(128 * 1024 + 1, "W").toString(), + }, + loader: { + ".woff2": "file", + }, + assetNaming: "[dir]/[name]-[hash].[ext]", + publicPath: "https://cdn.example.com/a/", + minifyWhitespace: true, + outdir: "/out", + onAfterBundle(api) { + expect(api.readFile("/out/entry.css")).toBe( + '@font-face{font-family:I;src:url("https://cdn.example.com/a/Brand Fonts/inter-tny5bcvp.woff2")}\n', + ); }, }); @@ -1318,16 +1341,23 @@ b { // Windows cannot represent `"` or a line break in a file name. describe.skipIf(isWindows)("asset path is escaped for the CSS string it lands in", () => { + // The stylesheet only says "icons/logo.png". The package's `exports` picks the file name. const injection = 'x");}body{color:red}a{b:url("y'; itBundled("css/FileLoaderURLEscapesFileName", { files: { - // `\22 ` is `"` and `\a ` is a line feed. + // `\a ` is a line feed. "/entry.css": String.raw` .a { background: url("./a\a b.png") } - .b { background: url("./x\22 );}body{color:red}a{b:url(\22 y.png") } + .b { background: url("icons/logo.png") } + .c { mask-image: image-set("./a\a b.png" 1x, "icons/logo.png" 2x) } `, "/a\nb.png": Buffer.alloc(128 * 1024 + 1, "Y").toString(), - [`/${injection}.png`]: Buffer.alloc(128 * 1024 + 1, "X").toString(), + "/node_modules/icons/package.json": JSON.stringify({ + name: "icons", + version: "1.0.0", + exports: { "./logo.png": `./${injection}.png` }, + }), + [`/node_modules/icons/${injection}.png`]: Buffer.alloc(128 * 1024 + 1, "X").toString(), }, loader: { ".png": "file", @@ -1337,6 +1367,7 @@ b { async onAfterBundle(api) { expect(api.readFile("/out/entry.css")).toBe( String.raw`.a{background:url("./a\a b-nkpagkva.png")}.b{background:url("./x\");}body{color:red}a{b:url(\"y-w49ecq2a.png")}` + + String.raw`.c{mask-image:image-set("./a\a b-nkpagkva.png" 1x,"./x\");}body{color:red}a{b:url(\"y-w49ecq2a.png" 2x)}` + "\n", ); // Every url() in the output has to name a file that was written next to it. From dc3d17b27047a7f6c23129f1e058db0a4209da4c Mon Sep 17 00:00:00 2001 From: robobun <117481402+robobun@users.noreply.github.com> Date: Mon, 19 Oct 2026 04:24:34 +0000 Subject: [PATCH 11/11] bundler: trim two comments --- src/bundler/linker_context/MetafileBuilder.rs | 2 -- src/css/printer.rs | 3 +-- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/src/bundler/linker_context/MetafileBuilder.rs b/src/bundler/linker_context/MetafileBuilder.rs index 9b219b9b44ce..ef3b83d6d47f 100644 --- a/src/bundler/linker_context/MetafileBuilder.rs +++ b/src/bundler/linker_context/MetafileBuilder.rs @@ -454,8 +454,6 @@ pub(crate) fn generate(c: &mut LinkerContext, chunks: &mut [Chunk]) -> crate::Re )?; // Get final output with all chunk references resolved. - // `code_for_metafile()` takes the dummy chunk and the full slice as `&`, so - // pass `&chunks[0]` directly — overlapping shared borrows are fine. let code_result = intermediate.code_for_metafile(parse_graph, &c.graph, &chunks[0], chunks)?; Ok(code_result.buffer) diff --git a/src/css/printer.rs b/src/css/printer.rs index 1fa5877b1ac9..260356c7efa9 100644 --- a/src/css/printer.rs +++ b/src/css/printer.rs @@ -404,8 +404,7 @@ impl<'a> Printer<'a> { pub(crate) struct ResolvedImportRecordUrl<'u> { pub(crate) url: &'u [u8], - /// A unique key the bundler replaces with the final output path after printing. - /// It is always printed quoted, as esbuild does since 0.19.5 (evanw/esbuild#3410). + /// A unique key the bundler fills in after printing. Always quoted, as in esbuild#3410. pub(crate) is_placeholder: bool, }