Merge pull request #19081 from ianic/tar_case_sensitive

std.tar don't overwrite files on unpack

lib/std/tar.zig

@@ -550,31 +550,15 @@ pub fn pipeToFileSystem(dir: std.fs.Dir, reader: anytype, options: Options) !voi
                 const file_name = stripComponents(file.name, options.strip_components);
                 if (file_name.len == 0) return error.BadFileName;
 
-                const fs_file = dir.createFile(file_name, .{}) catch |err| switch (err) {
-                    error.FileNotFound => again: {
-                        const code = code: {
-                            if (std.fs.path.dirname(file_name)) |dir_name| {
-                                dir.makePath(dir_name) catch |code| break :code code;
-                                break :again dir.createFile(file_name, .{}) catch |code| {
-                                    break :code code;
-                                };
-                            }
-                            break :code err;
-                        };
-                        const d = options.diagnostics orelse return error.UnableToCreateFile;
-                        try d.errors.append(d.allocator, .{ .unable_to_create_file = .{
-                            .code = code,
-                            .file_name = try d.allocator.dupe(u8, file_name),
-                        } });
-                        break :again null;
-                    },
-                    else => |e| return e,
-                };
-                defer if (fs_file) |f| f.close();
-
-                if (fs_file) |f| {
-                    try file.write(f);
-                } else {
+                if (createDirAndFile(dir, file_name)) |fs_file| {
+                    defer fs_file.close();
+                    try file.write(fs_file);
+                } else |err| {
+                    const d = options.diagnostics orelse return err;
+                    try d.errors.append(d.allocator, .{ .unable_to_create_file = .{
+                        .code = err,
+                        .file_name = try d.allocator.dupe(u8, file_name),
+                    } });
                     try file.skip();
                 }
             },
@@ -585,21 +569,10 @@ pub fn pipeToFileSystem(dir: std.fs.Dir, reader: anytype, options: Options) !voi
                 // The data inside the symbolic link.
                 const link_name = file.link_name;
 
-                dir.symLink(link_name, file_name, .{}) catch |err| again: {
-                    const code = code: {
-                        if (err == error.FileNotFound) {
-                            if (std.fs.path.dirname(file_name)) |dir_name| {
-                                dir.makePath(dir_name) catch |code| break :code code;
-                                break :again dir.symLink(link_name, file_name, .{}) catch |code| {
-                                    break :code code;
-                                };
-                            }
-                        }
-                        break :code err;
-                    };
+                createDirAndSymlink(dir, link_name, file_name) catch |err| {
                     const d = options.diagnostics orelse return error.UnableToCreateSymLink;
                     try d.errors.append(d.allocator, .{ .unable_to_create_sym_link = .{
-                        .code = code,
+                        .code = err,
                         .file_name = try d.allocator.dupe(u8, file_name),
                         .link_name = try d.allocator.dupe(u8, link_name),
                     } });
@@ -610,6 +583,31 @@ pub fn pipeToFileSystem(dir: std.fs.Dir, reader: anytype, options: Options) !voi
     }
 }
 
+fn createDirAndFile(dir: std.fs.Dir, file_name: []const u8) !std.fs.File {
+    const fs_file = dir.createFile(file_name, .{ .exclusive = true }) catch |err| {
+        if (err == error.FileNotFound) {
+            if (std.fs.path.dirname(file_name)) |dir_name| {
+                try dir.makePath(dir_name);
+                return try dir.createFile(file_name, .{ .exclusive = true });
+            }
+        }
+        return err;
+    };
+    return fs_file;
+}
+
+fn createDirAndSymlink(dir: std.fs.Dir, link_name: []const u8, file_name: []const u8) !void {
+    dir.symLink(link_name, file_name, .{}) catch |err| {
+        if (err == error.FileNotFound) {
+            if (std.fs.path.dirname(file_name)) |dir_name| {
+                try dir.makePath(dir_name);
+                try dir.symLink(link_name, file_name, .{});
+            }
+        }
+        return err;
+    };
+}
+
 fn stripComponents(path: []const u8, count: u32) []const u8 {
     var i: usize = 0;
     var c = count;

lib/std/tar/test.zig

@@ -373,3 +373,118 @@ const Md5Writer = struct {
         return std.fmt.bytesToHex(s, .lower);
     }
 };
