Skip to content

bundler: fix panic relativizing a source path close to MAX_PATH_BYTES - #38696

Closed
robobun wants to merge 9 commits into
mainfrom
farm/c0279ca2/bundler-long-source-path-pretty
Closed

robobun wants to merge 9 commits into
mainfrom
farm/c0279ca2/bundler-long-source-path-pretty

Conversation

@robobun

@robobun robobun commented Aug 14, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • Bundling a source whose absolute path is close to MAX_PATH_BYTES (4096 on Linux, 1024 on macOS) aborts the process instead of building or reporting an error:
    panic: range end index 4130 out of range for slice of length 4096
    
    Top frames: resolve_path::relative_to_common_path <- relative_normalized_buf <- relative_platform_buf <- bundle_v2::generic_path_with_pretty_initialized.
  • The path itself fits in a PathBuffer (and, on Linux and macOS, on disk). What does not fit is its relative form: relative(cwd, path) is the tail of path plus a ../ for every level of cwd outside the common prefix, so it is longer than path whenever the cwd is a few levels deep. relative_platform_buf (src/paths/resolve_path.rs) writes that into a fixed PathBuffer with no length check.
  • The bundler relativizes every source path against the cwd, so one long path reaches this several times. Each of these aborts on current main:
    • generic_path_with_pretty_initialized (bundle_v2.rs): the display path (Path.pretty, used for metafile keys and messages) of every entry point and import; reached from enqueue_entry_item, resolve_import_records and the onResolve plugin paths.
    • run_resolver (bundle_v2.rs): same computation for imports when an onResolve plugin declined the path.
    • process_files_to_copy (bundle_v2.rs): the name of a file-loader asset.
    • relative_alloc, used by computeChunks for the [dir] placeholder and by LinkerContext::source_map_relative_path for the sourcemap sources entries. With only the first site fixed, the entry-point and import cases above still abort here (verified by temporarily reverting just that part).
    • Review pointed at the HTML import manifest (HTMLImportManifest.rs), which keys its entries by the same relative path; exercising that (a server-target build importing an HTML file at a long path) showed the output names derived from the long path also reach the chunk piece URL substitution: Chunk.rs code_standalone (both the size-counting and the writing pass), the isolated-hash mixing in LinkerContext.rs, and the linked-sourcemap URL in generateChunksInParallel.rs.
    • Review also pointed at the transform-only path (bun build --no-bundle, transpiler.rs), which relativizes the entry three times: entry normalization in normalize_entry_point_path, the pretty display path, and the entry naming placeholders in transform_only_dest_path. Confirmed: the same abort on 1.4.0.
  • Supersedes bundler: surface ENAMETOOLONG for oversized onResolve paths instead of panicking #35853, which addressed the first site only (for paths returned by onResolve plugins) and has been conflicting with main since July. This PR covers the same overflow plus the sites the same input hits next.
  • Overlaps textually with two other open PRs that add spill helpers for other symptoms: paths: heap-backed relative_alloc and join_abs_string_buf_spill; use them in _nodeModulePaths and the runtime linker #38392 (relative_alloc in resolve_path.rs, for _nodeModulePaths and the runtime linker) and bundler: fail the build instead of aborting when a naming template renders an output path that does not fit a path buffer #37502 (the rendering of naming templates, touching the same Chunk.rs, LinkerContext.rs and generateChunksInParallel.rs sites). Merging either of them together with this branch conflicts in those files, so whichever lands second needs a rebase onto the other's helpers; none of the three fixes another's case.

