bundler: stop rendering "././" output paths from naming templates that start with [dir] - #38366
bundler: stop rendering "././" output paths from naming templates that start with [dir]#38366robobun wants to merge 1 commit into
Conversation
The CLI and Bun.build prefix user naming templates with "./", and [dir] renders as "." for a file at the root, so a template starting with [dir] rendered "././a.js" (and "static/[dir]/..." rendered "./static/./a.js"). That raw string was stored as the chunk's final_rel_path and the asset's dest_path, so it showed up in the HTML import manifest, the metafile, public path joins and the build summary. Windows had a separate partial fix that fully normalized such paths instead, dropping the leading "./" and diverging from the other platforms. PathTemplate::render now renders chunk and asset paths and removes every "." segment after the first one, so custom templates produce the same "./a.js" shape as the default templates on every platform. The Windows-only normalization is removed since the paths it matched can no longer occur.
|
Warning Review limit reached
Next review available in: 41 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
Comment |
|
Status: fix pushed, waiting on CI. Reproduced on bun 1.4.0 with Tests: |
There was a problem hiding this comment.
LGTM — targeted normalization of rendered naming-template paths, thoroughly tested.
What was reviewed:
remove_redundant_dot_segments: segment walk keeps first segment, drops later.segments;copy_withinbounds andindex_of_any(Option<usize>) arithmetic checked;../dotfiles///correctly preserved per unit test.PathTemplate::printmade module-private — grep confirms onlyrenderand the const-variantDisplaystill call it; both external render sites (chunks ingenerateChunksInParallel.rs, assets inbundle_v2.rs) now go throughrender.- Removed
#[cfg(windows)]normalize block: it only fired on/./and additionally collapsed../leading./, so Windows diverged from POSIX; new path is platform-consistent and PR reportsnaming/WithPathTraversalstill passes on Windows. - New tests cover CLI + API, root + nested, chunks + assets,
[dir]/...andstatic/[dir]/..., plus metafile keys and publicPath specifiers.
Extended reasoning...
Overview
This PR fixes rendered bundler output paths that contained redundant . segments (././a.js, ./static/./a.js) when a naming template starts with or embeds [dir] and the source is at the project root. It adds a small in-place byte helper remove_redundant_dot_segments in src/bundler/options.rs, wraps the existing PathTemplate::print in a new render that applies the helper, makes print module-private so nothing bypasses it, and rewires the two render sites (generateChunksInParallel.rs for chunks, bundle_v2.rs for copied assets). A Windows-only normalize_buf block that fired on /./ is removed because render now handles . segments uniformly.
Security risks
None. The change is pure string normalization on paths the bundler itself constructed; it removes segments the OS already treats as no-ops, so which file is written is unchanged. .. segments are explicitly left alone (the existing write_sanitized_parent_dirs still guards outdir escape for disk output), and no user-controlled data reaches new sinks.
Level of scrutiny
Moderate. The rendered path feeds several user-visible surfaces (HTML import manifest, metafile keys, publicPath joins, --compile embedded keys, build summary), so a wrong normalization would be widely visible — but the transformation is narrow (drop non-leading . segments only) and cannot change filesystem resolution. The helper is ~25 lines of straightforward two-pointer byte copying with a unit test covering the exact shapes the templates produce plus adversarial cases (.., _.._, dotfiles, .well-known, Windows \\). I traced strings::index_of_any to confirm it returns Option<usize>, so read + i is well-typed, and grepped for other PathTemplate::print callers to confirm making it private breaks nothing.
Other factors
The removed Windows block was itself inconsistent: it ran a full posix normalize (dropping leading ./ and collapsing ..) but only when /./ appeared, so the same template rendered differently on Windows vs POSIX and differently on Windows depending on whether [dir] happened to be at the root. The new behavior is platform-uniform and matches what POSIX already did. The PR description documents verification on a Windows debug build (including naming/WithPathTraversal, which exercises literal .. in a template) and 14 existing bundler test suites. Three new integration tests cover both Bun.build and CLI entry points, root and nested sources, and both the [dir]/... and prefix/[dir]/... shapes; naming/DirTemplateAtRoot additionally asserts the metafile key and the publicPath-joined import specifier. Chunk hashes are unaffected because the content hash covers template text, not the rendered path. No outstanding reviewer comments.
|
Updated 2:22 AM PT - Aug 14th, 2026
✅ @robobun, your commit f615195502e8f85934cd7806a7605b6a3ba4ea67 passed in 🧪 To try this PR locally: bunx bun-pr 38366That installs a local version of the PR into your bun-38366 --bun |
Problem
[dir](bun build --entry-naming "[dir]/[name].[ext]", orBun.build({ naming: { entry: "[dir]/[name].[ext]" } })) embeds"index":"././home.html"and"path":"././home-<hash>.js"in the HTML import manifest. The default templates give./home.html. Same for--chunk-namingand--asset-naming, andstatic/[dir]/...gives./static/./home.html."././server.js"), the value appended to--public-path(https://cdn.example/./a-<hash>.js, becausecheap_prefix_normalizerstrips one./), thebun buildsummary (./server.js), and the embedded file keys of--compile.src/runtime/cli/Arguments.rs:2571andsrc/runtime/api/JSBundler.rs:998prefix user templates with./, andpath_template_print(src/bundler/options.rs) renders[dir]as.for a file at the root, so./[dir]/[name].[ext]renders as././a.js. The rendered bytes are stored verbatim aschunk.final_rel_path(src/bundler/linker_context/generateChunksInParallel.rs) and as the asset'sdest_path(src/bundler/bundle_v2.rs,process_files_to_copy);HTMLImportManifest.rsand the other consumers copy them through.#[cfg(windows)]block ingenerateChunksInParallel.rsthat ran a full normalize whenever/./appeared, which also drops the leading./: the same build printedhome.htmlon Windows,././home.htmlon Linux, and./home.htmlwith the default templates.Fix
PathTemplate::renderrenders the template and removes every.segment after the first one; both render sites (chunks and copied assets) go through it, andprintbecomes private so nothing can bypass it. The Windows-only block is deleted because a/./can no longer reach it../xis the shape every default template renders (./chunk-[hash].[ext],./[name]-[hash].[ext], and[dir]/[name].[ext]at the root), and the existing manifest and metafile snapshots encode it. Custom templates now render the same shape on every platform..segment never changes which file is written (the OS resolves it the same way); it only changes the string handed to the manifest, metafile, public path join, summary and--compilekeys, which now all agree with the defaults.--compile:StandaloneModuleGraph.rskeys the embedded files by the samedest_pathstring the manifest prints, so the two must be derived from one path...,_.._,//and separators are left alone.--compilerelies on a leading..surviving,naming/WithPathTraversalrelies on a literal..being left to the filesystem, and asset paths on Windows still carry\(that is a separate bug, bundler: emit posix-relative paths in the HTML-import manifest on Windows #34557).test/bundler/html-import-manifest.test.ts(manifest-paths-with-dir-template-api/-cli): root and nested HTML imports, JS/CSS/HTML chunks and assets,[dir]/...andstatic/[dir]/...templates, through bothBun.buildand the CLI flags. Fail on the release binary with././index.html/./static/./favicon-….svg, pass withbun bd test.test/bundler/bundler_naming.test.ts(naming/DirTemplateAtRoot): metafile keys and thepublicPathimport specifier. Fails on the release binary with././a.js, passes withbun bd test.options.rs, run withcargo miri test -p bun_bundler --lib(the bundler crate's unit tests cannot link outside miri).bun bd testonbundler_naming,html-import-manifest,bun-serve-html-manifest,metafile,bundler_html,bundler_html_server,bundler_files,bundler_loader,bundler_edgecase,bun-build-api,bundler_splitting,bundler_compile_splitting,compile-asset-bunfs,standalone: all green../home.htmllike the other platforms,.\.\icon-….pngbecomes.\icon-….png, andnaming/EntryNamingTemplate1,naming/WithPathTraversal,naming/ImplicitOutbase2,naming/DirTemplateAtRootpass without the removed block (itBundled tests have to be forced to run there, see test/bundler: stop silently dropping every itBundled test on Windows #34552).Background
--entry-naming,--chunk-naming,--asset-naming,Bun.build({ naming })) describe output paths with[dir],[name],[hash],[ext]placeholders.[dir]is the source file's directory relative to the project root; for a file at the root it is empty and is rendered as.so that[dir]/[name].[ext]gives./a.jsrather than/a.js.final_rel_path(chunks) anddest_path(copied assets) are the outdir-relative output paths computed once per build after hashing. The HTML import manifest, the metafile, the import paths printed into other chunks, the build summary and the--compilefile table all read them.import home from "./home.html":indexis the HTML file's output path andfiles[]lists every output the page needs, each with its outdir-relativepath.Bun.serveturns those paths into routes, and users readfiles[].pathdirectly (for example to upload or precompress the files).Repro, before and after
bun 1.4.0:
This branch:
Same build with the default templates, on both versions:
./home.html [ "./home-…js", "./home.html" ].