Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
26 changes: 9 additions & 17 deletions src/resolver/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,18 +102,6 @@
|P| ::bun_paths::resolve_path::join_abs_string_buf::<P>(cwd, buf, parts)
)
}
pub(super) fn join_abs(cwd: &[u8], platform: Platform, part: &[u8]) -> &'static [u8] {
// NOTE: `resolve_path::join_abs` ties the result lifetime to `cwd`, but the
// returned slice always points into the threadlocal `PARSER_JOIN_INPUT_BUFFER`
// (or is `cwd` itself when `parts.is_empty()`, which never happens here — we
// pass exactly one part). Re-erase to `'static` so the resolver can hold it
// across `&mut self` calls.
let s = dispatch_platform!(platform, |P| ::bun_paths::resolve_path::join_abs::<P>(
cwd, part
));
// SAFETY: see NOTE — slice borrows threadlocal storage, valid 'static per-thread.
unsafe { bun_ptr::detach_lifetime(s) }
}
pub(super) fn join(parts: &[&[u8]], platform: Platform) -> &'static [u8] {
dispatch_platform!(platform, |P| ::bun_paths::resolve_path::join::<P>(parts))
}
Expand Down Expand Up @@ -2482,11 +2470,15 @@
return false;
}

let joined = bun_paths::join_abs(
bun_paths::dirname_platform(import_source_file, bun_paths::Platform::AUTO),
bun_paths::Platform::AUTO,
specifier,
);
// `specifier` is arbitrary source text. A path that does not fit in a
// path buffer cannot be in either cache, so there is nothing to bust.
Comment thread
robobun marked this conversation as resolved.
Outdated
let source_dir = bun_paths::dirname_platform(import_source_file, bun_paths::Platform::AUTO);
let mut buf = bun_paths::path_buffer_pool::get();
let Some(joined) = bun_paths::resolve_path::join_abs_string_buf_checked::<
bun_paths::platform::Auto,
>(source_dir, &mut buf.0, &[specifier]) else {
return false;
};

Check notice on line 2481 in src/resolver/resolver.rs

View check run for this annotation

Claude / Claude Code Review

Same-class sibling sites still use unchecked join_abs_string_buf on user-controlled specifiers

Two more sites join a user-controlled specifier into a fixed path buffer with the unchecked `join_abs_string_buf` and can panic the same way: `HTMLScanner::create_import_record` (src/bundler/HTMLScanner.rs:38 and :60, reached by the dev server / `bun build --watch` on an HTML entry with a long `/`-prefixed `<script src>`) and `FileMap::resolve` (src/bundler/bundle_v2.rs:1007, plus the Windows `path_to_posix_buf` at :955, reached via `Bun.build({ files: {...} })`). Both are pre-existing and in fi
Comment thread
robobun marked this conversation as resolved.
Outdated
let dir = bun_paths::dirname_platform(joined, bun_paths::Platform::AUTO);

let a = self.bust_dir_cache(dir);
Expand Down
16 changes: 11 additions & 5 deletions src/runtime/bake/dev_server/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1273,12 +1273,18 @@ impl DirectoryWatchStore {
_ => debug_assert!(false),
}

// `specifier` is arbitrary source text. A path that does not fit in a
// path buffer cannot exist, so there is no directory to watch for it.
Comment thread
robobun marked this conversation as resolved.
Outdated
let mut buf = bun_paths::path_buffer_pool::get();
let joined = bun_paths::resolve_path::join_abs_string_buf::<bun_paths::platform::Auto>(
bun_paths::resolve_path::dirname::<bun_paths::platform::Auto>(import_source),
&mut buf.0,
&[specifier],
);
let Some(joined) =
bun_paths::resolve_path::join_abs_string_buf_checked::<bun_paths::platform::Auto>(
bun_paths::resolve_path::dirname::<bun_paths::platform::Auto>(import_source),
&mut buf.0,
&[specifier],
)
else {
return Ok(());
};
let dir = bun_paths::resolve_path::dirname::<bun_paths::platform::Auto>(joined);

// The `import_source` parameter is not a stable string. Since the
Expand Down
44 changes: 44 additions & 0 deletions test/bake/dev/bundle.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// Bundle tests are tests concerning bundling bugs that only occur in DevServer.
import { expect } from "bun:test";
import { isWindows } from "harness";
import { devTest, emptyHtmlFile, minimalFramework } from "../bake-harness";

devTest("import identifier doesnt get renamed", {
Expand Down Expand Up @@ -865,3 +866,46 @@ devTest("barrel optimization: namespace re-export cycle through a star-exported
await c.expectMessage("result: object Y KEEP DEEP OTHER");
},
});

// Resolution failures are tracked so the import is retried when its directory
// changes (Resolver.bust_dir_cache_from_specifier, then
// DirectoryWatchStore.track_resolution_failure). Both joined the importer's
// directory with the specifier into a fixed-size path buffer; a specifier that
// did not fit aborted the whole process with
// "panic: range end index N out of range for slice of length M" instead of
// reporting the unresolved import. The buffer is MAX_PATH_BYTES: 4 KiB on
// Linux, 1 KiB on macOS and about 96 KiB on Windows.
const specifierLongerThanPathBuffer = Buffer.alloc((isWindows ? 96 : 4) * 1024 + 1024, "a").toString();

devTest("unresolvable relative import longer than the path buffer is a bundling error", {
files: {
"index.html": emptyHtmlFile({ scripts: ["index.ts"] }),
"index.ts": `
import './${specifierLongerThanPathBuffer}';
console.log('loaded');
`,
},
async test(dev) {
expect((await dev.fetch("/")).status).toBe(500);
await dev.write("index.ts", `console.log('fixed');`);
expect((await dev.fetch("/")).status).toBe(200);
},
});

// A CSS url() without "./" skips the resolver's directory cache busting, so
// this only reaches DirectoryWatchStore.track_resolution_failure.
devTest("unresolvable css url() longer than the path buffer is a bundling error", {
files: {
"index.html": emptyHtmlFile({ styles: ["styles.css"] }),
"styles.css": `
body {
background-image: url(${specifierLongerThanPathBuffer});
}
`,
},
async test(dev) {
expect((await dev.fetch("/")).status).toBe(500);
await dev.write("styles.css", `body { color: blue; }`);
expect((await dev.fetch("/")).status).toBe(200);
},
});
44 changes: 44 additions & 0 deletions test/bundler/cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -466,6 +466,50 @@ test("multi-entry build writes each entry point into the output directory", asyn
expect(b).toContain('"B"');
});

async function readUntil(stream: ReadableStream<Uint8Array>, needle: string): Promise<string> {
const reader = stream.getReader();
const decoder = new TextDecoder();
let output = "";
try {
while (!output.includes(needle)) {
const { value, done } = await reader.read();
if (done) throw new Error(`stream closed before ${JSON.stringify(needle)} appeared. Output:\n${output}`);
output += decoder.decode(value, { stream: true });
}
} finally {
reader.releaseLock();
}
return output;
}

test("--watch reports an unresolvable relative import longer than the path buffer and keeps watching", async () => {
// When watching, an unresolved relative import busts the resolver's directory
// cache for the path it would have resolved to. That path used to be joined
// into a 4 KiB buffer, so a longer specifier aborted the process with
// "panic: range end index N out of range for slice of length 4095".
const specifier = "./" + Buffer.alloc(5 * 1024, "a").toString();
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
using dir = tempDir("build-watch-long-specifier", {
"entry.ts": `import "${specifier}";\nconsole.log("entry");`,
});

await using proc = Bun.spawn({
cmd: [bunExe(), "build", "--watch", "entry.ts", "--outdir", "dist"],
env: bunEnv,
cwd: String(dir),
stdin: "ignore",
stdout: "pipe",
stderr: "pipe",
});

await readUntil(proc.stderr, `error: Could not resolve: "${specifier}"`);
expect(proc.exitCode).toBeNull();

// The failed build left the watcher running: fixing the file triggers a rebuild.
await Bun.write(path.join(String(dir), "entry.ts"), `console.log("fixed");`);
expect(await readUntil(proc.stdout, "entry.js")).toContain("Bundled 1 module");
expect(await Bun.file(path.join(String(dir), "dist", "entry.js")).text()).toContain("fixed");
});

describe("CLI argument error messages", () => {
test("--format with an unrecognized value echoes the value back", async () => {
using dir = tempDir("build-format-err", { "in.js": "console.log(1)" });
Expand Down
Loading