bundler: allow relative FileMap keys without tripping absolute-path debug asserts - #32716
bundler: allow relative FileMap keys without tripping absolute-path debug asserts#32716robobun wants to merge 4 commits into
Conversation
…ebug asserts
Bun.build's `files` option accepts user-supplied keys that may be
relative (the docs show "./src/generated.ts"). Two debug assertions
assumed file-namespace paths are always absolute:
- enqueue_entry_item asserted every file-namespace path is absolute,
but FileMap keys are lookup identities, not real fs paths. Skip the
assertion when the path is a FileMap key.
- generate_isolated_hash asserted path_with_pretty_initialized always
yields pretty.ptr != text.ptr, but for a bare relative key like
"e.js" the computed pretty equals text and dupe_alloc aliases them.
Relax the assertion to allow this for non-absolute text.
Release builds were unaffected (both are debug_assert); only CI_ASSERT
builds panicked.
Fuzzer repro:
Bun.build({entrypoints:["./e.js"], files:{"./e.js": ")"}, throw:false})
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe bundler now conditions absolute-path assertions on FileMap-backed entry keys and relaxes pretty-path aliasing checks for equal text/pretty paths. Tests add subprocess coverage for relative-key ChangesRelative-key bundler assertions
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
|
Updated 7:58 PM PT - Jun 25th, 2026
❌ @robobun, your commit 0791891 has 3 failures in
🧪 To try this PR locally: bunx bun-pr 32716That installs a local version of the PR into your bun-32716 --bun |
There was a problem hiding this comment.
LGTM — both prior nits addressed; debug-assert-only relaxations with regression tests.
Extended reasoning...
Overview
This PR relaxes two over-strict debug assertions in the bundler that fired when Bun.build's files option was given relative keys (e.g. "./e.js") as entry points — a documented and release-correct usage. Touches:
src/bundler/bundle_v2.rs(+7/-1): gateassert_file_path_is_absoluteinenqueue_entry_itemso it skips paths that are FileMap keys; thefile_map.contains()lookup is itself gated onCI_ASSERTso it dead-code-eliminates in release.src/bundler/LinkerContext.rs(+6/-4): relax thepretty.ptr != text.ptrdebug_assert!ingenerate_isolated_hashto allow aliasing whentextis non-absolute (the original invariant for absolute paths is preserved via|| !is_absolute(text)).test/bundler/bundler_files.test.ts(+63/-1): four new subprocess regression tests covering"./e.js","e.js","./src/e.js", and the fuzzer's parse-error input.
Security risks
None. Both source changes touch only debug_assert! / CI_ASSERT-gated code paths and have zero effect on release builds. No auth, crypto, fs, or network surface is involved.
Level of scrutiny
Low. Release behaviour is unchanged by construction — assert_file_path_is_absolute (src/paths/lib.rs:960) is already a no-op outside CI_ASSERT, and debug_assert! compiles out in release. The relaxations are principled and narrow: the bundle_v2.rs change only exempts paths that are verifiably FileMap keys, and the LinkerContext.rs change keeps the original invariant for absolute paths. The PR description's root-cause analysis (dupe_alloc's index_of re-aliasing pretty→text at offset 0 for bare relative keys) is consistent with the code.
Other factors
- Both of my earlier nits (4-line comment, sequential subprocess tests) were addressed in 89a51df and 0791891 respectively, and both threads are resolved.
- No CODEOWNERS coverage for
src/bundler/ortest/bundler/. - The bug-hunting system found no issues.
- Tests run in subprocesses specifically so a regression panic fails the test rather than killing the runner — appropriate for assertion-panic coverage.
|
CI status: |
|
#38650 takes a different approach to the same assertion failure: it resolves relative |
|
Closing in favour of #38650, which fixes the same assertion failure from the other side: relative The cases covered here ( |
Repro
Under a debug/ASAN build:
The invalid JS is incidental; any relative key used as an entry point panics. A second assertion in
generate_isolated_hash(LinkerContext.rs:1780) fires for bare keys like"e.js"once the first one is past. Release builds are unaffected since both aredebug_assert!.Cause
Bun.build'sfilesoption stores user-supplied keys verbatim. The docs show relative keys as valid ("./src/generated.ts"), and release builds handle them correctly. Two CI-only assertions were too strict:enqueue_entry_itemasserts everyfile:-namespace path is absolute.FileMap::resolvereturns the raw key in thefilenamespace, so relative keys trip it. The other twoFileMap::resolvecall sites (run_resolver,resolve_import_records) bypassenqueue_entry_itemand never hit this.generate_isolated_hashassertspretty.ptr != text.ptrafterpath_with_pretty_initialized. For a bare relative key like"e.js",relative(top_level_dir, "e.js")yields"e.js"again, anddupe_alloc'sindex_ofoptimization re-aliasesprettytotextat offset 0.Fix
assert_file_path_is_absoluteinenqueue_entry_itemwhen the path is a FileMap key (gated onCI_ASSERTso thecontainslookup costs nothing in release).generate_isolated_hashassertion to allowpretty.ptr == text.ptrfor non-absolutetext; absolute paths keep the original invariant.Verification
bun bd test test/bundler/bundler_files.test.ts: 27 pass (4 new subprocess tests covering"./e.js","e.js","./src/e.js", and the fuzzer's parse-error input). Withsrc/reverted, all 4 new tests fail on the panic.