From 4f47ae5866fef387d0f6e4e78da6bc4edff54b74 Mon Sep 17 00:00:00 2001 From: Amir Alawi <83512437+amiralawi@users.noreply.github.com> Date: Fri, 18 Aug 2023 17:27:52 -0400 Subject: [PATCH 1/4] fix std.fs.Dir.makePath silent failure std.fs.dir.makePath silently fails if one of the items in the path already exists. For example: cwd.makePath("foo/bar/baz") Silently failing is OK if "bar" is already a directory - this is the intended use of makePath (like mkdir -p). But if bar is a file then the subdirectory baz cannot be created - the end result is that makePath doesn't do anything which should be a detectable error because baz is never created. The existing code had a TODO comment that did not specifically cover this error, but the solution for this silent failure also accomplishes the TODO task - the code now stats "foo" and returns an appropriate error. The new code also handles potential race condition if "bar" is deleted/permissions changed/etc in between the initial makeDir and statFile calls. --- lib/std/fs/Dir.zig | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/std/fs/Dir.zig b/lib/std/fs/Dir.zig index 8a1d38996eb6..0f996affcc93 100644 --- a/lib/std/fs/Dir.zig +++ b/lib/std/fs/Dir.zig @@ -1125,9 +1125,17 @@ pub fn makePath(self: Dir, sub_path: []const u8) !void { while (true) { self.makeDir(component.path) catch |err| switch (err) { error.PathAlreadyExists => { - // TODO stat the file and return an error if it's not a directory + // stat the file and return an error if it's not a directory // this is important because otherwise a dangling symlink // could cause an infinite loop + check_dir: { + // workaround for windows, see https://github.com/ziglang/zig/issues/16738 + const fstat = self.statFile(component.path) catch |stat_err| switch (stat_err) { + error.IsDir => break :check_dir, + else => |e| return e, + }; + if (fstat.kind != .directory) return error.NotDir; + } }, error.FileNotFound => |e| { component = it.previous() orelse return e; From 4418622d1220e55d8fff4133dd8b124f9d1f0504 Mon Sep 17 00:00:00 2001 From: Amir Alawi <83512437+amiralawi@users.noreply.github.com> Date: Sat, 19 Aug 2023 17:46:35 -0400 Subject: [PATCH 2/4] Added test --- lib/std/fs/test.zig | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/lib/std/fs/test.zig b/lib/std/fs/test.zig index 0cdcd0a631e4..d8a5fc8dc155 100644 --- a/lib/std/fs/test.zig +++ b/lib/std/fs/test.zig @@ -1089,6 +1089,25 @@ test "makePath in a directory that no longer exists" { try testing.expectError(error.FileNotFound, tmp.dir.makePath("sub-path")); } +test "makePath but sub_path contains pre-existing file" { + // makePath tmp/foo/bar/baz, but bar is a file + var buf: [std.fs.MAX_PATH_BYTES]u8 = undefined; + var fba = std.heap.FixedBufferAllocator.init(&buf); + var buf_alloc = fba.allocator(); + + var tmp = tmpDir(.{}); + defer tmp.cleanup(); + + try tmp.dir.makeDir("foo"); + var foo = try tmp.dir.openDir("foo", .{}); + defer foo.close(); + var bar = try foo.createFile("bar", .{}); + defer bar.close(); + + var sub_path = try std.fs.path.join(buf_alloc, &[_][]const u8{"foo", "bar", "baz"}); + try testing.expectError(error.NotDir, tmp.dir.makePath(sub_path)); +} + fn expectDir(dir: Dir, path: []const u8) !void { var d = try dir.openDir(path, .{}); d.close(); From 8bea454a6088ba853633bbb0a6e4c33a9137c207 Mon Sep 17 00:00:00 2001 From: Amir Alawi <83512437+amiralawi@users.noreply.github.com> Date: Sun, 20 Aug 2023 23:22:37 -0400 Subject: [PATCH 3/4] test simplification --- lib/std/fs/test.zig | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/lib/std/fs/test.zig b/lib/std/fs/test.zig index d8a5fc8dc155..2c6a8eaa5e2b 100644 --- a/lib/std/fs/test.zig +++ b/lib/std/fs/test.zig @@ -1090,22 +1090,13 @@ test "makePath in a directory that no longer exists" { } test "makePath but sub_path contains pre-existing file" { - // makePath tmp/foo/bar/baz, but bar is a file - var buf: [std.fs.MAX_PATH_BYTES]u8 = undefined; - var fba = std.heap.FixedBufferAllocator.init(&buf); - var buf_alloc = fba.allocator(); - var tmp = tmpDir(.{}); defer tmp.cleanup(); - + try tmp.dir.makeDir("foo"); - var foo = try tmp.dir.openDir("foo", .{}); - defer foo.close(); - var bar = try foo.createFile("bar", .{}); - defer bar.close(); + try tmp.dir.writeFile("foo" ++ fs.path.sep_str ++ "bar", ""); - var sub_path = try std.fs.path.join(buf_alloc, &[_][]const u8{"foo", "bar", "baz"}); - try testing.expectError(error.NotDir, tmp.dir.makePath(sub_path)); + try testing.expectError(error.NotDir, tmp.dir.makePath("foo/bar/baz")); } fn expectDir(dir: Dir, path: []const u8) !void { From 3f6c199a68a7e1ee3a57f60d25bf2203ff381be6 Mon Sep 17 00:00:00 2001 From: Amir Alawi <83512437+amiralawi@users.noreply.github.com> Date: Mon, 21 Aug 2023 09:53:37 -0400 Subject: [PATCH 4/4] whitespace formatting + path consistency --- lib/std/fs/test.zig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/std/fs/test.zig b/lib/std/fs/test.zig index 2c6a8eaa5e2b..4a8cc19517c5 100644 --- a/lib/std/fs/test.zig +++ b/lib/std/fs/test.zig @@ -1094,8 +1094,8 @@ test "makePath but sub_path contains pre-existing file" { defer tmp.cleanup(); try tmp.dir.makeDir("foo"); - try tmp.dir.writeFile("foo" ++ fs.path.sep_str ++ "bar", ""); - + try tmp.dir.writeFile("foo/bar", ""); + try testing.expectError(error.NotDir, tmp.dir.makePath("foo/bar/baz")); }