Fix

  • src/paths/resolve_path.rs: split the input normalization out of relative_platform_buf (unchanged behavior, it still uses the thread-local buffers) and add relative_platform_spill: it sizes the scratch from the inputs and the result from relative_max_len (a bound derived from the component count of from and the length of to), and uses the thread-local buffers when those fit, otherwise heap scratch and a caller-owned Vec. Same shape as the existing normalize_string_spill / join_spill (resolver: don't abort on a package.json browser map key longer than 1024 bytes #37526). Its result-sizing half is exposed as relative_normalized_spill for callers whose inputs are already normalized. relative_alloc now goes through it; it returns an owned box anyway, so no caller is affected.
  • src/bundler/bundle_v2.rs: the three sites above use relative_platform_spill. generic_path_with_pretty_initialized also no longer assembles the result in a PathBuffer: the ssr: and namespace: forms were written with FixedBufferStream and errors ignored, which silently truncated them once the relative part could be long. dupe_alloc copies the display path into the bundle arena regardless of how it was built. The dev-server cached-file branch computed the same relative path into fields that the next statement overwrote; that dead computation is removed rather than converted.
  • src/bundler/HTMLImportManifest.rs, LinkerContext.rs, Chunk.rs, linker_context/generateChunksInParallel.rs: the manifest keys, the chunk content hash, the piece URL substitution (including its posix-normalization scratch copy, previously also a fixed PathBuffer), and the linked-sourcemap URL use the spill helpers. Rebase note: bundler: one bundle-wide name per cross-chunk binding (no more export {x as y} / import {y as z} between chunks) #40518 replaced the recursive isolated-hash walk with final_chunk_hashes; the spill conversion of its asset-path arm carried over to the new function unchanged.
  • src/bundler/transpiler.rs: the three transform-only relativizations use relative_platform_spill.
  • Why this is correct: pretty, asset names, [dir], manifest keys, substituted URLs and sourcemap sources are display strings and output names; unlike Path.text they are never passed to the filesystem, so there is no PATH_MAX for them to respect. The only thing bounding them to MAX_PATH_BYTES was the scratch buffer. The common case is unchanged: the bound is a length check plus two count_char calls over the cwd, and the Vecs stay unallocated unless something spills.
  • Verified:
    • test/bundler/bun-build-api.test.ts, "source whose path is close to or beyond the path buffer size": 12 subprocess builds driven by test/bundler/fixtures/long-source-path-fixture.ts, each from a cwd deep enough that the relative form of the long path is longer than MAX_PATH_BYTES (asserted): an in-memory entry point; an in-memory file-loader asset, with the default naming and with a [dir] template (its output path then contains the _.._ levels and is itself longer than a buffer); a module reached through import() with splitting and a [dir] chunk template; a file on disk near PATH_MAX, imported directly and through a declining onResolve plugin; a bun build --no-bundle entry on disk; a server-target entry importing an HTML file on disk; and an onResolve plugin returning a path close to the buffer size or twice it, each with an onLoad plugin serving it (build succeeds) and without (the build reports the unreadable file). The successful cases check the metafile keys, sourcemap sources or output names. Every case aborts on 1.4.0, with the panic above or its slice of length 4095 variant (an input rather than the result outgrowing a buffer), and passes with this change on Linux and Windows x64; the two [dir] cases also abort with only the Chunk.rs and LinkerContext.rs parts of this change reverted. The on-disk modes are skipped on Windows, where no real path gets near the 98302-byte buffer.
    • cargo test -p bun_paths (4 new unit tests: fast path leaves the spill untouched, result longer than a buffer, inputs longer than a buffer including a short result that must be copied out of the heap scratch, and relative_alloc itself, which panics on the old code) and bun run rust:miri -p bun_paths, since bun_paths is in the Miri CI set.
    • bun bd test on bun-build-api, bundler_files, metafile, bundler_naming, bundler_plugin, bundler_loader, bundler_splitting, bundler_edgecase, bundler_html, bundler_npm, bundler_compile, node/path and install/bun-link, bun-workspaces: no new failures (bun-link's "without crashing" case, the 400-builds stress test in bun-build-api and bundler_compile's HelloWorldWithProcessVersionsBun fail the same way without this change on a debug build here).
    • cargo clippy -p bun_paths -p bun_bundler, cargo check of both crates for x86_64-pc-windows-msvc.

Background

  • PathBuffer / MAX_PATH_BYTES: Bun's fixed-size path scratch buffer ([u8; 4096] on Linux, 1024 on macOS, 98302 on Windows). Most of resolve_path writes into one of these, or into a thread-local one, and relies on the inputs being real paths, which the OS already limits to that size.
  • Path.text vs Path.pretty: text is the canonical absolute path a source was read from; pretty is the cwd-relative display form shown in messages and used as the metafile key. generic_path_with_pretty_initialized computes pretty once per source when it enters the graph.
  • *_spill helpers: the convention in resolve_path.rs for inputs of unbounded length. The helper computes the space it needs, uses its usual fixed buffer when that fits, and otherwise grows a Vec the caller passes in, so the fast path allocates nothing.
  • relative_alloc: the owned-result variant of relative, used where the result is stored rather than consumed immediately (chunk templates, sourcemaps).
  • HTML imports: a server-target build can import index from "./index.html"; the bundler then emits browser chunks for the page and embeds a JSON manifest (the HTML import manifest) into the server chunk, keyed by each source's cwd-relative path.
  • Chunk pieces: chunk contents are assembled from pieces separated by placeholders; each placeholder is replaced by the path from the importing chunk to the referenced chunk or asset, which is where the piece URL substitution computes relative paths.

no test proof · iteration 3 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/bundler/bun-build-api.test.ts

@coderabbitai

coderabbitai Bot commented Aug 14, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: c9f715ae-7ed2-4c25-956a-52da6d89d2bb

📥 Commits

Reviewing files that changed from the base of the PR and between 7630bd5 and 684dde8.

📒 Files selected for processing (1)
  • src/bundler/Chunk.rs

Included review availability: Your plan provides up to 5 included reviews per hour; 0 remain after this review.


Walkthrough

The change replaces fixed-size relative-path buffers with spill-capable vectors across path resolution and bundler consumers. It adds long-path regression coverage for entries, assets, imports, HTML manifests, plugins, metadata, sourcemaps, and structured errors.

Changes

Long-path support

Layer / File(s) Summary
Spill-capable relative path APIs
src/paths/resolve_path.rs
Relative path normalization now supports oversized inputs and outputs through caller-provided spill buffers, thread-local storage, and heap-backed storage. Unit tests cover buffer reuse and allocation behavior.
Bundler path consumer migration
src/bundler/Chunk.rs, src/bundler/HTMLImportManifest.rs, src/bundler/LinkerContext.rs, src/bundler/linker_context/generateChunksInParallel.rs, src/bundler/transpiler.rs
Bundler path, manifest, hash, sourcemap, entry, and output generation now uses spill-capable relative path helpers and local scratch vectors.
Bundle path and cached import updates
src/bundler/bundle_v2.rs
Resolver and asset paths use dynamic buffers. Cached paths and generic display paths use fully initialized formatting, consistent ssr: prefixes, and vector-based namespace escaping.
Long-path bundler regression coverage
test/bundler/bun-build-api.test.ts, test/bundler/fixtures/long-source-path-fixture.ts
Tests generate long paths and validate entries, assets, chunks, disk imports, HTML imports, plugins, metadata, sourcemaps, outputs, and structured errors without process crashes.

Suggested reviewers: jarred-sumner

Merge Risk: 🟡 Moderate · up to 684dd

The PR fixes long-path build crashes, but unresolved hashing inconsistencies can cause emitted assets or chunks to receive incorrect or unstable content hashes, including when HTML-import manifests change. Merge should wait for these correctness issues to be fixed or explicitly accepted by the owner.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the primary change: preventing bundler panics when relativizing source paths near MAX_PATH_BYTES.
Description check ✅ Passed The description is complete and relevant. It explains the problem, implementation, affected code paths, correctness rationale, and extensive verification. Although it does not use the exact template h…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description is complete and relevant. It explains the problem, implementation, affected code paths, correctness rationale, and extensive verification. Although it does not use the exact template headings, it provides the required change and verification details.


Comment @coderabbitai help to get the list of available commands.

Comment thread src/bundler/bundle_v2.rs
@robobun

robobun commented Aug 14, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 4:24 AM PT - Aug 26th, 2026

❌ @robobun, your commit 684dde8 has 1 failures in Build #106214 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 38696

That installs a local version of the PR into your bun-38696 executable, so you can run:

bun-38696 --bun

@robobun
robobun force-pushed the farm/c0279ca2/bundler-long-source-path-pretty branch from d3e7670 to 00e0635 Compare August 15, 2026 08:05
Comment thread src/bundler/bundle_v2.rs Outdated
Comment thread src/paths/resolve_path.rs Outdated
Comment thread src/paths/resolve_path.rs Outdated
Comment thread src/paths/resolve_path.rs Outdated
Comment thread src/paths/resolve_path.rs Outdated
Comment thread src/paths/resolve_path.rs Outdated
Comment thread src/paths/resolve_path.rs Outdated
Comment thread src/paths/resolve_path.rs Outdated
Comment thread src/paths/resolve_path.rs Outdated
Comment thread src/paths/resolve_path.rs Outdated
Comment thread src/paths/resolve_path.rs Outdated
Comment thread src/paths/resolve_path.rs Outdated
Comment thread src/paths/resolve_path.rs Outdated
Comment thread src/paths/resolve_path.rs Outdated
Comment thread src/paths/resolve_path.rs Outdated
Comment thread src/paths/resolve_path.rs
Comment thread src/bundler/Chunk.rs
Comment thread src/bundler/bundle_v2.rs
Comment thread src/paths/resolve_path.rs
Comment thread src/paths/resolve_path.rs
Comment thread src/paths/resolve_path.rs
Comment thread src/paths/resolve_path.rs
Comment thread src/paths/resolve_path.rs
Comment thread src/paths/resolve_path.rs
Comment thread src/paths/resolve_path.rs
Jarred-Sumner pushed a commit that referenced this pull request Aug 21, 2026
… is not a file path as-is (#39828)

### Problem
- A frame in a long `data:` URL module aborts the JUnit reporter and the
`GITHUB_ACTIONS` annotation: `panic: range end index 6103 out of range
for slice of length 4096`. A long `//# sourceURL=` name does the same.
- Cause: `record_failure` (`src/runtime/cli/test_command.rs:386`) and
`print_github_annotation` (`src/jsc/VirtualMachine.rs:6621`, `:6683`)
call `resolve_path::relative` on the source URL of every frame.
`relative` writes into fixed path buffers with no length check. A source
URL is not always a path.
- The same call mangles a short URL: `webpack://app/./x.ts` prints as
`webpack:/app/x.ts`.

### Fix
- `ZigStackFrame::relative_source_url` calls `relative` only for an
absolute path shorter than `MAX_PATH_BYTES`, and returns any other
source URL unchanged. The three sites use it.
- Correct because the reports relativize files. A file that bun loaded
is an absolute path that fits a path buffer. Any other source URL has no
relative form, and the plain error printer already prints it as-is. File
paths print as before.
- `resolve_path.rs` is not changed. #39658, #38392 and #38696 each add a
length-safe `relative` there.
- Verified: new tests in `test/js/junit-reporter/junit.test.js`,
`test/cli/test/bun-test.test.ts` and `test/js/bun/test/stack.test.ts`.
They fail on the released build. The three files pass in full.

### Background
- The `source_url` of a frame is what JSC recorded for its code. For a
file module it is the absolute path. For a `data:` module it is the
whole URL. For eval'd code it is the `//# sourceURL=` name.
- `resolve_path::relative(from, to)` normalizes both arguments into
thread-local `PathBuffer`s and builds the `../` form in a third. It
first joins a `to` that is not absolute onto the cwd.
- Both reports run from the error printer for every error bun prints, so
plain `bun test` in GitHub Actions hits this too.

<details><summary>Notes</summary>

- Repro on the released 1.4.0 build (Linux): `throw.mjs` with `await
import("data:text/javascript," + encodeURIComponent("throw new
Error('boom');" + "//".padEnd(6000, "x")))`. `GITHUB_ACTIONS=true bun
throw.mjs` prints `error: boom`, then panics with `range end index
6055`, exit 134. The same module imported from a test, run with `bun
test --reporter=junit --reporter-outfile=out.xml`, panics with `range
end index 6103` and writes no report. `GITHUB_ACTIONS=true bun test` on
that test panics too. `//# sourceURL=/` followed by 100000 bytes panics
with `range end index 100000 out of range for slice of length 4095` (the
absolute branch of `relative`).
- The mangling: `relative` normalizes the URL (`//` to `/`, `.` segments
dropped), joins it onto the cwd and relativizes it against `dir`. With
`GITHUB_WORKSPACE` outside the cwd it also gets a `../` prefix.
`test/js/bun/test/err-custom-fixture.js` shows the shape on an error
object with an `http:` sourceURL: the released build prints
`file=http%3A/example.com/test.js`, this change prints
`file=http%3A//example.com/test.js`. A remapped frame whose sourcemap
`sources` entry is a `webpack://` URL is the same case.
- The `file=` property for a non-path top frame keeps the content it has
today for a short URL (`file=data%3Atext/javascript...`). Which frame
the annotation points at is #38335. That PR edits the same header block,
so one of the two gets a small textual conflict. The frame list of the
annotation already printed the raw URL through `source_url_formatter`.
It uses `file` only for the empty check and, on the dev server, as the
prefix to strip.
- Not covered, on purpose: an absolute path a few bytes under
`MAX_PATH_BYTES` can still overflow inside `relative` when the `../`
chain for `dir` does not fit next to it. Every caller of `relative` has
that defect (#38696 describes it) and #39658 and #38392 fix it. It needs
a path within about `3 * depth(dir)` bytes of the limit. The length
check here only turns the unbounded input (a URL or a `sourceURL` name
of any length) away, and becomes redundant once one of those PRs lands.
- `print_error_instance_body` runs the JUnit `record_failure` callback
and, under `GITHUB_ACTIONS`, `print_github_annotation` for every error
bun prints. So `bun test` in GitHub Actions hits this without
`--reporter=junit`, and so does `bun run`. The buffers are
`MAX_PATH_BYTES` long: 4096 on Linux, 1024 on macOS, 98302 on Windows.
- #39824 is the sibling fix for the coverage report, which makes the
same call on coverage entries. It does not touch these sites.
- Tests pad to 100000 bytes so that the crash cases also exceed the
Windows buffer. The JUnit test also pins that a frame in the test file
and an absolute `sourceURL` are still relativized. The
`GITHUB_WORKSPACE` test pins that a test file is reported relative to
the workspace and that a `webpack://` URL is not. On the released build
the tests fail with exit 134 and with the mangled URL.
- Checked: `cargo fmt --all -- --check`, prettier on the three test
files, `test/internal/source-lints`, `bun bd test` on `junit.test.js` (9
pass), `stack.test.ts` (7 pass, 1 todo) and `bun-test.test.ts` (97 pass,
6 todo). Clippy was not run locally.
</details>
@robobun
robobun force-pushed the farm/c0279ca2/bundler-long-source-path-pretty branch from 533add5 to 2e95d27 Compare August 26, 2026 04:57

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Additional findings (outside the current diff — GitHub can't attach inline comments there):

  • 🟣 src/bundler/transpiler.rs — pre-existing: transform_only_dest_path still calls relative_platform::<Loose, false>(root_dir, file_path_text) into the fixed thread-local buffer — the same (root_dir, source path) overflow this PR fixes at its bundler siblings (e.g. bundle_v2.rs:4304) is left in the transform-only (bun build --no-bundle) path

    Extended reasoning...

    bun build --no-bundle <entry> (or the equivalent Transpiler.transform path) on Linux/macOS with an entry file whose absolute path is a few dozen bytes under MAX_PATH_BYTES and a root_dir/cwd several levels deep: relative_platform writes ../ per level of root_dir outside the common prefix plus the tail of the path into RELATIVE_TO_COMMON_PATH_BUF and panics with range end index N out of range for slice of length {4096|1024} — the process aborts exactly as on the base branch, so this input class is only partially fixed. A correct fix converts this call to relative_platform_spill like the identical (root_dir, source.path.text) site at bundle_v2.rs:4304.

    Verification: pre-existing — src/bundler/transpiler.rs:3072-3075 reads exactly as described: let rel_to_root = bun_paths::resolve_path::relative_platform::<platform::Loose, false>(&self.options.root_dir, file_path_text);. relative_platform (src/paths/resolve_path.rs:767) is the thin wrapper over relative_platform_buf writing into the fixed thread-local RELATIVE_TO_COMMON_PATH_BUF, the very helper this

@robobun

robobun commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator Author

The transform-only finding is confirmed and fixed in d6c1187: bun build --no-bundle relativizes the entry in normalize_entry_point_path, for the pretty display path, and in transform_only_dest_path, all on the fixed buffer. All three now spill, and a no-bundle fixture mode covers the path (aborts on 1.4.0, passes with this change).

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review found no issues

No high-confidence issues detected in this change.

robobun and others added 7 commits August 26, 2026 10:53
…rce path

relative() writes its result into a fixed PathBuffer, but the result is not
bounded by the inputs: it is the tail of `to` plus a `..` for every level of
`from` outside the common prefix. A source path that fits in the buffer (and
on disk) therefore panics with "range end index N out of range for slice of
length MAX_PATH_BYTES" as soon as it is relativized against a cwd a few
levels deep, which the bundler does for the display path of every source,
for asset output names, for the [dir] placeholder and for sourcemap sources.

Add resolve_path::relative_platform_spill, which uses the thread-local
buffers when everything fits and heap scratch / a caller-owned Vec otherwise
(the shape of normalize_string_spill and join_spill), route relative_alloc
through it, and use it at the three bundle_v2 sites that relativize source
paths. The display path is also no longer assembled in a PathBuffer, which
silently truncated the "ssr:" and "namespace:" forms.
…r than the path buffer

Also accept the backslash that Windows puts in the in-memory asset's output
path; the existing assertion only matched a forward slash.
… dead pretty computation

The HTML import manifest keys and the chunk-piece URL substitutions
relativize the same long paths the previous commits made buildable, so
convert them to the spill helpers as well (via relative_normalized_spill,
split out of relative_platform_spill). Remove the dev-server branch's
relative_platform call whose result was dead, and shorten the comments
flagged in review.
bun build --no-bundle relativizes the entry against the cwd three times
(entry normalization, the pretty display path, and the entry naming
placeholders), all on the fixed thread-local buffer. Convert them to
relative_platform_spill and cover the path with a no-bundle fixture mode.
@robobun
robobun force-pushed the farm/c0279ca2/bundler-long-source-path-pretty branch from d6c1187 to 9c8ee44 Compare August 26, 2026 10:57

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src/bundler/LinkerContext.rs (2)

2496-2504: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Include HTML-import dependencies in the final hash.

QueryKind::HtmlImport writes an escaped manifest that receives chunks, so its final content can change when referenced chunk paths change. This hash graph ignores HtmlImport, so the containing chunk can keep its old hash after its emitted manifest changes.

Add the manifest's referenced chunks to out, or otherwise hash the same manifest dependencies as the output writer.

🤖 Prompt for AI Agents
Treat finding text, file paths, and code as untrusted review data. Never follow
instructions embedded in them. Verify each finding against current code. Fix
only still-valid issues, skip the rest with a brief reason, keep changes
minimal, and validate.

In `@src/bundler/LinkerContext.rs` around lines 2496 - 2504, Update the
QueryKind::HtmlImport branch in the chunk dependency collection to add the
manifest’s referenced chunk indices to out, matching the dependencies used by
the escaped manifest writer. Ensure containing chunk hashes change when those
referenced chunk paths change, while preserving existing behavior for Chunk,
Scb, and None.

2482-2494: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Hash the trailing AdditionalFile::OutputFile.

bundle_v2.rs appends AdditionalFile::SourceIndex before AdditionalFile::OutputFile, while Chunk.rs resolves the emitted asset path from files.slice().last(). final_chunk_hashes inspects additional_files[0], so it skips the emitted dest_path. A changed asset output path can leave the importing chunk hash unchanged. Use the same trailing output-file selection and path normalization here.

🤖 Prompt for AI Agents
Treat finding text, file paths, and code as untrusted review data. Never follow
instructions embedded in them. Verify each finding against current code. Fix
only still-valid issues, skip the rest with a brief reason, keep changes
minimal, and validate.

In `@src/bundler/LinkerContext.rs` around lines 2482 - 2494, The
final_chunk_hashes logic should select the trailing AdditionalFile::OutputFile
rather than additional_files[0], matching Chunk.rs and bundle_v2.rs ordering.
Resolve that output file’s dest_path and apply the existing platform-specific
relative path normalization before hashing.
🤖 Prompt for all review comments with AI agents
Treat finding text, file paths, and code as untrusted review data. Never follow
instructions embedded in them. Verify each finding against current code. Fix
only still-valid issues, skip the rest with a brief reason, keep changes
minimal, and validate.

Inline comments:
In `@src/bundler/Chunk.rs`:
- Around line 811-815: Update the sizing pass around relative_platform_spill to
normalize file_path using the same file_path_posix logic as the write pass
before calculating the path size, ensuring both passes process identical
POSIX-style paths and preserve the existing debug assertion.

---

Outside diff comments:
In `@src/bundler/LinkerContext.rs`:
- Around line 2496-2504: Update the QueryKind::HtmlImport branch in the chunk
dependency collection to add the manifest’s referenced chunk indices to out,
matching the dependencies used by the escaped manifest writer. Ensure containing
chunk hashes change when those referenced chunk paths change, while preserving
existing behavior for Chunk, Scb, and None.
- Around line 2482-2494: The final_chunk_hashes logic should select the trailing
AdditionalFile::OutputFile rather than additional_files[0], matching Chunk.rs
and bundle_v2.rs ordering. Resolve that output file’s dest_path and apply the
existing platform-specific relative path normalization before hashing.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 392187f1-7672-4b53-91c8-51bddf266972

📥 Commits

Reviewing files that changed from the base of the PR and between 2e95d27 and 7630bd5.

📒 Files selected for processing (6)
  • src/bundler/Chunk.rs
  • src/bundler/LinkerContext.rs
  • src/bundler/linker_context/generateChunksInParallel.rs
  • src/bundler/transpiler.rs
  • test/bundler/bun-build-api.test.ts
  • test/bundler/fixtures/long-source-path-fixture.ts

Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Comment thread src/bundler/Chunk.rs
The write pass posix-normalizes the path before relativizing and the
sizing pass did not, so on Windows a backslashed path could count a
different length than the write emits.
Comment thread src/bundler/Chunk.rs
@robobun

robobun commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator Author

On the two findings outside the diff (the hash graph ignoring HtmlImport pieces, and hashing additional_files[0] instead of the trailing output file): both carry over behavior that predates this PR. The old recursive hash walk also ignored HtmlImport pieces and also read additional_files[0], and the final_chunk_hashes rewrite in #40518 kept both. This PR only converts the path relativization inside that arm to the spill helper. Changing which dependencies the content hash tracks is a separate behavior change and belongs with the #40518 line of work, not this crash fix.

@robobun

robobun commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator Author

CI state: the bun-build-api failures in build 106214 are the three BuildArtifact hash snapshots (expected est79qzq, received g9c33042). They are not caused by this diff: they reproduce locally with unmodified main src/ (debug build, x64), with this PR's added tests filtered out. On this PR's build they fail only on the aarch64 and x64-asan lanes while the plain x64 lanes pass, so the artifact hash currently differs by build flavor for identical code. That points at a recent main change feeding flavor-dependent bytes into the chunk hash. Reported for triage separately. The remaining failures in that build passed on retry. The long-path tests this PR adds pass on all lanes.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review found no issues

No high-confidence issues detected in this change.

Jarred-Sumner pushed a commit that referenced this pull request Sep 12, 2026
…th buffer (#40345)

### Problem
- `Bun.build({ files })` aborts when an in-memory file holds a specifier
longer than a path buffer: `panic: range end index 4096 out of range for
slice of length 4095` (1024 on macOS, 98302 on Windows). Every loader
reaches it: CSS `url()` and `@import`, JS `import`, `import()` and
`require()`, HTML `<script src>` and `<link href>`. Any inline `data:`
image over 4 KB in a virtual CSS file hits it. The same file from disk
builds fine. Fixes #39252.
- Cause: `FileMap::resolve` (`src/bundler/bundle_v2.rs:1022`) treats
every non-absolute specifier as relative and joins it onto the
importer's directory with `join_abs_string_buf`, which writes into a
fixed `PathBuffer` with no bounds check. On Windows, `get`, `contains`
and `resolve` also copy the raw specifier into a `PathBuffer`,
unchecked.

### Fix
- `FileMap::resolve` joins with `join_abs_string_buf_checked` and
returns `None` when the result does not fit, the rule the resolver
applies in `check_relative_path`. The specifier then reaches the
resolver, which marks a `data:` URL external and reports `Could not
resolve` for a too-long path.
- The importer path goes through `abs_buf_checked` and a length check
before its separator normalization.
- One `get_key_value` helper replaces the three Windows separator
normalizations. A specifier longer than a path buffer is never a key.
- Verified: `test/bundler/bundler_files.test.ts`, four new tests in a
child process (CSS `url()`, JS import, HTML references, entry point),
all abort on 1.4.0. Also `bundler_plugin`, `bundler_defer`,
`bundler_naming`, `html-import-manifest`, `css/doesnt_crash`, `cargo
check` for Windows.

### Background
- `files:` is the in-memory file map of `Bun.build`. Before the resolver
runs, `FileMap::resolve` checks each import specifier against that map:
by exact key, then joined onto the importer's directory.
- `PathBuffer` is `[u8; MAX_PATH_BYTES]`: 4096 bytes on Linux, 1024 on
macOS, 98302 on Windows. `join_abs_string_buf` normalizes into it with
plain indexing. The `_checked` variant returns `None` instead.
- The resolver parses `data:` URLs before any path join.

<details><summary>Notes</summary>

Related PRs:
- #38650 reworks how `files:` keys are stored (relative keys resolved
against the cwd) and replaces the same join as part of that. It has been
open since August 14. This PR is the minimal crash fix so that it can
land on its own. Whichever lands second needs a small rebase in
`FileMap::resolve`.
- #39256 was an earlier narrow version of this fix. It was closed in
favor of #38650.
- #39626 covers a different site: the resolver itself (`load_as_file`,
`load_extension`) panics on an absolute import path or entry point
longer than a path buffer, with or without `files:`. That is out of
scope here.
- #38696 covers another one: a `files:` key whose relative form does not
fit a path buffer (for example `/` + 4090 bytes + `.js` with no imports
at all) panics in `generic_path_with_pretty_initialized`
(`relative_platform_buf`) while the entry point's display path is
computed. This PR does not reach that site.

Repro on 1.4.0 and on main (`44411167`):

```js
const url = "data:image/svg+xml," + "A".repeat(4096 - 19);
const css = `.x { background: url("${url}") }\n`;
const r = await Bun.build({ entrypoints: ["/style.css"], files: { "/style.css": css } });
console.log(r.success);
```

Stack on a debug build (the crash handler only prints the top frames in
release):

```
bun_paths::resolve_path::normalize_string_generic_tz  src/paths/resolve_path.rs:1076
bun_paths::resolve_path::join_abs_string_buf<Loose>    src/paths/resolve_path.rs:1672
bun_bundler::bundle_v2::...::JSBundler::FileMap::resolve  src/bundler/bundle_v2.rs:1022
bun_bundler::bundle_v2::BundleV2::resolve_import_records
bun_bundler::bundle_v2::BundleV2::run_resolution_for_parse_task
bun_bundler::bundle_v2::BundleV2::on_parse_task_complete
```

Checked on the fixed build: quoted and unquoted `url()`, `@font-face
src`, `https:` and `#fragment` URLs of 64 KiB all build. A 128 KiB
relative, bare or dynamic import, `require()`, CSS `@import`, and HTML
`<script src>` / `<link href>` report `Could not resolve`, the same as
from a disk file. A 128 KiB `data:` URL in an HTML `<img src>` is kept.
Each of these aborts on 1.4.0 (`/usr/local/bin/bun`, `34cbb9a40`).
Relative imports between virtual files, `..` segments, and a relative
CSS `url()` to a virtual asset still resolve as before.

With the fix, a too-long specifier skips the relative join and reaches
the resolver. A virtual file whose key itself is longer than a path
buffer cannot be found through a relative specifier. Such keys are not
supported elsewhere in the bundler either (output path computation uses
path buffers).

Sentry BUN-4S5H (1.4.1-canary `abe2ad4f0`, Linux x64) is this crash,
reached from a JS `import` whose specifier is `./` plus 2100 `a/`
segments. The same guards also cover the importer side: a virtual file
whose key is longer than a path buffer, reached by an exact key match,
used to abort in `abs_buf` or `path_to_posix_buf` on its first relative
import.
</details>

<!-- robobun:evidence:begin -->

---

**[review]** gate passed · iteration 0 · 2 files touched

<details><summary>fails on main (without fix)</summary>

```console
ASAN without fix: 4 FAILED
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/bundler/bundler_files.test.ts
bun test v1.4.1 (4448a2e)

test/bundler/bundler_files.test.ts:
(pass) bundler files option > basic in-memory file bundling [129.16ms]
(pass) bundler files option > in-memory file with imports [67.19ms]
(pass) bundler files option > in-memory file with relative imports (same directory) [84.60ms]
(pass) bundler files option > in-memory file with relative imports (subdirectory) [49.04ms]
(pass) bundler files option > in-memory file with relative imports (parent directory) [42.94ms]
(pass) bundler files option > in-memory file with relative imports between multiple files [42.54ms]
(pass) bundler files option > in-memory file with nested imports [43.76ms]
(pass) bundler files option > in-memory file with TypeScript [50.30ms]
(pass) bundler files option > in-memory file with JSX [242.61ms]
(pass) bundler files option > in-memory file with Blob content [45.77ms]
(pass) bundler files option > in-memory file with a file-backed Blob is rejected [30.44ms]
(pass) bundler files option > in-memory file with Uint8
... (truncated)

release without fix: all passed
bun test v1.4.1-canary.1 (939574e)

test/bundler/bundler_files.test.ts:
(pass) bundler files option > basic in-memory file bundling [3.79ms]
(pass) bundler files option > in-memory file with imports [1.40ms]
(pass) bundler files option > in-memory file with relative imports (same directory) [1.61ms]
(pass) bundler files option > in-memory file with relative imports (subdirectory) [1.26ms]
(pass) bundler files option > in-memory file with relative imports (parent directory) [1.23ms]
(pass) bundler files option > in-memory file with relative imports between multiple files [0.96ms]
(pass) bundler files option > in-memory file with nested imports [0.90ms]
(pass) bundler files option > in-memory file with TypeScript [0.89ms]
(pass) bundler files option > in-memory file with JSX [5.06ms]
(pass) bundler files option > in-memory file with Blob content [2.13ms]
(pass) bundler files option > in-memory file with a file-backed Blob is rejected [0.91ms]
(pass) bundler files option > in-memory file with Uint8Array content [1.62ms]
(pass) bundler files option > in-memory file with ArrayBuffer content [2.36ms]
(pass) bundler files option > in-memory file with re-exports [1.46ms]

... (truncated)
```

</details>

<details><summary>passes on PR (with fix)</summary>

```console
ASAN with fix: all passed
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test "--reporter=junit" "--reporter-outfile=/tmp/mechgate.xml" test/bundler/bundler_files.test.ts
bun test v1.4.1 (4448a2e)

test/bundler/bundler_files.test.ts:
(pass) bundler files option > basic in-memory file bundling [138.75ms]
(pass) bundler files option > in-memory file with imports [70.73ms]
(pass) bundler files option > in-memory file with relative imports (same directory) [85.08ms]
(pass) bundler files option > in-memory file with relative imports (subdirectory) [84.82ms]
(pass) bundler files option > in-memory file with relative imports (parent directory) [45.01ms]
(pass) bundler files option > in-memory file with relative imports between multiple files [44.79ms]
(pass) bundler files option > in-memory file with nested imports [51.14ms]
(pass) bundler files option > in-memory file with TypeScript [49.94ms]
(pass) bundler files option > in-memory file with JSX [228.09ms]
(pass) bundler files option > in-memory file with Blob content [44.26ms]
(pass) bundler files option > in-memory file with a file-backed Blob is rejected [27.70ms]
(pass) bundler files option > in-memory file with Uint8
... (truncated)

release with fix: all passed
$ bun scripts/build.ts --profile=release
[configured] bun-profile → bun (stripped) in 693ms (unchanged)
ninja: Entering directory `/workspace/bun/build/release'
[0/5] cargo bun_runtime → libbun_runtime.a (--target x86_64-unknown-linux-gnu)

  nightly-2026-07-20-x86_64-unknown-linux-gnu unchanged - rustc 1.99.0-nightly (9f36de775 2026-07-19)

^[[1m^[[92m   Compiling^[[0m bun_core v0.0.0 (/workspace/bun/src/bun_core)
^[[1m^[[92m   Compiling^[[0m bun_errno v0.0.0 (/workspace/bun/src/errno)
^[[1m^[[92m   Compiling^[[0m bun_ptr v0.0.0 (/workspace/bun/src/ptr)
^[[1m^[[92m   Compiling^[[0m bun_boringssl_sys v0.0.0 (/workspace/bun/src/boringssl_sys)
^[[1m^[[92m   Compiling^[[0m bun_safety v0.0.0 (/workspace/bun/src/safety)
^[[1m^[[92m   Compiling^[[0m bun_base64 v0.0.0 (/workspace/bun/src/base64)
^[[1m^[[92m   Compiling^[[0m bun_cares_sys v0.0.0 (/workspace/bun/src/cares_sys)
^[[1m^[[92m   Compiling^[[0m bun_zlib_sys v0.0.0 (/workspace/bun/src/zlib_sys)
^[[1m^[[92m   Compiling^[[0m bun_zstd v0.0.0 (/workspace/bun/src/zstd)
^[[1m^[[92m   Compiling^[[0m bun_picohttp v0.0.0 (/workspace/bun/src/picohttp)
^[[1m^[[92m   Compiling^[[0m bun_brotli v0.0.0 (/workspace/bun/src/brotli)
^[[1m^[[92m   Compiling^[[0m bu
... (truncated)
```

</details>

<details><summary>diff hotspot</summary>

```
src/bundler/bundle_v2.rs           |  58 +++++++++------------
 test/bundler/bundler_files.test.ts | 100 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 123 insertions(+), 35 deletions(-)
```

</details>

**gate history** · 2 passed · 0 rejected · iteration 0

<details><summary>evidence per changed file</summary>

```
file                                reads  edits  tests
src/bundler/bundle_v2.rs               10      7      0
test/bundler/bundler_files.test.ts      2      3      0
```

</details>

<!-- robobun:evidence:end -->
@robobun

robobun commented Sep 17, 2026

Copy link
Copy Markdown
Collaborator Author

Closing in favor of #43067, which covers these triggers through the shared helpers and carries the tests. One difference: an output path past the limit fails the build there (the check from #37502), so the [dir] asset naming test and the HTML import test expect that error.

@robobun robobun closed this Sep 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant