Fix crash when onResolve plugin appends query string to file path#28626
Open
Fix crash when onResolve plugin appends query string to file path#28626
Conversation
Strip query string (?...) and fragment (#...) suffixes from plugin-resolved paths in the file namespace before using them for filesystem lookup. The bundler was trying to open the path including the suffix as a literal filename, causing a 'File not found' error or crash.
Collaborator
Author
|
Updated 2:04 PM PT - Mar 28th, 2026
❌ @robobun, your commit bb91421 has 2 failures in
🧪 To try this PR locally: bunx bun-pr 28626That installs a local version of the PR into your bun-28626 --bun |
Contributor
WalkthroughBundleV2 resolution now treats file-namespace results specially: if the resolved path's filename contains Changes
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
Contributor
|
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 (2)
WalkthroughModified the bundler to handle query strings and fragments in resolver paths by stripping them for filesystem lookups before setting the file namespace. Added regression tests verifying that Changes
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
- Check file existence before stripping ?/# suffixes, since these are valid filename characters on Unix (e.g. C#/main.js) - Use test.concurrent for independent subprocess tests - Add stderr assertions with ASAN warning filtering
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/bundler/bundle_v2.zig`:
- Around line 2641-2650: The code uses bun.strings.indexOfAny(result.path, "?#")
which finds the first `?` or `#` anywhere and truncates too much; change the
logic that computes path_to_use (involving is_file_namespace and result.path) to
locate the last occurrence of `?` or `#` (scan from the right or use a
lastIndexOfAny helper) and only strip the trailing query/fragment suffix if the
full path doesn't exist (use bun.sys.exists(result.path) to decide). Update the
branch that constructs path_to_use so it uses the right-most marker index when
slicing result.path instead of the first index returned by indexOfAny.
🪄 Autofix (Beta)
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: c10198f4-7ffc-496a-83cc-360127810208
📒 Files selected for processing (2)
src/bundler/bundle_v2.zigtest/regression/issue/28625.test.ts
indexOfAny finds the first ?/# anywhere in the path, which would truncate at directory names like C#/. lastIndexOfAny finds the last occurrence, which is always the plugin-appended suffix.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/bundler/bundle_v2.zig`:
- Around line 2658-2659: The code currently replaces the module key by calling
Fs.Path.init(path_to_use) and using that sanitized path for result.path, which
causes getOrPutPath and onLoad to see the stripped path and collapse distinct
resources; instead, keep result.path as the original plugin-visible module path
(do not overwrite result.path with Fs.Path.init), and only create a separate
cleaned disk-access path variable (e.g., cleaned_path =
Fs.Path.init(path_to_use) or similar) to use exclusively for file system
operations and deduping; update calls that need the filesystem key to use the
cleaned_path while leaving getOrPutPath, onLoad, and result.path unchanged so
suffixes like ?raw and ?url remain distinct.
- Around line 2647-2652: The code uses std.mem.lastIndexOfAny to strip only the
final delimiter, leaving query strings when multiple delimiters exist; change
the logic in the path_to_use computation to find the first occurrence of '?' or
'#' (use std.mem.indexOfAny on result.path), and when a match is found slice
result.path[0..index] to remove the entire trailing query/fragment; ensure the
exists check (bun.sys.exists) uses that stripped path_to_use and fall back to
result.path only if no delimiter is found.
🪄 Autofix (Beta)
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: 25fac64a-d129-4695-b4c4-ef0ffce7399d
📒 Files selected for processing (1)
src/bundler/bundle_v2.zig
Instead of lastIndexOfAny on the full path, find first ?/# only in the filename portion (after last path separator). This correctly handles both C#/main.js (# in directory preserved) and file.html?env=browser#ssr (entire suffix stripped at first ?).
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/bundler/bundle_v2.zig`:
- Around line 2647-2653: The current logic assumes the first '?' or '#' in the
basename is the suffix and trims at that point, causing incorrect disk paths
when filenames legitimately contain those characters; change the block that
computes path_to_use (the branch under is_file_namespace using basename_start,
bun.strings.indexOfAny, bun.sys.exists and result.path) to, after locating the
basename, search for all occurrences of '?' or '#' in the basename, iterate
those cut indices from rightmost to leftmost, and for each candidate build the
trimmed path and call bun.sys.exists until you find the first existing file—use
that path; if none exist, fall back to using result.path as-is (or the original
behavior when bun.sys.exists(result.path) is true).
🪄 Autofix (Beta)
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: 7f2a95b2-c152-4e59-95ca-be749766bb6e
📒 Files selected for processing (1)
src/bundler/bundle_v2.zig
Comment on lines
+2647
to
+2653
| const path_to_use = if (is_file_namespace) blk: { | ||
| const basename_start = if (bun.path.lastIndexOfSep(result.path)) |sep| sep + 1 else 0; | ||
| if (bun.strings.indexOfAny(result.path[basename_start..], "?#")) |offset| { | ||
| break :blk if (bun.sys.exists(result.path)) | ||
| result.path | ||
| else | ||
| result.path[0 .. basename_start + offset]; |
Contributor
There was a problem hiding this comment.
Don't assume the first ?/# in the basename is the appended suffix.
On POSIX, filenames can legitimately contain ? or #. With result.path = "/tmp/foo#bar.txt?env=browser", Line 2649 cuts the path to /tmp/foo, so plugin-resolved file paths with literal ?/# in the filename still resolve to the wrong disk path. Once the full path doesn't exist, try candidate cut points from the end of the basename back toward the start and use the first candidate that exists.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/bundler/bundle_v2.zig` around lines 2647 - 2653, The current logic
assumes the first '?' or '#' in the basename is the suffix and trims at that
point, causing incorrect disk paths when filenames legitimately contain those
characters; change the block that computes path_to_use (the branch under
is_file_namespace using basename_start, bun.strings.indexOfAny, bun.sys.exists
and result.path) to, after locating the basename, search for all occurrences of
'?' or '#' in the basename, iterate those cut indices from rightmost to
leftmost, and for each candidate build the trimmed path and call bun.sys.exists
until you find the first existing file—use that path; if none exist, fall back
to using result.path as-is (or the original behavior when
bun.sys.exists(result.path) is true).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #28625
Problem
When a bundler plugin's
build.onResolvehook returns a path with a query string (e.g.,/path/to/file.html?env=browser) or hash fragment in thefilenamespace, the bundler tries to open the literal filename including the suffix, causing a "File not found" error or crash.Cause
In
BundleV2.onResolve, plugin-resolved paths are passed directly toFs.Path.initwithout stripping query/fragment suffixes. The resolver already handles this for CSS imports (kind.isFromCSS()), but plugin-resolved paths in thefilenamespace bypass the resolver entirely and never get stripped.Fix
Strip
?and#suffixes from plugin-resolved paths in thefilenamespace before constructing theFs.Path, so the filesystem lookup uses the clean path.Verification
USE_SYSTEM_BUN=1 bun test test/regression/issue/28625.test.ts→ 2 failbun bd test test/regression/issue/28625.test.ts→ 2 pass