+
+test "tar should not overwrite existing file" {
+    // Starting from this folder structure:
+    // $ tree root
+    //    root
+    //    ├── a
+    //    │   └── b
+    //    │       └── c
+    //    │           └── file.txt
+    //    └── d
+    //        └── b
+    //            └── c
+    //                └── file.txt
+    //
+    // Packed with command:
+    // $ cd root; tar cf overwrite_file.tar *
+    // Resulting tar has following structure:
+    // $ tar tvf overwrite_file.tar
+    //  size path
+    //  0    a/
+    //  0    a/b/
+    //  0    a/b/c/
+    //  2    a/b/c/file.txt
+    //  0    d/
+    //  0    d/b/
+    //  0    d/b/c/
+    //  2    d/b/c/file.txt
+    //
+    // Note that there is no root folder in archive.
+    //
+    // With strip_components = 1 resulting unpacked folder was:
+    //  root
+    //     └── b
+    //         └── c
+    //             └── file.txt
+    //
+    // a/b/c/file.txt is overwritten with d/b/c/file.txt !!!
+    // This ensures that file is not overwritten.
+    //
+    const data = @embedFile("testdata/overwrite_file.tar");
+    var fsb = std.io.fixedBufferStream(data);
+
+    // Unpack with strip_components = 1 should fail
+    var root = std.testing.tmpDir(.{});
+    defer root.cleanup();
+    try testing.expectError(
+        error.PathAlreadyExists,
+        tar.pipeToFileSystem(root.dir, fsb.reader(), .{ .mode_mode = .ignore, .strip_components = 1 }),
+    );
+
+    // Unpack with strip_components = 0 should pass
+    fsb.reset();
+    var root2 = std.testing.tmpDir(.{});
+    defer root2.cleanup();
+    try tar.pipeToFileSystem(root2.dir, fsb.reader(), .{ .mode_mode = .ignore, .strip_components = 0 });
+}
+
+test "tar case sensitivity" {
+    // Mimicking issue #18089, this tar contains, same file name in two case
+    // sensitive name version. Should fail on case insensitive file systems.
+    //
+    // $ tar tvf 18089.tar
+    //     18089/
+    //     18089/alacritty/
+    //     18089/alacritty/darkermatrix.yml
+    //     18089/alacritty/Darkermatrix.yml
+    //
+    const data = @embedFile("testdata/18089.tar");
+    var fsb = std.io.fixedBufferStream(data);
+
+    var root = std.testing.tmpDir(.{});
+    defer root.cleanup();
+
+    tar.pipeToFileSystem(root.dir, fsb.reader(), .{ .mode_mode = .ignore, .strip_components = 1 }) catch |err| {
+        // on case insensitive fs we fail on overwrite existing file
+        try testing.expectEqual(error.PathAlreadyExists, err);
+        return;
+    };
+
+    // on case sensitive os both files are created
+    try testing.expect((try root.dir.statFile("alacritty/darkermatrix.yml")).kind == .file);
+    try testing.expect((try root.dir.statFile("alacritty/Darkermatrix.yml")).kind == .file);
+}
+
+test "tar pipeToFileSystem" {
+    // $ tar tvf
+    //    pipe_to_file_system_test/
+    //    pipe_to_file_system_test/b/
+    //    pipe_to_file_system_test/b/symlink -> ../a/file
+    //    pipe_to_file_system_test/a/
+    //    pipe_to_file_system_test/a/file
+    //    pipe_to_file_system_test/empty/
+    const data = @embedFile("testdata/pipe_to_file_system_test.tar");
+    var fsb = std.io.fixedBufferStream(data);
+
+    var root = std.testing.tmpDir(.{ .no_follow = true });
+    defer root.cleanup();
+
+    tar.pipeToFileSystem(root.dir, fsb.reader(), .{
+        .mode_mode = .ignore,
+        .strip_components = 1,
+        .exclude_empty_directories = true,
+    }) catch |err| {
+        // Skip on platform which don't support symlinks
+        if (err == error.UnableToCreateSymLink) return error.SkipZigTest;
+        return err;
+    };
+
+    try testing.expectError(error.FileNotFound, root.dir.statFile("empty"));
+    try testing.expect((try root.dir.statFile("a/file")).kind == .file);
+    // TODO is there better way to test symlink
+    try testing.expect((try root.dir.statFile("b/symlink")).kind == .file); // statFile follows symlink
+    var buf: [8]u8 = undefined;
+    _ = try root.dir.readLink("b/symlink", &buf);
+}