Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
2764861
resolver: don't treat '?' in a directory name as a query-string separ…
robobun Aug 1, 2026
2ef1185
address review: skip only '?/' sequences, add ImportMetaObject, extra…
robobun Aug 1, 2026
9ea7307
trim doc comments (comment-cop)
robobun Aug 1, 2026
33285f4
trim doc comment to one line
robobun Aug 1, 2026
795a109
apply ?/ skip to CommonJS.ts .node gate, cfg-gate on Windows
robobun Aug 1, 2026
665c21e
test: run 07928 subprocess tests concurrently
robobun Aug 1, 2026
3e2d29e
test: add positive assertions in .node and ?/ trade-off tests
robobun Aug 1, 2026
81a32a3
ci: retrigger
robobun Aug 1, 2026
c458e97
Merge main and remove the '?/' rule
robobun Oct 15, 2026
ecd30d2
Decide in _resolve what a '?' in a module specifier is, and carry the…
robobun Oct 15, 2026
c10a6d3
import.meta, the .node checks and bun's own imports read where the pa…
robobun Oct 15, 2026
9cb0f19
Test a '?' in a file or folder name in import-query.test.ts
robobun Oct 15, 2026
da9fe91
Test bun test in a folder with '?' in its name
robobun Oct 15, 2026
332b645
Test bun -e in a working directory with '?' in its name
robobun Oct 15, 2026
6fa1de1
Test import.meta of a module in a folder with '?' in its name
robobun Oct 15, 2026
e120caf
Test a worker whose file is in a folder with '?' in its name
robobun Oct 15, 2026
4115112
Test plugins with an importer and an answer in a folder with '?' in i…
robobun Oct 15, 2026
e410e39
Test a macro and a cron script in a folder with '?' in its name
robobun Oct 15, 2026
3b3e8ae
Test an HTML entry point in a folder with '?' in its name
robobun Oct 15, 2026
3d46c2c
Keep the '?' tests in files that pass in a debug build
robobun Oct 15, 2026
ad9c1c5
Format
robobun Oct 15, 2026
e55e50c
Test bunfig serve plugins in a folder with '?' in its name
robobun Oct 15, 2026
268b7f9
A name right in the importing folder needs no path join, and a module…
robobun Oct 15, 2026
df4a7ed
Test a query after a path with '?' that is not ASCII
robobun Oct 15, 2026
f3e51d5
Merge remote-tracking branch 'origin/main' into farm/f2eb0668/fix-que…
robobun Oct 16, 2026
e0afafb
Read every string that is not an ES specifier as a path with a query …
robobun Oct 16, 2026
3ee0b87
Keep the macro and cron imports as they are on Windows
robobun Oct 16, 2026
bafe680
Record the key of a path with '?' in _resolve, and scan a key with no…
robobun Oct 16, 2026
e4f170c
Ask for the path length of a required id only when it can name an addon
robobun Oct 16, 2026
3e546b8
Ask the folder cache about a late folder only for an absolute path
robobun Oct 16, 2026
b2739ea
Look for '.node' in a required id with one call, as before
robobun Oct 16, 2026
369c978
Declare the return of $internalRequire as any
robobun Oct 16, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion src/bundler/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -443,8 +443,18 @@
let specifier = slice;
let mut query: &[u8] = b"";

if let Some(i) = strings::index_of_char(slice, b'?') {
let i = i as usize;
let mut i = i as usize;
// A '?' that sits before a path separator is part of a directory name
// (POSIX allows '?' in filenames), not a query string. Only split on a
// '?' that appears in the final path segment. #7928
if strings::contains_char(&slice[i + 1..], b'/') {
let base = strings::last_index_of_char(slice, b'/').unwrap_or(0) + 1;
match strings::index_of_char(&slice[base..], b'?') {
Some(j) => i = base + j as usize,
None => return (slice, specifier, query),
}
}

Check warning on line 457 in src/bundler/options.rs

View check run for this annotation

Claude / Claude Code Review

Same ~10-line query-split heuristic copy-pasted into four call sites

The same ~10-line `contains_char` / `last_index_of_char` / `index_of_char` block (with the identical `#7928` comment) is copy-pasted verbatim into four functions across three crates. Per REVIEW.md — *"The second time a multi-line block appears in your diff, extract a named helper and use it at EVERY parallel site"* — this should be a single `fn index_of_query_separator(s: &[u8]) -> Option<usize>` in `bun_core::strings` (which all four sites already import), so future edits to the heuristic don't
Comment thread
robobun marked this conversation as resolved.
Resolved
query = &slice[i..];
slice = &slice[..i];
}
Expand Down
12 changes: 11 additions & 1 deletion src/jsc/VirtualMachine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2938,7 +2938,17 @@ fn normalize_specifier_for_resolution<'a>(
specifier_: &'a [u8],
query_string: &mut &'a [u8],
) -> &'a [u8] {
if let Some(i) = bun_core::strings::index_of_char_usize(specifier_, b'?') {
if let Some(mut i) = bun_core::strings::index_of_char_usize(specifier_, b'?') {
// A '?' that sits before a path separator is part of a directory name
// (POSIX allows '?' in filenames), not a query string. Only split on a
// '?' that appears in the final path segment. #7928
if bun_core::strings::contains_char(&specifier_[i + 1..], b'/') {
let base = bun_core::strings::last_index_of_char(specifier_, b'/').unwrap_or(0) + 1;
match bun_core::strings::index_of_char_usize(&specifier_[base..], b'?') {
Some(j) => i = base + j,
None => return specifier_,
}
}
*query_string = &specifier_[i..];
&specifier_[..i]
} else {
Expand Down
26 changes: 22 additions & 4 deletions src/runtime/jsc_hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3916,8 +3916,17 @@
}
let specifier = slice;
let mut query: &[u8] = b"";
if let Some(i) = bun_core::strings::index_of_char_usize(slice, b'?') {
let i = i as usize;
if let Some(mut i) = bun_core::strings::index_of_char_usize(slice, b'?') {
// A '?' that sits before a path separator is part of a directory name
// (POSIX allows '?' in filenames), not a query string. Only split on a
// '?' that appears in the final path segment. #7928
if bun_core::strings::contains_char(&slice[i + 1..], b'/') {
let base = bun_core::strings::last_index_of_char(slice, b'/').unwrap_or(0) + 1;
match bun_core::strings::index_of_char_usize(&slice[base..], b'?') {
Some(j) => i = base + j,
None => return (slice, specifier, query),
}
}
query = &slice[i..];
slice = &slice[..i];
}
Expand Down Expand Up @@ -4843,8 +4852,17 @@
specifier: &'a [u8],
query_string: &mut &'a [u8],
) -> &'a [u8] {
if let Some(i) = bun_core::strings::index_of_char_usize(specifier, b'?') {
let i = i as usize;
if let Some(mut i) = bun_core::strings::index_of_char_usize(specifier, b'?') {
// A '?' that sits before a path separator is part of a directory name
// (POSIX allows '?' in filenames), not a query string. Only split on a
// '?' that appears in the final path segment. #7928
if bun_core::strings::contains_char(&specifier[i + 1..], b'/') {
let base = bun_core::strings::last_index_of_char(specifier, b'/').unwrap_or(0) + 1;
match bun_core::strings::index_of_char_usize(&specifier[base..], b'?') {
Some(j) => i = base + j,
None => return specifier,
}
}

Check failure on line 4865 in src/runtime/jsc_hooks.rs

View check run for this annotation

Claude / Claude Code Review

Query strings containing '/' now break resolution (regression)

This heuristic regresses any import whose query string contains `/` (e.g. `import './foo.js?path=/bar/baz'`): `last_index_of_char` scans the *entire* specifier, so `base` lands inside the query, the `None` arm returns the full unsplit specifier, and the resolver fails with "Module not found" where it previously succeeded. Compute `base` from `last_index_of_char(&slice[..i], b'/')` (the portion before the first `?`) instead — the same fix is needed at all four call sites (`options.rs`, `VirtualMa
Comment thread
robobun marked this conversation as resolved.
Resolved
*query_string = &specifier[i..];
&specifier[..i]
} else {
Expand Down
95 changes: 95 additions & 0 deletions test/regression/issue/07928.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
// https://github.com/oven-sh/bun/issues/7928
// A '?' in a directory name was being treated as a query-string separator by
// the module resolver/loader, so files under that directory could not be
// loaded. Windows does not allow '?' in path names, so these tests only run on
// POSIX.
import { expect, test } from "bun:test";
import { bunEnv, bunExe, isWindows, normalizeBunSnapshot, tempDir } from "harness";

test.skipIf(isWindows)("`bun test` runs tests from a directory whose name contains '?'", async () => {
Comment thread
robobun marked this conversation as resolved.
Resolved
using dir = tempDir("issue-7928-test", {
"some-path?/my.test.js": `
import { test, expect } from "bun:test";
test("inside dir with ?", () => { expect(1 + 1).toBe(2); });
`,
});
await using proc = Bun.spawn({
cmd: [bunExe(), "test", "my.test.js"],
env: bunEnv,
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
const out = normalizeBunSnapshot(stdout + stderr, dir);
expect(out).toContain("(pass) inside dir with ?");
expect(out).toContain("1 pass");
expect(out).not.toContain("Module not found");
expect(out).not.toContain("FileNotFound");
expect(exitCode).toBe(0);
});

test.skipIf(isWindows)("`bun <file>` runs a file under a directory whose name contains '?'", async () => {
using dir = tempDir("issue-7928-run", {
"some-path?/main.js": `console.log("ran from dir with ?");`,
});
await using proc = Bun.spawn({
cmd: [bunExe(), "./some-path?/main.js"],
env: bunEnv,
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).toBe("");
expect(stdout).toBe("ran from dir with ?\n");
expect(exitCode).toBe(0);
});

test.skipIf(isWindows)("import resolves across a directory whose name contains '?'", async () => {
using dir = tempDir("issue-7928-import", {
"dir?/dep.js": `export const value = 42;`,
"dir?/entry.js": `
import { value } from "./dep.js";
console.log(JSON.stringify({ value, url: import.meta.url }));
`,
});
await using proc = Bun.spawn({
cmd: [bunExe(), "./dir?/entry.js"],
env: bunEnv,
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).toBe("");
const { value, url } = JSON.parse(stdout.trim());
expect(value).toBe(42);
expect(decodeURIComponent(url)).toEndWith("dir?/entry.js");
expect(exitCode).toBe(0);
});

Check failure on line 70 in test/regression/issue/07928.test.ts

View check run for this annotation

Claude / Claude Code Review

Fifth sibling '?'-split site not fixed: import.meta.dir/path/file are wrong under a '?' directory

There's a fifth sibling `?`-split site this PR doesn't touch: `ImportMetaObject::createFromSpecifier` (src/jsc/bindings/ImportMetaObject.cpp:169) still does `specifier.find('?')`, so for a module at `/tmp/dir?/entry.js` — which this PR now makes loadable — `import.meta.path`→`/tmp/dir`, `import.meta.dir`→`/tmp`, `import.meta.file`→`dir`. This test's `decodeURIComponent(url).toEndWith("dir?/entry.js")` is satisfied by both the correct `file:///…/dir%3F/entry.js` and the incorrect `file:///…/dir?/
Comment thread
robobun marked this conversation as resolved.
Resolved

test.skipIf(isWindows)("query-string suffix still works when a parent directory contains '?'", async () => {
using dir = tempDir("issue-7928-query", {
"dir?/dep.js": `export const url = import.meta.url;`,
"dir?/entry.js": `
const a = await import("./dep.js");
const b = await import("./dep.js?v=1");
console.log(JSON.stringify({ a: a.url, b: b.url, distinct: a !== b }));
`,
});
await using proc = Bun.spawn({
cmd: [bunExe(), "./dir?/entry.js"],
env: bunEnv,
cwd: String(dir),
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).toBe("");
const { a, b, distinct } = JSON.parse(stdout.trim());
expect(decodeURIComponent(a)).toEndWith("dir?/dep.js");
expect(decodeURIComponent(b)).toEndWith("dir?/dep.js?v=1");
expect(distinct).toBe(true);
expect(exitCode).toBe(0);
});
Loading