Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
27 changes: 22 additions & 5 deletions src/bun.js/api/filesystem_router.zig
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,13 @@ pub const FileSystemRouter = struct {
const path_to_use = (root_dir_path.cloneWithTrailingSlash(allocator) catch unreachable).slice();

const root_dir_info = vm.transpiler.resolver.readDirInfo(path_to_use) catch {
// Build the JS error before freeing the arena: `log` is backed by the arena allocator.
// Capture the error union so cleanup runs even if toJS itself fails.
const err_value = log.toJS(globalThis, globalThis.allocator(), "reading root directory");
origin_str.deinit();
arena.deinit();
globalThis.allocator().destroy(arena);
return globalThis.throwValue(try log.toJS(globalThis, globalThis.allocator(), "reading root directory"));
return globalThis.throwValue(try err_value);
} orelse {
origin_str.deinit();
arena.deinit();
Expand All @@ -127,10 +130,13 @@ pub const FileSystemRouter = struct {
}) catch unreachable;

router.loadRoutes(&log, root_dir_info, Resolver, &vm.transpiler.resolver, router.config.dir) catch {
// Build the JS error before freeing the arena: `log` is backed by the arena allocator.
// Capture the error union so cleanup runs even if toJS itself fails.
const err_value = log.toJS(globalThis, globalThis.allocator(), "loading routes");
origin_str.deinit();
arena.deinit();
globalThis.allocator().destroy(arena);
return globalThis.throwValue(try log.toJS(globalThis, globalThis.allocator(), "loading routes"));
return globalThis.throwValue(try err_value);
};

if (try argument.get(globalThis, "origin")) |origin| {
Expand All @@ -143,10 +149,13 @@ pub const FileSystemRouter = struct {
}

if (log.errors + log.warnings > 0) {
// Build the JS error before freeing the arena: `log` is backed by the arena allocator.
// Capture the error union so cleanup runs even if toJS itself fails.
const err_value = log.toJS(globalThis, globalThis.allocator(), "loading routes");
origin_str.deinit();
arena.deinit();
globalThis.allocator().destroy(arena);
return globalThis.throwValue(try log.toJS(globalThis, globalThis.allocator(), "loading routes"));
return globalThis.throwValue(try err_value);
}

var fs_router = globalThis.allocator().create(FileSystemRouter) catch unreachable;
Expand Down Expand Up @@ -233,7 +242,12 @@ pub const FileSystemRouter = struct {
bustDirCache(this, globalThis);

const root_dir_info = vm.transpiler.resolver.readDirInfo(this.router.config.dir) catch {
return globalThis.throwValue(try log.toJS(globalThis, globalThis.allocator(), "reading root directory"));
// Build the JS error before freeing the arena: `log` is backed by the arena allocator.
// Capture the error union so cleanup runs even if toJS itself fails.
const err_value = log.toJS(globalThis, globalThis.allocator(), "reading root directory");
arena.deinit();
globalThis.allocator().destroy(arena);
return globalThis.throwValue(try err_value);
} orelse {
arena.deinit();
globalThis.allocator().destroy(arena);
Expand All @@ -246,9 +260,12 @@ pub const FileSystemRouter = struct {
.asset_prefix_path = this.router.config.asset_prefix_path,
}) catch unreachable;
router.loadRoutes(&log, root_dir_info, Resolver, &vm.transpiler.resolver, router.config.dir) catch {
// Build the JS error before freeing the arena: `log` is backed by the arena allocator.
// Capture the error union so cleanup runs even if toJS itself fails.
const err_value = log.toJS(globalThis, globalThis.allocator(), "loading routes");
arena.deinit();
globalThis.allocator().destroy(arena);
return globalThis.throwValue(try log.toJS(globalThis, globalThis.allocator(), "loading routes"));
return globalThis.throwValue(try err_value);
};
Comment thread
robobun marked this conversation as resolved.

this.router.deinit();
Expand Down
34 changes: 34 additions & 0 deletions test/js/bun/util/filesystem_router.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -510,3 +510,37 @@ it("MatchedRoute.params does not leak", async () => {
expect(stdout).toBe("");
expect(exitCode).toBe(0);
}, 60_000);

it("throws a clean error for invalid route filenames (no use-after-free)", async () => {
// The constructor's log is backed by an arena allocator. When route loading
// produces errors (e.g. a filename like `[foo.tsx` missing its closing bracket),
// the arena must not be freed before log.toJS() reads the messages.
// Run in a subprocess so an ASAN crash doesn't take down the test runner.
using dir = tempDir("fsr-invalid-route", {
"pages/[foo.tsx": "export default 1;",
});

const code = /* ts */ `
try {
new Bun.FileSystemRouter({
style: "nextjs",
dir: ${JSON.stringify(path.join(String(dir), "pages"))},
fileExtensions: [".tsx"],
});
console.log("no-throw");
} catch (e) {
console.log("caught:" + (e?.message ?? String(e)));
}
`;

await using proc = Bun.spawn({
cmd: [bunExe(), "-e", code],
env: bunEnv,
stdout: "pipe",
stderr: "pipe",
});
const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stderr).toBe("");
expect(stdout.trim()).toBe("caught:Route is missing a closing bracket]");
expect(exitCode).toBe(0);
});
Loading