Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix fs.mkdir regression #16497

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions src/bun.js/node/node_fs.zig
Original file line number Diff line number Diff line change
Expand Up @@ -834,7 +834,7 @@ pub fn NewAsyncCpTask(comptime is_shell: bool) type {
var buf: bun.OSPathBuffer = undefined;

const normdest: bun.OSPathSliceZ = if (Environment.isWindows)
switch (bun.sys.normalizePathWindows(u16, bun.invalid_fd, dest, &buf)) {
switch (bun.sys.normalizePathWindows(u16, bun.invalid_fd, dest, &buf, .{ .add_nt_prefix = false })) {
.err => |err| {
this.finishConcurrently(.{ .err = err });
return false;
Expand Down Expand Up @@ -3755,10 +3755,7 @@ pub const NodeFS = struct {
pub fn mkdirRecursiveImpl(this: *NodeFS, args: Arguments.Mkdir, comptime Ctx: type, ctx: Ctx) Maybe(Return.Mkdir) {
const buf = bun.OSPathBufferPool.get();
defer bun.OSPathBufferPool.put(buf);
const path: bun.OSPathSliceZ = if (Environment.isWindows)
strings.toNTPath(buf, args.path.slice())
else
args.path.osPath(buf);
const path = args.path.osPath(buf);

// TODO: remove and make it always a comptime argument
return switch (args.always_return_none) {
Expand Down
2 changes: 1 addition & 1 deletion src/c.zig
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ pub fn copyFileZSlowWithHandle(in_handle: bun.FileDescriptor, to_dir: bun.FileDe
var buf0: bun.WPathBuffer = undefined;
var buf1: bun.WPathBuffer = undefined;

const dest = switch (bun.sys.normalizePathWindows(u8, to_dir, destination, &buf0)) {
const dest = switch (bun.sys.normalizePathWindows(u8, to_dir, destination, &buf0, .{})) {
.result => |x| x,
.err => |e| return .{ .err = e },
};
Expand Down
2 changes: 1 addition & 1 deletion src/cli/upgrade_command.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1000,7 +1000,7 @@ pub const upgrade_js_bindings = struct {

var buf: bun.WPathBuffer = undefined;
const tmpdir_path = fs.FileSystem.RealFS.getDefaultTempDir();
const path = switch (bun.sys.normalizePathWindows(u8, bun.invalid_fd, tmpdir_path, &buf)) {
const path = switch (bun.sys.normalizePathWindows(u8, bun.invalid_fd, tmpdir_path, &buf, .{})) {
.err => return .undefined,
.result => |norm| norm,
};
Expand Down
6 changes: 3 additions & 3 deletions src/install/install.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1565,7 +1565,7 @@ pub fn NewPackageInstall(comptime kind: PkgInstallKind) type {
state.buf[i] = 0;
const fullpath = state.buf[0..i :0];

_ = node_fs_for_package_installer.mkdirRecursiveOSPathImpl(void, {}, fullpath, 0, false).unwrap() catch |err| {
_ = node_fs_for_package_installer.mkdirRecursiveOSPathImpl(void, {}, strings.withoutNTPrefix(fullpath), 0, false).unwrap() catch |err| {
state.cached_package_dir.close();
state.walker.deinit();
return Result.fail(err, .copying_files);
Expand Down Expand Up @@ -1838,7 +1838,7 @@ pub fn NewPackageInstall(comptime kind: PkgInstallKind) type {

dest[dest.len - task.basename - 1] = 0;
const dirpath = dest[0 .. dest.len - task.basename - 1 :0];
_ = node_fs_for_package_installer.mkdirRecursiveOSPathImpl(void, {}, dirpath, 0, false).unwrap() catch {};
_ = node_fs_for_package_installer.mkdirRecursiveOSPathImpl(void, {}, strings.withoutNTPrefix(dirpath), 0, false).unwrap() catch {};
dest[dest.len - task.basename - 1] = std.fs.path.sep;

if (bun.windows.CreateHardLinkW(dest.ptr, src.ptr, null) != 0) {
Expand Down Expand Up @@ -2286,7 +2286,7 @@ pub fn NewPackageInstall(comptime kind: PkgInstallKind) type {
wbuf[i] = 0;
const fullpath = wbuf[0..i :0];

_ = node_fs_for_package_installer.mkdirRecursiveOSPathImpl(void, {}, fullpath, 0, false).unwrap() catch |err| {
_ = node_fs_for_package_installer.mkdirRecursiveOSPathImpl(void, {}, strings.withoutNTPrefix(fullpath), 0, false).unwrap() catch |err| {
return Result.fail(err, .linking_dependency);
};
}
Expand Down
83 changes: 77 additions & 6 deletions src/string_immutable.zig
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,14 @@ pub inline fn containsChar(self: string, char: u8) bool {
return indexOfChar(self, char) != null;
}

pub inline fn containsCharT(comptime T: type, self: []const T, char: u8) bool {
return switch (T) {
u8 => containsChar(self, char),
u16 => std.mem.indexOfScalar(u16, self, char) != null,
else => @compileError("invalid type"),
};
}

pub inline fn contains(self: string, str: string) bool {
return containsT(u8, self, str);
}
Expand Down Expand Up @@ -1899,11 +1907,30 @@ pub fn fromWPath(buf: []u8, utf16: []const u16) [:0]const u8 {
return buf[0..encode_into_result.written :0];
}

pub fn withoutNTPrefix(path: [:0]const u16) [:0]const u16 {
if (hasPrefixComptimeUTF16(path, &bun.windows.nt_object_prefix_u8)) {
return path[bun.windows.nt_object_prefix.len..];
}
if (hasPrefixComptimeUTF16(path, &bun.windows.nt_maxpath_prefix_u8)) {
return path[bun.windows.nt_maxpath_prefix.len..];
}
if (hasPrefixComptimeUTF16(path, &bun.windows.nt_unc_object_prefix_u8)) {
return path[bun.windows.nt_unc_object_prefix.len..];
}
return path;
}

pub fn toNTPath(wbuf: []u16, utf8: []const u8) [:0]u16 {
if (!std.fs.path.isAbsoluteWindows(utf8)) {
return toWPathNormalized(wbuf, utf8);
}

if (strings.hasPrefixComptime(utf8, &bun.windows.nt_object_prefix_u8) or
strings.hasPrefixComptime(utf8, &bun.windows.nt_unc_object_prefix_u8))
{
return wbuf[0..toWPathNormalized(wbuf, utf8).len :0];
}

// UNC absolute path, replace leading '\\' with '\??\UNC\'
if (strings.hasPrefixComptime(utf8, "\\\\")) {
const prefix = bun.windows.nt_unc_object_prefix;
Expand All @@ -1916,6 +1943,28 @@ pub fn toNTPath(wbuf: []u16, utf8: []const u8) [:0]u16 {
return wbuf[0 .. toWPathNormalized(wbuf[prefix.len..], utf8).len + prefix.len :0];
}

pub fn toNTPath16(wbuf: []u16, path: []const u16) [:0]u16 {
if (!std.fs.path.isAbsoluteWindowsWTF16(path)) {
return toWPathNormalized16(wbuf, path);
}

if (strings.hasPrefixComptimeUTF16(path, &bun.windows.nt_object_prefix_u8) or
strings.hasPrefixComptimeUTF16(path, &bun.windows.nt_unc_object_prefix_u8))
{
return wbuf[0..toWPathNormalized16(wbuf, path).len :0];
}

if (strings.hasPrefixComptimeUTF16(path, "\\\\")) {
const prefix = bun.windows.nt_unc_object_prefix;
wbuf[0..prefix.len].* = prefix;
return wbuf[0 .. toWPathNormalized16(wbuf[prefix.len..], path[2..]).len + prefix.len :0];
}

const prefix = bun.windows.nt_object_prefix;
wbuf[0..prefix.len].* = prefix;
return wbuf[0 .. toWPathNormalized16(wbuf[prefix.len..], path).len + prefix.len :0];
}

pub fn toNTMaxPath(buf: []u8, utf8: []const u8) [:0]const u8 {
if (!std.fs.path.isAbsoluteWindows(utf8) or utf8.len <= 260) {
@memcpy(buf[0..utf8.len], utf8);
Expand Down Expand Up @@ -1978,6 +2027,20 @@ pub fn toWPathNormalized(wbuf: []u16, utf8: []const u8) [:0]u16 {

return toWPath(wbuf, path_to_use);
}

pub fn toWPathNormalized16(wbuf: []u16, path: []const u16) [:0]u16 {
var path_to_use = normalizeSlashesOnlyT(u16, wbuf, path, '\\', true);

// is there a trailing slash? Let's remove it before converting to UTF-16
if (path_to_use.len > 3 and bun.path.isSepAnyT(u16, path_to_use[path_to_use.len - 1])) {
path_to_use = path_to_use[0 .. path_to_use.len - 1];
}

wbuf[path_to_use.len] = 0;

return wbuf[0..path_to_use.len :0];
}

pub fn toPathNormalized(buf: []u8, utf8: []const u8) [:0]const u8 {
const renormalized = bun.PathBufferPool.get();
defer bun.PathBufferPool.put(renormalized);
Expand All @@ -1992,21 +2055,29 @@ pub fn toPathNormalized(buf: []u8, utf8: []const u8) [:0]const u8 {
return toPath(buf, path_to_use);
}

pub fn normalizeSlashesOnly(buf: []u8, utf8: []const u8, comptime desired_slash: u8) []const u8 {
pub fn normalizeSlashesOnlyT(comptime T: type, buf: []T, path: []const T, comptime desired_slash: u8, comptime always_copy: bool) []const T {
comptime bun.unsafeAssert(desired_slash == '/' or desired_slash == '\\');
const undesired_slash = if (desired_slash == '/') '\\' else '/';

if (bun.strings.containsChar(utf8, undesired_slash)) {
@memcpy(buf[0..utf8.len], utf8);
for (buf[0..utf8.len]) |*c| {
if (bun.strings.containsCharT(T, path, undesired_slash)) {
@memcpy(buf[0..path.len], path);
for (buf[0..path.len]) |*c| {
if (c.* == undesired_slash) {
c.* = desired_slash;
}
}
return buf[0..utf8.len];
return buf[0..path.len];
}

return utf8;
if (comptime always_copy) {
@memcpy(buf[0..path.len], path);
return buf[0..path.len];
}
return path;
}

pub fn normalizeSlashesOnly(buf: []u8, utf8: []const u8, comptime desired_slash: u8) []const u8 {
return normalizeSlashesOnlyT(u8, buf, utf8, desired_slash, false);
}

pub fn toWDirNormalized(wbuf: []u16, utf8: []const u8) [:0]const u16 {
Expand Down
13 changes: 7 additions & 6 deletions src/sys.zig
Original file line number Diff line number Diff line change
Expand Up @@ -772,7 +772,8 @@ pub fn fstatat(fd: bun.FileDescriptor, path: [:0]const u8) Maybe(bun.Stat) {
};
}
var stat_ = mem.zeroes(bun.Stat);
if (Maybe(bun.Stat).errnoSysFP(syscall.fstatat(fd.int(), path, &stat_, 0), .fstatat, fd, path)) |err| {
const fd_valid = if (fd == bun.invalid_fd) std.posix.AT.FDCWD else fd.int();
if (Maybe(bun.Stat).errnoSysFP(syscall.fstatat(fd_valid, path, &stat_, 0), .fstatat, fd, path)) |err| {
log("fstatat({}, {s}) = {s}", .{ fd, path, @tagName(err.getErrno()) });
return err;
}
Expand Down Expand Up @@ -889,6 +890,7 @@ pub fn normalizePathWindows(
dir_fd: bun.FileDescriptor,
path_: []const T,
buf: *bun.WPathBuffer,
comptime opts: struct { add_nt_prefix: bool = true },
) Maybe([:0]const u16) {
if (comptime T != u8 and T != u16) {
@compileError("normalizePathWindows only supports u8 and u16 character types");
Expand Down Expand Up @@ -918,7 +920,7 @@ pub fn normalizePathWindows(
}
}

const norm = bun.path.normalizeStringGenericTZ(u16, path, buf, .{ .add_nt_prefix = true, .zero_terminate = true });
const norm = bun.path.normalizeStringGenericTZ(u16, path, buf, .{ .add_nt_prefix = opts.add_nt_prefix, .zero_terminate = true });
return .{ .result = norm };
}

Expand Down Expand Up @@ -1116,7 +1118,7 @@ fn openDirAtWindowsT(
const wbuf = bun.WPathBufferPool.get();
defer bun.WPathBufferPool.put(wbuf);

const norm = switch (normalizePathWindows(T, dirFd, path, wbuf)) {
const norm = switch (normalizePathWindows(T, dirFd, path, wbuf, .{})) {
.err => |err| return .{ .err = err },
.result => |norm| norm,
};
Expand Down Expand Up @@ -1310,7 +1312,7 @@ pub fn openFileAtWindowsT(
const wbuf = bun.WPathBufferPool.get();
defer bun.WPathBufferPool.put(wbuf);

const norm = switch (normalizePathWindows(T, dirFd, path, wbuf)) {
const norm = switch (normalizePathWindows(T, dirFd, path, wbuf, .{})) {
.err => |err| return .{ .err = err },
.result => |norm| norm,
};
Expand Down Expand Up @@ -2843,10 +2845,9 @@ pub fn directoryExistsAt(dir: anytype, subpath: anytype) JSC.Maybe(bool) {
const wbuf = bun.WPathBufferPool.get();
defer bun.WPathBufferPool.put(wbuf);
const path = if (std.meta.Child(@TypeOf(subpath)) == u16)
bun.strings.addNTPathPrefixIfNeeded(wbuf, subpath)
bun.strings.toNTPath16(wbuf, subpath)
else
bun.strings.toNTPath(wbuf, subpath);
bun.path.dangerouslyConvertPathToWindowsInPlace(u16, path);

const path_len_bytes: u16 = @truncate(path.len * 2);
var nt_name = w.UNICODE_STRING{
Expand Down
44 changes: 44 additions & 0 deletions test/regression/issue/16474.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import { mkdirSync, writeFileSync } from "fs";
import { mkdir } from "fs/promises";
import { test, expect } from "bun:test";
import { tmpdirSync } from "harness";
import { join } from "path";

test("fs.mkdir recursive should not error on existing", async () => {
const testDir = tmpdirSync();

const dir1 = join(testDir, "test123");
expect(mkdirSync(dir1, { recursive: true })).toBe(dir1);
expect(mkdirSync(dir1, { recursive: true })).toBeUndefined();
expect(() => {
mkdirSync(dir1);
}).toThrow("EEXIST: file already exists");

// relative
expect(() => {
mkdirSync("123test", { recursive: true });
mkdirSync("123test", { recursive: true });

mkdirSync("123test/456test", { recursive: true });
mkdirSync("123test/456test", { recursive: true });
}).not.toThrow();

const dir2 = join(testDir, "test456");
expect(await mkdir(dir2)).toBeUndefined();
expect(await mkdir(dir2, { recursive: true })).toBeUndefined();

// nested
const dir3 = join(testDir, "test789", "nested");
expect(mkdirSync(dir3, { recursive: true })).toBe(join(testDir, "test789"));
expect(mkdirSync(dir3, { recursive: true })).toBeUndefined();

// file
const file = join(testDir, "test789", "file.txt");
writeFileSync(file, "hi");
expect(() => {
mkdirSync(file, { recursive: true });
}).toThrow("EEXIST: file already exists");
expect(async () => {
await mkdir(file, { recursive: true });
}).toThrow("EEXIST: file already exists");
});
Loading