Fix fs.mkdir recursive regression from Bun v1.1.44 (#16464)

oven-sh · Jan 17, 2025 · 9579e42 · 9579e42
1 parent 4c57915
commit 9579e42
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 7 deletions.
diff --git a/src/bun.js/node/node_fs.zig b/src/bun.js/node/node_fs.zig
@@ -3805,7 +3805,11 @@ pub const NodeFS = struct {
                     else => {
                         return .{ .err = err.withPath(this.osPathIntoSyncErrorBuf(path[0..len])) };
                     },
-                    .EXIST => return switch (bun.sys.directoryExistsAt(bun.invalid_fd, path)) {
+                    // mkpath_np in macOS also checks for EISDIR.
+                    .ISDIR,
+                    // check if it was actually a directory or not.
+                    .EXIST,
+                    => return switch (bun.sys.directoryExistsAt(bun.invalid_fd, path)) {
                         .err => .{ .err = .{
                             .errno = err.errno,
                             .syscall = .mkdir,

diff --git a/src/sys.zig b/src/sys.zig
@@ -2891,20 +2891,30 @@ pub fn directoryExistsAt(dir: anytype, subpath: anytype) JSC.Maybe(bool) {
         if (Maybe(bool).errnoSys(bun.C.linux.statx(
             dir_fd.cast(),
             subpath,
-            std.os.linux.AT.SYMLINK_NOFOLLOW | std.os.linux.AT.STATX_SYNC_AS_STAT,
+            // Don't follow symlinks, don't automount, minimize permissions needed
+            std.os.linux.AT.SYMLINK_NOFOLLOW | std.os.linux.AT.NO_AUTOMOUNT,
+            // We only need the file type to check if it's a directory
             std.os.linux.STATX_TYPE,
             &statx,
         ), .statx)) |err| {
-            if (err.err.getErrno() == .NOSYS) break :brk; // Linux < 4.11
+            switch (err.err.getErrno()) {
+                .OPNOTSUPP, .NOSYS => break :brk, // Linux < 4.11
+
+                // truly doesn't exist.
+                .NOENT => return .{ .result = false },
+
+                else => return err,
+            }
             return err;
         }
         return .{ .result = S.ISDIR(statx.mode) };
     }
 
-    // TODO: on macOS, try getattrlist
-
     return switch (fstatat(dir_fd, subpath)) {
-        .err => |err| .{ .err = err },
+        .err => |err| switch (err.getErrno()) {
+            .NOENT => .{ .result = false },
+            else => .{ .err = err },
+        },
         .result => |result| .{ .result = S.ISDIR(result.mode) },
     };
 }
@@ -4116,7 +4126,7 @@ pub const coreutils_error_map = brk: {
             .{ "EROFS", "Read-only file system" },
             .{ "ERPCMISMATCH", "RPC version wrong" },
             .{ "ESHLIBVERS", "Shared library version mismatch" },
-            .{ "ESHUTDOWN", "Can’t send after socket shutdown" },
+            .{ "ESHUTDOWN", "Can't send after socket shutdown" },
             .{ "ESOCKTNOSUPPORT", "Socket type not supported" },
             .{ "ESPIPE", "Illegal seek" },
             .{ "ESRCH", "No such process" },

diff --git a/test/js/node/fs/fs.test.ts b/test/js/node/fs/fs.test.ts
@@ -3462,3 +3462,13 @@ it("open mode verification", async () => {
     RangeError(`The value of "mode" is out of range. It must be an integer. Received 4294967298.5`),
   );
 });
+
+it("fs.mkdirSync recursive should not error when the directory already exists, but should error when its a file", () => {
+  expect(() => mkdirSync(import.meta.dir, { recursive: true })).not.toThrowError();
+  expect(() => mkdirSync(import.meta.path, { recursive: true })).toThrowError();
+});
+
+it("fs.mkdirSync recursive: false should error when the directory already exists, regardless if its a file or dir", () => {
+  expect(() => mkdirSync(import.meta.dir, { recursive: false })).toThrowError();
+  expect(() => mkdirSync(import.meta.path, { recursive: false })).toThrowError();
+});