diff --git a/src/bun.js/api/filesystem_router.zig b/src/bun.js/api/filesystem_router.zig index a5da347f1cb9..05361c3ac5e3 100644 --- a/src/bun.js/api/filesystem_router.zig +++ b/src/bun.js/api/filesystem_router.zig @@ -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(); @@ -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| { @@ -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; @@ -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); @@ -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); }; this.router.deinit(); diff --git a/test/js/bun/util/filesystem_router.test.ts b/test/js/bun/util/filesystem_router.test.ts index 02e27b5dfacd..a3338064e45e 100644 --- a/test/js/bun/util/filesystem_router.test.ts +++ b/test/js/bun/util/filesystem_router.test.ts @@ -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); +});