Fix Bun.FileSystemRouter crashes on a directory listing that is not the one asked for, and reload() through a symlink

src/router/lib.rs

@@ -476,7 +476,7 @@ impl<'a> RouteLoader<'a> {
             index: None,
             route_dirname_len,
         };
-        this.load(resolver, root_dir_info, base_dir);
+        this.load(resolver, root_dir_info, base_dir, base_dir);
         if this.all_routes.is_empty() {
             return Routes {
                 static_: this.static_list,
@@ -541,20 +541,29 @@ impl<'a> RouteLoader<'a> {
         &mut self,
         resolver: &mut R,
         root_dir_info: &DirInfo,
+        dir: &[u8],
         base_dir: &[u8],
     ) {
         let fs = self.fs;
 
         // Snapshot the cached `DirEntry`'s entry pointers under `entries_mutex`:
         // another thread (e.g. the bundler's resolver) rewrites this map in place
         // under that lock, so iterating it live would walk freed buckets.
-        let entry_ptrs: Vec<*mut Fs::Entry> = {
+        let (entries_dir, entry_ptrs): (&'static [u8], Vec<*mut Fs::Entry>) = {
             let _entries_lock = fs.fs.entries_mutex.lock_guard();
             match root_dir_info.get_entries_const() {
-                Some(entries) => entries.data.values().copied().collect(),
+                Some(entries) => (entries.dir, entries.data.values().copied().collect()),
                 None => return,
             }
         };
+        // The resolver's caches match a path by its hash alone, so a hit can be another directory's listing.
+        if strings::without_trailing_slash(entries_dir) != strings::without_trailing_slash(dir) {
+            return;
+        }
+        let Some(public_dir) = entries_dir.strip_prefix(strings::without_trailing_slash(base_dir))
+        else {
+            return;
+        };
         // NOTE: the guard is dropped before this loop on purpose — the
         // `read_dir_info_ignore_error` recursion below re-acquires `entries_mutex`.
         'outer: for entry_ptr in entry_ptrs {
@@ -586,10 +595,10 @@ impl<'a> RouteLoader<'a> {
                         }
                     }
 
-                    let abs_parts = [entry.dir(), entry.base()];
-                    if let Some(dir_info) = resolver.read_dir_info_ignore_error(fs.abs(&abs_parts))
-                    {
-                        self.load(resolver, &dir_info, base_dir);
+                    // Copied out of the buffer `fs.abs` shares with the resolver.
+                    let abs_path = fs.abs(&[entries_dir, entry.base()]).to_vec();
+                    if let Some(dir_info) = resolver.read_dir_info_ignore_error(&abs_path) {
+                        self.load(resolver, &dir_info, &abs_path, base_dir);
                     }
                 }
 
@@ -602,21 +611,6 @@ impl<'a> RouteLoader<'a> {
 
                     for _extname in self.config.extensions.iter() {
                         if &extname[1..] == _extname.as_ref() {
-                            // `entry.dir()` is `base_dir` or a subdirectory of it, cached
-                            // with or without a trailing slash depending on which resolver
-                            // spelled it first (`base_dir` always has one). Both spellings
-                            // trim to the same `public_dir`.
-                            let entry_dir = entry.dir();
-                            debug_assert!(entry_dir.len() + 1 >= base_dir.len());
-                            if entry_dir.len() >= base_dir.len() {
-                                debug_assert!(bun_paths::resolve_path::is_sep_any(
-                                    entry_dir[base_dir.len() - 1]
-                                ));
-                            }
-
-                            // SAFETY: entry.dir is at least base_dir.len()-1 bytes; verified above in debug
-                            let public_dir = &entry_dir[base_dir.len() - 1..entry_dir.len()];
-
                             // SAFETY: `entry_ptr` is EntryStore-owned (process
                             // lifetime) with no other live `&mut` borrow here.
                             let route = unsafe {

src/runtime/api/filesystem_router.rs

@@ -125,8 +125,14 @@ fn load_router(
 
     let resolver = &mut VirtualMachine::get().as_mut().transpiler.resolver;
     let root_dir_info = match resolver.read_dir_info(&config.dir) {
-        Ok(Some(info)) => info,
-        Ok(None) => return Ok(None),
+        // The resolver's cache matches a path by its hash alone, so a hit can be another directory.
+        Ok(Some(info))
+            if strings::without_trailing_slash(info.abs_path)
+                == strings::without_trailing_slash(&config.dir) =>
+        {
+            info
+        }
+        Ok(_) => return Ok(None),
         Err(_) => {
             let error = log.to_js(global, format_args!("reading root directory"))?;
             return Err(global.throw_value(error));
@@ -253,23 +259,19 @@ impl FileSystemRouterClass for FileSystemRouter {
         Valid::no_null_bytes(options.dir, "dir", "a string", global)?;
 
         let mut out_buf = [0u8; MAX_PATH_BYTES * 2];
-        let mut root_dir_path = global.bun_vm().top_level_dir();
-        if !(options.dir.is_empty() || options.dir == b".") {
-            // An absolute path is joined too: a route's name is cut from the resolver's normalized spelling.
-            let Some(joined) =
-                path::resolve_path::join_abs_string_buf_checked::<path::platform::Auto>(
-                    Fs::FileSystem::instance().top_level_dir(),
-                    &mut out_buf,
-                    &[options.dir],
-                )
-            else {
-                return Err(cx.heap.error(format_args!(
-                    "Unable to find directory: {}",
-                    bstr::BStr::new(options.dir)
-                )));
-            };
-            root_dir_path = joined;
-        }
+        // Every path is joined, absolute or empty: the loader needs the resolver's normalized spelling.
+        let Some(root_dir_path) =
+            path::resolve_path::join_abs_string_buf_checked::<path::platform::Auto>(
+                Fs::FileSystem::instance().top_level_dir(),
+                &mut out_buf,
+                &[options.dir],
+            )
+        else {
+            return Err(cx.heap.error(format_args!(
+                "Unable to find directory: {}",
+                bstr::BStr::new(options.dir)
+            )));
+        };
 
         let mut extensions: Vec<Box<[u8]>> = options
             .file_extensions
@@ -287,7 +289,7 @@ impl FileSystemRouterClass for FileSystemRouter {
             dir: strings::paths::clone_normalizing_separators(root_dir_path).into_boxed_slice(),
             extensions: extensions.into(),
         };
-        let Some((mut router, root_dir_info)) = load_router(global, config, Load::Cached)? else {
+        let Some((router, root_dir_info)) = load_router(global, config, Load::Cached)? else {
             return Err(cx.heap.error(format_args!(
                 "Unable to find directory: {}",
                 bstr::BStr::new(root_dir_path)
@@ -305,7 +307,6 @@ impl FileSystemRouterClass for FileSystemRouter {
             RefCountedString::new(vm, bytes).map_err(|_| global.throw_string_too_long())
         };
         let base_dir = ref_counted(base_dir_str)?;
-        router.config.dir = Box::from(base_dir.bytes());
         let not_empty = |bytes: &&[u8]| !bytes.is_empty();
         Ok(FileSystemRouter {
             origin: options

test/js/bun/util/filesystem_router.test.ts

@@ -464,6 +464,17 @@ it("reload() works", () => {
   expect(router.match("/posts")!.name).toBe("/posts");
 });
 
+it("reload() works when dir is reached through a symlink", () => {
+  using tmp = tempDir("filesystem-router-symlink", { "real/pages/index.tsx": "", "real/pages/posts/hey.tsx": "" });
+  fs.symlinkSync(path.join(String(tmp), "real"), path.join(String(tmp), "link"), "junction");
+  const router = new FileSystemRouter({ dir: path.join(String(tmp), "link", "pages"), style: "nextjs" });
+  const routes = { ...router.routes };
+  expect(Object.keys(routes).sort()).toEqual(["/", "/posts/hey"]);
+  router.reload();
+  expect(router.routes).toEqual(routes);
+  expect(router.match("/posts/hey")?.name).toBe("/posts/hey");
+});
+
 it("reload() works with new dirs/files", () => {
   const { dir } = make(["posts.tsx"]);
 
@@ -573,13 +584,62 @@ it("dir should be validated", async () => {
   }).toThrow("FileSystemRouterOptions.dir must be a string");
 });
 
+it.each([".", ""])("dir %j is the current directory", async dir => {
+  using tmp = tempDir("filesystem-router-cwd", { "index.tsx": "", "posts/hey.tsx": "" });
+  const cwd = String(tmp);
+  await using proc = Bun.spawn({
+    cmd: [
+      bunExe(),
+      "-e",
+      `console.log(JSON.stringify(Object.keys(new Bun.FileSystemRouter({ dir: process.argv[1], style: "nextjs" }).routes).sort()))`,
+      dir,
+    ],
+    env: bunEnv,
+    // The resolver caches a Windows path with its drive letter in uppercase.
+    cwd: cwd[0].toLowerCase() + cwd.slice(1),
+    stdout: "pipe",
+    stderr: "pipe",
+  });
+  const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+  expect({ stdout, stderr, exitCode }).toEqual({ stdout: `["/","/posts/hey"]\n`, stderr: "", exitCode: 0 });
+});
+
 it("rejects a dir with a null byte", () => {
   const { dir } = make(["index.tsx"]);
   expect(() => new FileSystemRouter({ style: "nextjs", dir: `${dir}\0suffix` })).toThrow(
     expect.objectContaining({ code: "ERR_INVALID_ARG_VALUE" }),
   );
 });
 
+// The colliding path below is an absolute POSIX path.
+it.skipIf(isWindows)("does not use another directory that shares dir's resolver cache entry", async () => {
+  // The resolver's directory cache is keyed by the hash of the path without trailing slashes, so "/" is keyed as "".
+  const dir = "/ajgq" + Buffer.alloc(27, "a").toString() + "b\x05\x1db6@o\x084o.uNpJ0";
+  expect(Bun.hash(dir)).toBe(Bun.hash(""));
+
+  await using proc = Bun.spawn({
+    cmd: [
+      bunExe(),
+      "-e",
+      `
+        Bun.resolveSync(process.execPath, "/"); // caches "/"
+        try {
+          new Bun.FileSystemRouter({ dir: process.argv[1], style: "nextjs" });
+          console.log("no error");
+        } catch (error) {
+          console.log(error.message === "Unable to find directory: " + process.argv[1]);
+        }
+      `,
+      dir,
+    ],
+    env: bunEnv,
+    stdout: "pipe",
+    stderr: "pipe",
+  });
+  const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
+  expect({ stdout, stderr, exitCode }).toEqual({ stdout: "true\n", stderr: "", exitCode: 0 });
+});
+
 it("origin should be validated", async () => {
   const { dir } = make(["posts.tsx"]);