| author |  Amir Alawi <83512437+amiralawi@users.noreply.github.com> 2024-01-08 15:58:14-05:00 |
| committer |  GitHub <noreply@github.com> 2024-01-08 15:58:14-05:00 |
| log | 4cbf74bd9b839501a4fd4bc5f39a25e216237b3c |
| tree | 6cc25456391073e1820167cf896c2cf710dd18dc |
| parent | ed410b9c1e9a09e674f862b5f3af9d207837a472 |
| signature |  Signed by PGP key 4AEE18F83AFDEB23 |
fix std.fs.Dir.makePath silent failure (#16878)
std.fs.dir.makePath silently failed 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.2 files changed, 19 insertions(+), 1 deletions(-)
lib/std/fs/Dir.zig+9-1
| ... | @@ -1125,9 +1125,17 @@ pub fn makePath(self: Dir, sub_path: []const u8) !void { | ... | @@ -1125,9 +1125,17 @@ pub fn makePath(self: Dir, sub_path: []const u8) !void { |
| 1125 | while (true) { | 1125 | while (true) { |
| 1126 | self.makeDir(component.path) catch |err| switch (err) { | 1126 | self.makeDir(component.path) catch |err| switch (err) { |
| 1127 | error.PathAlreadyExists => { | 1127 | error.PathAlreadyExists => { |
| 1128 | // TODO stat the file and return an error if it's not a directory | 1128 | // stat the file and return an error if it's not a directory |
| 1129 | // this is important because otherwise a dangling symlink | 1129 | // this is important because otherwise a dangling symlink |
| 1130 | // could cause an infinite loop | 1130 | // could cause an infinite loop |
| 1131 | check_dir: { | ||
| 1132 | // workaround for windows, see https://github.com/ziglang/zig/issues/16738 | ||
| 1133 | const fstat = self.statFile(component.path) catch |stat_err| switch (stat_err) { | ||
| 1134 | error.IsDir => break :check_dir, | ||
| 1135 | else => |e| return e, | ||
| 1136 | }; | ||
| 1137 | if (fstat.kind != .directory) return error.NotDir; | ||
| 1138 | } | ||
| 1131 | }, | 1139 | }, |
| 1132 | error.FileNotFound => |e| { | 1140 | error.FileNotFound => |e| { |
| 1133 | component = it.previous() orelse return e; | 1141 | component = it.previous() orelse return e; |
lib/std/fs/test.zig+10
| ... | @@ -1089,6 +1089,16 @@ test "makePath in a directory that no longer exists" { | ... | @@ -1089,6 +1089,16 @@ test "makePath in a directory that no longer exists" { |
| 1089 | try testing.expectError(error.FileNotFound, tmp.dir.makePath("sub-path")); | 1089 | try testing.expectError(error.FileNotFound, tmp.dir.makePath("sub-path")); |
| 1090 | } | 1090 | } |
| 1091 | 1091 | ||
| 1092 | test "makePath but sub_path contains pre-existing file" { | ||
| 1093 | var tmp = tmpDir(.{}); | ||
| 1094 | defer tmp.cleanup(); | ||
| 1095 | |||
| 1096 | try tmp.dir.makeDir("foo"); | ||
| 1097 | try tmp.dir.writeFile("foo/bar", ""); | ||
| 1098 | |||
| 1099 | try testing.expectError(error.NotDir, tmp.dir.makePath("foo/bar/baz")); | ||
| 1100 | } | ||
| 1101 | |||
| 1092 | fn expectDir(dir: Dir, path: []const u8) !void { | 1102 | fn expectDir(dir: Dir, path: []const u8) !void { |
| 1093 | var d = try dir.openDir(path, .{}); | 1103 | var d = try dir.openDir(path, .{}); |
| 1094 | d.close(); | 1104 | d.close(); |