bake: load files outside the bundle by their Windows path during prerendering - #39092
bake: load files outside the bundle by their Windows path during prerendering#39092robobun wants to merge 4 commits into
Conversation
…endering A production route that does import(join(import.meta.dir, "x.mjs")) at render time hands BakeProdResolve a drive path. The posix join used for bundle keys does not recognize it as absolute and appends it to the referrer's bundle directory, and BakeToWindowsPath then returned a \\?\ prefixed path, which the module loader cuts at the ? as a query string, so the build failed with EINVAL reading "\\". Resolve against disk paths with Windows path semantics and key them the way file: URLs spell them (bake:/C:/a/x.mjs, bake://server/share/x.mjs), and have BakeToWindowsPath turn such a key back into a plain Win32 path for the loader. Bundle keys keep the posix join.
|
Warning Review limit reached
Next review available in: 36 seconds 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 (2)
Comment |
|
Updated 10:05 AM PT - Aug 15th, 2026
❌ @robobun, your commit d39457b has some failures in 🧪 To try this PR locally: bunx bun-pr 39092That installs a local version of the PR into your bun-39092 --bun |
|
Status
|
… the drive letter
There was a problem hiding this comment.
I re-reviewed after the follow-up commits and didn't find any bugs — the earlier stdout: "pipe" nit is addressed (now "ignore"), and the Rust changes look internally consistent. Because this introduces a new Windows key-spelling scheme (bake:/C:/..., bake://server/share/...) with a fixed-point invariant that only Windows CI exercises, a human look at the design would still be worthwhile.
What was reviewed:
resolve_disk_keyfixed-point:/C:/...and//server/...decode →is_disk_path→ re-encode to the same key; bundle keys (/_bun/...) fall through to the unchanged posix join.BakeToWindowsPathinverse:/C:/a→C:\\a,//server/s/x→\\\\server\\s\\x; no more\\\\?\\prefix that the loader would split on?.- Overflow path uses
join_abs_string_buf_checkedand throws + returnsBunString::dead(), matching the existing non-relative-import error pattern inBakeProdResolve. - Non-Windows: only a local rename (
after_scheme→referrer_key_path) and#[cfg(unix)]→#[cfg(not(windows))]on the panic arm; behavior unchanged.
Extended reasoning...
Overview
The PR fixes bun build --app prerendering on Windows when a route dynamically imports a file the bundler never saw (via import(join(import.meta.dir, ...))). Two extern "C" functions in src/runtime/bake/production.rs are changed: BakeProdResolve gains a #[cfg(windows)] branch that recognizes drive-qualified and UNC paths and resolves them with join_abs::<platform::Windows>, encoding the result as bake:/C:/... or bake://server/share/...; BakeToWindowsPath is rewritten to invert that encoding into a plain Win32 path (previously it produced \\\\?\\..., which the module loader then truncated at the first ?). Three small #[cfg(windows)] helpers (key_path_to_disk_path, is_disk_path, resolve_disk_key) are added. A new test in test/bake/dev/production.test.ts builds a fixture that imports an out-of-bundle .mjs (which itself has a relative import) via two different specifier spellings and asserts the rendered HTML.
Security risks
None identified. The input is a module specifier during a local build's prerender step; the resolved path is handed to the regular module loader, which is the same loader that already reads arbitrary user paths during bun run. The overflow case (join_abs_string_buf_checked returning None) throws instead of indexing past the buffer.
Level of scrutiny
Moderate-to-high. The change is small and entirely #[cfg(windows)]-gated (non-Windows behavior is byte-identical apart from a renamed local), but it encodes a non-obvious invariant: the key spelling must be a fixed point of BakeProdResolve because JSC's module loader re-resolves the returned key with referrer bake:/. The PR description walks through why /C:/... and //server/... satisfy this, and I traced it (key_path_to_disk_path strips the leading / before a drive letter; UNC //server/share already has a Windows volume so is_disk_path is true; both re-encode to themselves). This is the kind of platform-specific path logic where a maintainer familiar with the bake loader hooks should confirm the encoding choice is the one they want long-term.
Other factors
- My prior inline nit (undrained
stdout: "pipe") was addressed in b6f36de — the test now usesstdout: "ignore". That thread is resolved. - The comment-cop bot fired again on the two remaining doc comments (lines ~1322 and ~1357) after the last trim commit. Those comments are 2 and 4 lines respectively and document the encoding invariant rather than justify a workaround, so I don't consider them blocking, but the author may want to trim once more or accept the bot noise.
- The new test passes on Linux with or without the fix (posix disk paths were never affected), so the failing-before evidence is Windows-only. The PR description reports it was verified on Windows Server 2019 against both the current canary (fails) and this branch's debug build (passes).
- No CODEOWNERS entry covers
src/runtime/bake/.
Problem
bun build --appfails to prerender a route that loads a file the bundler did not see, such asawait import(join(import.meta.dir, "../extra/banner.mjs"))inside a page component. The build printserror: EINVAL reading "\\"followed byRoute "pages\index.tsx" cannot be pre-rendered to a static page.and exits 1. The same project builds on Linux and macOS. Reproduced with the current canary on Windows Server 2019.import.meta.diris inlined as the source file's directory, so on Windows the specifier is a drive path (C:\app\extra\banner.mjs).BakeProdResolve(src/runtime/bake/production.rs) turns every specifier into a key withjoin_abs::<Posix>(dirname(referrer key), specifier); a drive path is not posix-absolute, so it is appended under the referrer's bundle directory:bake:/_bun/C:/app/extra/banner.mjs.bakeModuleLoaderFetch(src/runtime/bake/BakeGlobalObject.cpp) stripsbake:and callsBakeToWindowsPath(production.rs), which usedto_w_path_normalize_auto_extendand returned an extended-length path,\\?\_bun\C:\app\.... The regular loader cuts a specifier at its first?as a query string (normalize_specifier_for_loaderin src/runtime/jsc_hooks.rs), so the file it tried to read was\\. Because of this second defect, fixing the key alone would still have failed.Fix
BakeProdResolvenow resolves with Windows path semantics whenever the referrer's key or the specifier is a disk path (drive-qualified or UNC), using the samejoin_absthe rest of the runtime uses forpath.resolve, and spells the result into the key namespace the wayfile:URLs do:C:\app\extra\banner.mjsbecomesbake:/C:/app/extra/banner.mjs,\\server\share\x.mjsbecomesbake://server/share/x.mjs. Bundled modules (bake:/_bun/...) take the unchanged posix join, and non-Windows builds are unchanged apart from a renamed local (the new code is#[cfg(windows)]).BakeToWindowsPathnow inverts that spelling (/C:/app/...->C:\app\...,//server/share/...->\\server\share\...) and returns a plain Win32 path, which is what the loader can read.import()a second time (referrerbake:/, specifier = the key's path), so the key spelling must be a fixed point ofBakeProdResolve./C:/...decodes to a disk path and resolves back to itself;//server/share/...is already a UNC path and does the same; bundle paths have no volume and keep taking the posix join. The debug log in the details below shows the keys that come out.bake:/C:/...key as the referrer. Decoding the referrer before takingdirnamekeeps them on disk as well (./detail.mjs,../extra2/sibling.mjsin the test and log below); with the posix join they would only have worked by accident for drive paths and not at all for UNC.bake:/, which is the prefix all three loader hooks dispatch on.BakeProdResolvedoes, rather than indexing out of the buffer.path.joinspecifier and by an unnormalized one (..segment, mixed separators on Windows), where that file has a relative import of its own, and checks the rendered HTML shows both exports and that the twoimport()s returned one module instance.EINVAL reading "\\"output above. Windows x64, debug build of this branch: passes, and the whole file passes (10 tests).--timeout 120000(every full-build test in this file needs more than the 5s local default on a debug build here; CI passes its own timeout). The new test also passes on Linux without the fix: posix disk paths were never affected, so the failing-before evidence for this change is the Windows run, and the CI Windows lanes are what exercise it.BakeToWindowsPathstill returns an ownedWTFStringImpl(clone_utf8instead ofclone_utf16), so that change stays correct on top of this one. Neither touches this hunk of production.rs.Background
bun build --appbundles the app's server code into chunks, then prerenders the static routes in a dedicated JS global (Bake::GlobalObject, created byBakeCreateProdGlobal) whose module loader hooks serve the chunks from memory. Module keys arebake:plus a posix-style absolute path; a chunk written todist/_bun/abc.jsis the modulebake:/_bun/abc.js, and imports between chunks are resolved by joining the relative specifier onto the referrer's key.bakeModuleLoaderFetchstripsbake:and hands the remainder to the regular module loader, which reads it from disk. That is how a route can load a file at render time that the bundler did not see. On POSIX the remainder is already the disk path.BakeToWindowsPathexists to convert the remainder into a Windows path before the loader sees it.join_absin bun_paths ispath.resolveparameterized by platform. The Windows variant understands drive letters and UNC volumes (\\server\share); the posix variant only treats a leading/as absolute, which is what the key namespace needs for bundle paths.\\?\is the Win32 extended-length path prefix; it is meant for direct file system calls, never for module specifiers, which the loader parses for?querysuffixes like?raw.Keys produced by the debug build on Windows (BUN_DEBUG_production=1), fixture with a drive path, an unnormalized spelling, a lowercase drive letter, and UNC spellings through the C$ admin share
pages/index.tsximportsextra/banner.mjsviajoin(import.meta.dir, "..", "extra", "banner.mjs"), via[import.meta.dir, "..", "extra", "banner.mjs"].join("/")and via the lowercased join;pages/unc.tsximports it via\\localhost\C$\tmp\bakefix\extra\banner.mjsand via//localhost/C$/tmp/bakefix/extra/../extra/banner.mjs.banner.mjsitself imports./detail.mjsand../extra2/sibling.mjs.Rendered output (
a === banda === care the unnormalized and lowercase spellings;unc === uncFwdthe two UNC spellings):The UNC case is not in the committed test because it depends on the administrative share being reachable on the runner.