fixup

oven-sh · Feb 2, 2024 · 1237940 · 1237940
1 parent a8feb4e
commit 1237940
Show file tree

Hide file tree

Showing 5 changed files with 64 additions and 38 deletions.
diff --git a/src/bun.js/api/filesystem_router.zig b/src/bun.js/api/filesystem_router.zig
@@ -249,6 +249,7 @@ pub const FileSystemRouter = struct {
         };
 
         this.arena.deinit();
+        this.router.deinit();
         globalThis.allocator().destroy(this.arena);
 
         this.arena = arena;
@@ -380,6 +381,7 @@ pub const FileSystemRouter = struct {
             dir.deref();
         }
 
+        this.router.deinit();
         this.arena.deinit();
     }
 };

diff --git a/src/resolver/resolve_path.zig b/src/resolver/resolve_path.zig
@@ -2126,15 +2126,26 @@ export fn ResolvePath__joinAbsStringBufCurrentPlatformBunString(
 pub fn platformToPosixInPlace(comptime T: type, path_buffer: []T) void {
     if (std.fs.path.sep == '/') return;
     var idx: usize = 0;
-    while (std.mem.indexOfScalarPos(T, path_buffer, idx, std.fs.path.sep)) |index| : (idx = index) {
+    while (std.mem.indexOfScalarPos(T, path_buffer, idx, std.fs.path.sep)) |index| : (idx = index + 1) {
         path_buffer[index] = '/';
     }
 }
 
+pub fn platformToPosixBuf(comptime T: type, path: []const T, buf: []T) []T {
+    if (std.fs.path.sep == '/') return;
+    var idx: usize = 0;
+    while (std.mem.indexOfScalarPos(T, path, idx, std.fs.path.sep)) |index| : (idx = index + 1) {
+        @memcpy(buf[idx..index], path[idx..index]);
+        buf[index] = '/';
+    }
+    @memcpy(buf[idx..path.len], path[idx..path.len]);
+    return buf[0..path.len];
+}
+
 pub fn posixToPlatformInPlace(comptime T: type, path_buffer: []T) void {
     if (std.fs.path.sep == '/') return;
     var idx: usize = 0;
-    while (std.mem.indexOfScalarPos(T, path_buffer, idx, '/')) |index| : (idx = index) {
+    while (std.mem.indexOfScalarPos(T, path_buffer, idx, '/')) |index| : (idx = index + 1) {
         path_buffer[index] = std.fs.path.sep;
     }
 }
diff --git a/src/router.zig b/src/router.zig
@@ -62,6 +62,14 @@ pub fn init(
     };
 }
 
+pub fn deinit(this: *Router) void {
+    if (comptime Environment.isWindows) {
+        for (this.routes.list.items(.filepath)) |abs_path| {
+            this.allocator.free(abs_path);
+        }
+    }
+}
+
 pub fn getEntryPoints(this: *const Router) ![]const string {
     return this.routes.list.items(.filepath);
 }
@@ -164,7 +172,7 @@ pub const Routes = struct {
                     .name = index.name,
                     .path = index.abs_path.slice(),
                     .pathname = url_path.pathname,
-                    .basename = index.entry.base(),
+                    .basename = index.basename,
                     .hash = index_route_hash,
                     .file_path = index.abs_path.slice(),
                     .query_string = url_path.query_string,
@@ -187,7 +195,7 @@ pub const Routes = struct {
                 .name = route.name,
                 .path = route.abs_path.slice(),
                 .pathname = url_path.pathname,
-                .basename = route.entry.base(),
+                .basename = route.basename,
                 .hash = route.full_hash,
                 .file_path = route.abs_path.slice(),
                 .query_string = url_path.query_string,
@@ -543,10 +551,23 @@ pub const Route = struct {
     /// This is [inconsistent with Next.js](https://github.com/vercel/next.js/issues/21498)
     match_name: PathString,
 
-    entry: *Fs.FileSystem.Entry,
+    basename: string,
     full_hash: u32,
     param_count: u16,
-    abs_path: PathString,
+
+    // On windows we need to normalize this path to have forward slashes.
+    // To avoid modifying memory we do not own, allocate another buffer
+    abs_path: if (Environment.isWindows) struct {
+        path: string,
+
+        pub fn slice(this: @This()) string {
+            return this.path;
+        }
+
+        pub fn isEmpty(this: @This()) bool {
+            return this.path.len == 0;
+        }
+    } else PathString,
 
     /// URL-safe path for the route's transpiled script relative to project's top level directory
     /// - It might not share a prefix with the absolute path due to symlinks.
@@ -560,8 +581,9 @@ pub const Route = struct {
     pub const Ptr = TinyPtr;
 
     pub const index_route_name: string = "/";
-    var route_file_buf: [bun.MAX_PATH_BYTES]u8 = undefined;
-    var second_route_file_buf: [bun.MAX_PATH_BYTES]u8 = undefined;
+    threadlocal var route_file_buf: bun.PathBuffer = undefined;
+    threadlocal var second_route_file_buf: bun.PathBuffer = undefined;
+    threadlocal var normalized_abs_path_buf: bun.windows.PathBuffer = undefined;
 
     pub const Sorter = struct {
         const sort_table: [std.math.maxInt(u8)]u8 = brk: {
@@ -645,16 +667,6 @@ pub const Route = struct {
         else
             entry.abs_path.slice();
 
-        if (comptime Environment.isWindows) {
-            if (abs_path_str.len > 0) {
-                var ptr = @constCast(abs_path_str);
-                while (strings.indexOfChar(ptr, '\\')) |i| {
-                    ptr[i] = '/';
-                    ptr = ptr[i + 1 ..];
-                }
-            }
-        }
-
         const base = base_[0 .. base_.len - extname.len];
 
         const public_dir = std.mem.trim(u8, public_dir_, std.fs.path.sep_str);
@@ -679,16 +691,13 @@ pub const Route = struct {
             buf = buf[base.len..];
             bun.copy(u8, buf, extname);
             buf = buf[extname.len..];
-            break :brk route_file_buf[0 .. @intFromPtr(buf.ptr) - @intFromPtr(&route_file_buf)];
-        };
 
-        if (comptime Environment.isWindows) {
-            var ptr = @constCast(public_path);
-            while (strings.indexOfChar(ptr, '\\')) |i| {
-                ptr[i] = '/';
-                ptr = ptr[i + 1 ..];
+            if (comptime Environment.isWindows) {
+                bun.path.platformToPosixInPlace(u8, route_file_buf[0 .. @intFromPtr(buf.ptr) - @intFromPtr(&route_file_buf)]);
             }
-        }
+
+            break :brk route_file_buf[0 .. @intFromPtr(buf.ptr) - @intFromPtr(&route_file_buf)];
+        };
 
         var name = public_path[0 .. public_path.len - extname.len];
 
@@ -770,26 +779,25 @@ pub const Route = struct {
             };
 
             abs_path_str = FileSystem.DirnameStore.instance.append(@TypeOf(_abs), _abs) catch unreachable;
-            if (comptime Environment.isWindows) {
-                var ptr = @constCast(abs_path_str);
-                while (strings.indexOfChar(ptr, '\\')) |i| {
-                    ptr[i] = '/';
-                    ptr = ptr[i + 1 ..];
-                }
-            }
             entry.abs_path = PathString.init(abs_path_str);
         }
 
+        const abs_path = if (comptime Environment.isWindows)
+            allocator.dupe(u8, bun.path.platformToPosixBuf(u8, abs_path_str, &normalized_abs_path_buf)) catch bun.outOfMemory()
+        else
+            PathString.init(abs_path_str);
+
         if (comptime Environment.allow_assert and Environment.isWindows) {
             std.debug.assert(!strings.containsChar(name, '\\'));
             std.debug.assert(!strings.containsChar(public_path, '\\'));
             std.debug.assert(!strings.containsChar(match_name, '\\'));
-            std.debug.assert(!strings.containsChar(entry.abs_path.slice(), '\\'));
+            std.debug.assert(!strings.containsChar(abs_path, '\\'));
+            std.debug.assert(!strings.containsChar(entry.base(), '\\'));
         }
 
         return Route{
             .name = name,
-            .entry = entry,
+            .basename = entry.base(),
             .public_path = PathString.init(public_path),
             .match_name = PathString.init(match_name),
             .full_hash = if (is_index)
@@ -798,7 +806,9 @@ pub const Route = struct {
                 @as(u32, @truncate(bun.hash(name))),
             .param_count = validation_result.param_count,
             .kind = validation_result.kind,
-            .abs_path = entry.abs_path,
+            .abs_path = if (comptime Environment.isWindows) .{
+                .path = abs_path,
+            } else abs_path,
             .has_uppercase = has_uppercase,
         };
     }

diff --git a/src/windows.zig b/src/windows.zig
@@ -67,6 +67,10 @@ pub const nt_object_prefix = [4]u16{ '\\', '?', '?', '\\' };
 pub const nt_maxpath_prefix = [4]u16{ '\\', '\\', '?', '\\' };
 
 const std = @import("std");
+const Environment = bun.Environment;
+pub const PathBuffer = if (Environment.isWindows) bun.PathBuffer else void;
+pub const WPathBuffer = if (Environment.isWindows) bun.WPathBuffer else void;
+
 pub const HANDLE = win32.HANDLE;
 
 /// https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-setfilevaliddata

diff --git a/test/js/bun/util/filesystem_router.test.ts b/test/js/bun/util/filesystem_router.test.ts
@@ -1,4 +1,3 @@
-// @known-failing-on-windows: 1 failing
 import { FileSystemRouter } from "bun";
 import { it, expect } from "bun:test";
 import path, { dirname, resolve } from "path";
@@ -79,7 +78,7 @@ it("should find files", () => {
     // https://github.com/oven-sh/bun/issues/8276
     // https://github.com/oven-sh/bun/issues/8278
     ...Object.fromEntries(Array.from({ length: 65 }, (_, i) => [`/files/a${i}`, `${dir}/files/a${i}.tsx`])),
-  }
+  };
 
   for (const route in fixture) {
     if (!(route in routes)) {