From 1237940c2c08b00646466ecb39014327ebb111dc Mon Sep 17 00:00:00 2001 From: Dylan Conway Date: Fri, 2 Feb 2024 06:53:10 -0800 Subject: [PATCH] fixup --- src/bun.js/api/filesystem_router.zig | 2 + src/resolver/resolve_path.zig | 15 ++++- src/router.zig | 78 ++++++++++++---------- src/windows.zig | 4 ++ test/js/bun/util/filesystem_router.test.ts | 3 +- 5 files changed, 64 insertions(+), 38 deletions(-) diff --git a/src/bun.js/api/filesystem_router.zig b/src/bun.js/api/filesystem_router.zig index 6d4e21159e7900..d2827ffb6e4811 100644 --- 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 index f456af7aadd32d..78c66ae4e350f3 100644 --- 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 index 3a0009eccbd9f8..de6494591a61f1 100644 --- 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 index be35bc0eb658c8..ccfb7112fd0fb3 100644 --- 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 index 3054f15e217b02..a9aa29bf0f82dc 100644 --- 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)) {