diff --git a/src/bundler/bundle_v2.rs b/src/bundler/bundle_v2.rs index a2a31fd5e974..b77e9e566644 100644 --- a/src/bundler/bundle_v2.rs +++ b/src/bundler/bundle_v2.rs @@ -4265,14 +4265,9 @@ pub mod bv2_impl { .to_vec() .into_boxed_slice(); } - let mut v = Vec::new(); template - .print( - &mut v, - !self.transpiler.options.compile_mode.is_executable(), - ) - .expect("oom"); - v.into_boxed_slice() + .render(!self.transpiler.options.compile_mode.is_executable()) + .into_boxed_slice() }; let loader = loaders[index]; diff --git a/src/bundler/linker_context/generateChunksInParallel.rs b/src/bundler/linker_context/generateChunksInParallel.rs index fd5175c2aba4..3c401798e4ad 100644 --- a/src/bundler/linker_context/generateChunksInParallel.rs +++ b/src/bundler/linker_context/generateChunksInParallel.rs @@ -388,17 +388,11 @@ pub(crate) fn generate_chunks_in_parallel( let chunk = &mut chunks[index]; chunk.template.placeholder.hash = Some(hash.digest()); - let mut rel_path: Vec = Vec::new(); - // Use the byte-writer (`PathTemplate::print`) directly — - // routing through `Display`/`write!` goes via `from_utf8_lossy`, - // which would replace non-UTF-8 dir bytes with U+FFFD and corrupt - // the output path. // Disk output sanitizes leading `..`; `--compile` keeps it so // runtime bunfs references to out-of-root entrypoints resolve. - chunk + let mut rel_path: Vec = chunk .template - .print(&mut rel_path, !c.options.compile_mode.is_executable()) - .expect("write to Vec"); + .render(!c.options.compile_mode.is_executable()); path::resolve_path::platform_to_posix_in_place::(&mut rel_path); if path_names_map.get_or_put(&rel_path)?.found_existing { @@ -411,18 +405,6 @@ pub(crate) fn generate_chunks_in_parallel( continue; } - // resolve any /./ and /../ occurrences - // use resolvePosix since we asserted above all seps are '/' - #[cfg(windows)] - if strings::index_of(&rel_path, b"/./").is_some() { - let mut buf = bun_paths::PathBuffer::uninit(); - let rel_path_fixed: Box<[u8]> = Box::from(&*path::resolve_path::normalize_buf::< - path::platform::Posix, - >(&rel_path, &mut buf)); - chunk.final_rel_path = rel_path_fixed; - continue; - } - chunk.final_rel_path = rel_path.into_boxed_slice(); } diff --git a/src/bundler/options.rs b/src/bundler/options.rs index 66e0c833cd85..a4d8e6b7947f 100644 --- a/src/bundler/options.rs +++ b/src/bundler/options.rs @@ -2181,6 +2181,70 @@ pub fn write_sanitized_parent_dirs( } } +/// Drops every `.` segment after the first one, in place: `././a.js` becomes +/// `./a.js` and `./static/./a.js` becomes `./static/a.js`. The first segment is +/// kept because a leading `./` is the shape every default template renders to +/// (`[dir]` renders as `.` at the root so that `[dir]/[name].[ext]` yields +/// `./a.js`, not `/a.js`). +fn remove_redundant_dot_segments(path: &mut Vec) { + // `write_replacing_slashes_on_windows` emits native separators; on POSIX a + // `\` is an ordinary file name byte. + const SEPARATORS: &[u8] = if cfg!(windows) { b"/\\" } else { b"/" }; + let mut read = 0; + let mut write = 0; + let mut is_first_segment = true; + while read < path.len() { + // `next` is the start of the following segment, so `read..next` is this + // segment plus the separator that ends it (if any). + let (segment_end, next) = match strings::index_of_any(&path[read..], SEPARATORS) { + Some(i) => (read + i, read + i + 1), + None => (path.len(), path.len()), + }; + if is_first_segment || &path[read..segment_end] != b"." { + if write != read { + path.copy_within(read..next, write); + } + write += next - read; + } + is_first_segment = false; + read = next; + } + path.truncate(write); +} + +#[cfg(test)] +#[test] +fn remove_redundant_dot_segments_keeps_only_the_leading_one() { + fn run(path: &[u8]) -> Vec { + let mut out = path.to_vec(); + remove_redundant_dot_segments(&mut out); + out + } + // A user `[dir]/[name].[ext]` gets a `./` prefix from the CLI / `Bun.build` + // and `[dir]` renders as `.` at the root. + assert_eq!(run(b"././a.js"), b"./a.js"); + assert_eq!(run(b"./static/./a.js"), b"./static/a.js"); + assert_eq!(run(b"static/./a.js"), b"static/a.js"); + assert_eq!(run(b"./././a.js"), b"./a.js"); + // Everything the default templates render is already in canonical form. + assert_eq!(run(b"./a.js"), b"./a.js"); + assert_eq!(run(b"./chunk-abc123.js"), b"./chunk-abc123.js"); + assert_eq!(run(b"pages/a.html"), b"pages/a.html"); + assert_eq!(run(b"./pages/a.html"), b"./pages/a.html"); + assert_eq!(run(b"a.js"), b"a.js"); + assert_eq!(run(b""), b""); + // `..`, `_.._` and dotfiles are real segments. + assert_eq!(run(b"../a.js"), b"../a.js"); + assert_eq!(run(b"./../a.js"), b"./../a.js"); + assert_eq!(run(b"./_.._/a.js"), b"./_.._/a.js"); + assert_eq!(run(b"././.env"), b"./.env"); + assert_eq!(run(b"./.well-known/./a.txt"), b"./.well-known/a.txt"); + #[cfg(windows)] + assert_eq!(run(br".\.\a.js"), br".\a.js"); + #[cfg(not(windows))] + assert_eq!(run(br"./.\a.js"), br"./.\a.js"); +} + #[cfg(test)] #[test] fn write_sanitized_parent_dirs_rewrites_every_dotdot_segment() { @@ -2320,7 +2384,7 @@ impl PathTemplate { placeholder: PlaceholderConst::DEFAULT, }; - pub(crate) fn print( + fn print( &self, writer: &mut W, sanitize_parent_dirs: bool, @@ -2336,6 +2400,25 @@ impl PathTemplate { sanitize_parent_dirs, ) } + + /// Renders the outdir-relative output path of a chunk or asset. + /// + /// Works on raw bytes rather than `Display`, which would go through + /// `from_utf8_lossy` and corrupt non-UTF-8 directory names. + /// + /// The CLI and `Bun.build` prefix user templates with `./`, and `[dir]` is + /// `.` for a source at the root, so a user `[dir]/[name].[ext]` prints as + /// `././a.js` (and `static/[dir]/...` as `./static/./a.js`). The redundant + /// segments are removed here, before the path reaches the HTML import + /// manifest, the metafile, public path joins and standalone executable + /// keys, so those all see the `./a.js` the default templates produce. + pub(crate) fn render(&self, sanitize_parent_dirs: bool) -> Vec { + let mut path = Vec::new(); + self.print(&mut path, sanitize_parent_dirs) + .expect("writing to a Vec cannot fail"); + remove_redundant_dot_segments(&mut path); + path + } } #[derive(Debug, Clone, Default)] @@ -2417,9 +2500,7 @@ impl core::fmt::Display for PathTemplateConst { impl core::fmt::Display for PathTemplate { fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { - let mut buf = Vec::::new(); - self.print(&mut buf, true).map_err(|_| core::fmt::Error)?; - write!(f, "{}", bstr::BStr::new(&buf)) + write!(f, "{}", bstr::BStr::new(&self.render(true))) } } diff --git a/test/bundler/bundler_naming.test.ts b/test/bundler/bundler_naming.test.ts index fe2f1585825b..5f858137ed62 100644 --- a/test/bundler/bundler_naming.test.ts +++ b/test/bundler/bundler_naming.test.ts @@ -303,6 +303,42 @@ describe("bundler", () => { }, ], }); + // The naming option gets a "./" prefix and `[dir]` renders as "." for a file at + // the root, so templates starting with `[dir]` used to render "././a.js". That + // string is what the metafile reports and what gets appended to publicPath. + itBundled("naming/DirTemplateAtRoot", { + backend: "api", + files: { + "/a.js": /* js */ ` + import { shared } from "./shared.js"; + console.log("a", shared); + `, + "/sub/c.js": /* js */ ` + import { shared } from "../shared.js"; + console.log("c", shared); + `, + "/shared.js": /* js */ ` + export const shared = 1; + `, + }, + entryPoints: ["/a.js", "/sub/c.js"], + outputPaths: ["/out/a.js", "/out/sub/c.js"], + splitting: true, + chunkNaming: "[dir]/[name]-[hash].[ext]", + publicPath: "https://cdn.example/", + metafile: true, + onAfterBundle(api) { + const outputs = Object.keys(JSON.parse(api.readFile("metafile.json")).outputs); + expect(outputs).toHaveLength(3); + expect(outputs).toContain("./a.js"); + expect(outputs).toContain("./sub/c.js"); + const chunk = outputs.find(key => key !== "./a.js" && key !== "./sub/c.js")!; + expect(chunk).toMatch(/^\.\/[^./][^/]*\.js$/); + + const specifier = api.readFile("out/a.js").match(/from "(https:\/\/cdn\.example\/[^"]*)"/)![1]; + expect(specifier).toBe("https://cdn.example/" + chunk.slice("./".length)); + }, + }); // A non-ASCII ID_Continue basename char is preserved in the generated // CommonJS wrapper symbol, not replaced per-code-point (nor per-UTF-8-byte, // which once regressed to `require_caf__utils`). diff --git a/test/bundler/html-import-manifest.test.ts b/test/bundler/html-import-manifest.test.ts index 2867fd593d3d..b24287c0288b 100644 --- a/test/bundler/html-import-manifest.test.ts +++ b/test/bundler/html-import-manifest.test.ts @@ -1,6 +1,6 @@ import { describe, expect, test } from "bun:test"; import { tempDir } from "harness"; -import { readFileSync, writeFileSync } from "node:fs"; +import { readdirSync, readFileSync, writeFileSync } from "node:fs"; import { join } from "node:path"; import { itBundled } from "./expectBundled"; @@ -360,6 +360,107 @@ console.log("About manifest:", aboutHtml); }, }); + // Both `bun build --*-naming` and `Bun.build({ naming })` prefix the template + // with "./", and `[dir]` is "." for a file at the root, so a template starting + // with `[dir]` used to produce "././index.html" (and "static/[dir]/..." used to + // produce "./static/./favicon.svg") in `index` and `files[].path`. The default + // templates produce "./index.html"; custom ones must match. + const dirTemplateFiles = { + "/server.js": ` +import index from "./index.html"; +import about from "./pages/about.html"; +export { index, about }; +`, + "/index.html": ` + + + + + + + +`, + "/styles.css": `body { margin: 0; }`, + "/favicon.svg": ``, + "/app.js": `console.log("app");`, + "/pages/about.html": ` + + + + + + +`, + "/pages/logo.svg": ``, + "/pages/about-page.js": `console.log("about");`, + }; + + function readManifestPaths(serverCode: string) { + const manifests = [...serverCode.matchAll(/__jsonParse\("(.+?)"\)/gs)].map( + match => JSON.parse(JSON.parse('"' + match[1] + '"')) as { index: string; files: Array<{ path: string }> }, + ); + expect(manifests).toHaveLength(2); + const normalize = (p: string) => + p + // Asset paths are still printed with the native separator on Windows; + // this test is only about the "." segments. + .replaceAll("\\", "/") + .replace(/-[a-z0-9]{8}\./, "-HASH."); + return manifests.map(manifest => ({ + index: normalize(manifest.index), + files: manifest.files.map(file => normalize(file.path)), + })); + } + + // `Bun.build` (itBundled's api backend passes naming.entry = "[dir]/[name].[ext]" itself). + itBundled("html-import/manifest-paths-with-dir-template-api", { + outdir: "out/", + backend: "api", + files: dirTemplateFiles, + entryPoints: ["/server.js"], + target: "bun", + chunkNaming: "[dir]/[name]-[hash].[ext]", + assetNaming: "[dir]/[name]-[hash].[ext]", + onAfterBundle(api) { + expect(readManifestPaths(api.readFile("out/server.js"))).toEqual([ + { + index: "./index.html", + files: ["./index-HASH.js", "./index.html", "./index-HASH.css", "./favicon-HASH.svg"], + }, + { + index: "./pages/about.html", + files: ["./pages/about-HASH.js", "./pages/about.html", "./pages/logo-HASH.svg"], + }, + ]); + }, + }); + + // `bun build --entry-naming/--chunk-naming/--asset-naming`. + itBundled("html-import/manifest-paths-with-dir-template-cli", { + outdir: "out/", + backend: "cli", + files: dirTemplateFiles, + entryPoints: ["/server.js"], + target: "bun", + entryNaming: "[dir]/[name]-[hash].[ext]", + chunkNaming: "[dir]/[name]-[hash].[ext]", + assetNaming: "static/[dir]/[name]-[hash].[ext]", + onAfterBundle(api) { + const serverFile = readdirSync(api.join("out")).find(name => /^server-[a-z0-9]{8}\.js$/.test(name)); + expect(serverFile).toBeDefined(); + expect(readManifestPaths(api.readFile(join("out", serverFile!)))).toEqual([ + { + index: "./index-HASH.html", + files: ["./index-HASH.js", "./index-HASH.html", "./index-HASH.css", "./static/favicon-HASH.svg"], + }, + { + index: "./pages/about-HASH.html", + files: ["./pages/about-HASH.js", "./pages/about-HASH.html", "./static/pages/logo-HASH.svg"], + }, + ]); + }, + }); + // The HTML chunk's etag must change when only a referenced JS/CSS chunk // changes; otherwise the browser 304s to a body that points at chunks the // server no longer has.