package: skip unpack errors on paths excluded by manifest

lib/std/tar.zig

@@ -21,50 +21,6 @@ const testing = std.testing;
 
 pub const output = @import("tar/output.zig");
 
-/// Provide this to receive detailed error messages.
-/// When this is provided, some errors which would otherwise be returned
-/// immediately will instead be added to this structure. The API user must check
-/// the errors in diagnostics to know whether the operation succeeded or failed.
-pub const Diagnostics = struct {
-    allocator: std.mem.Allocator,
-    errors: std.ArrayListUnmanaged(Error) = .{},
-
-    pub const Error = union(enum) {
-        unable_to_create_sym_link: struct {
-            code: anyerror,
-            file_name: []const u8,
-            link_name: []const u8,
-        },
-        unable_to_create_file: struct {
-            code: anyerror,
-            file_name: []const u8,
-        },
-        unsupported_file_type: struct {
-            file_name: []const u8,
-            file_type: Header.Kind,
-        },
-    };
-
-    pub fn deinit(d: *Diagnostics) void {
-        for (d.errors.items) |item| {
-            switch (item) {
-                .unable_to_create_sym_link => |info| {
-                    d.allocator.free(info.file_name);
-                    d.allocator.free(info.link_name);
-                },
-                .unable_to_create_file => |info| {
-                    d.allocator.free(info.file_name);
-                },
-                .unsupported_file_type => |info| {
-                    d.allocator.free(info.file_name);
-                },
-            }
-        }
-        d.errors.deinit(d.allocator);
-        d.* = undefined;
-    }
-};
-
 /// pipeToFileSystem options
 pub const PipeOptions = struct {
     /// Number of directory levels to skip when extracting files.
@@ -73,8 +29,6 @@ pub const PipeOptions = struct {
     mode_mode: ModeMode = .executable_bit_only,
     /// Prevents creation of empty directories.
     exclude_empty_directories: bool = false,
-    /// Collects error messages during unpacking
-    diagnostics: ?*Diagnostics = null,
 
     pub const ModeMode = enum {
         /// The mode from the tar file is completely ignored. Files are created
@@ -247,16 +201,13 @@ pub const IteratorOptions = struct {
     file_name_buffer: []u8,
     /// Use a buffer with length `std.fs.MAX_PATH_BYTES` to match file system capabilities.
     link_name_buffer: []u8,
-    /// Collects error messages during unpacking
-    diagnostics: ?*Diagnostics = null,
 };
 
 /// Iterates over files in tar archive.
 /// `next` returns each file in tar archive.
 pub fn iterator(reader: anytype, options: IteratorOptions) Iterator(@TypeOf(reader)) {
     return .{
         .reader = reader,
-        .diagnostics = options.diagnostics,
         .file_name_buffer = options.file_name_buffer,
         .link_name_buffer = options.link_name_buffer,
     };
@@ -273,7 +224,6 @@ pub const FileKind = enum {
 pub fn Iterator(comptime ReaderType: type) type {
     return struct {
         reader: ReaderType,
-        diagnostics: ?*Diagnostics = null,
 
         // buffers for heeader and file attributes
         header_buffer: [Header.SIZE]u8 = undefined,
@@ -435,15 +385,11 @@ pub fn Iterator(comptime ReaderType: type) type {
                     },
                     // All other are unsupported header types
                     else => {
-                        const d = self.diagnostics orelse return error.TarUnsupportedHeader;
-                        try d.errors.append(d.allocator, .{ .unsupported_file_type = .{
-                            .file_name = try d.allocator.dupe(u8, header.name()),
-                            .file_type = kind,
-                        } });
                         if (kind == .gnu_sparse) {
                             try self.skipGnuSparseExtendedHeaders(header);
                         }
                         self.reader.skipBytes(size, .{}) catch return error.TarHeadersTooBig;
+                        return error.TarUnsupportedHeader;
                     },
                 }
             }
@@ -573,24 +519,11 @@ fn PaxIterator(comptime ReaderType: type) type {
 
 /// Saves tar file content to the file systems.
 pub fn pipeToFileSystem(dir: std.fs.Dir, reader: anytype, options: PipeOptions) !void {
-    switch (options.mode_mode) {
-        .ignore => {},
-        .executable_bit_only => {
-            // This code does not look at the mode bits yet. To implement this feature,
-            // the implementation must be adjusted to look at the mode, and check the
-            // user executable bit, then call fchmod on newly created files when
-            // the executable bit is supposed to be set.
-            // It also needs to properly deal with ACLs on Windows.
-            @panic("TODO: unimplemented: tar ModeMode.executable_bit_only");
-        },
-    }
-
     var file_name_buffer: [std.fs.MAX_PATH_BYTES]u8 = undefined;
     var link_name_buffer: [std.fs.MAX_PATH_BYTES]u8 = undefined;
     var iter = iterator(reader, .{
         .file_name_buffer = &file_name_buffer,
         .link_name_buffer = &link_name_buffer,
-        .diagnostics = options.diagnostics,
     });
     while (try iter.next()) |file| {
         switch (file.kind) {
@@ -605,16 +538,9 @@ pub fn pipeToFileSystem(dir: std.fs.Dir, reader: anytype, options: PipeOptions)
                 const file_name = stripComponents(file.name, options.strip_components);
                 if (file_name.len == 0) return error.BadFileName;
 
-                if (createDirAndFile(dir, file_name)) |fs_file| {
-                    defer fs_file.close();
-                    try file.writeAll(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),
-                    } });
-                }
+                var fs_file = try createDirAndFile(dir, file_name, fileMode(file.mode, options));
+                defer fs_file.close();
+                try file.writeAll(fs_file);
             },
             .sym_link => {
                 // The file system path of the symbolic link.
@@ -623,25 +549,37 @@ pub fn pipeToFileSystem(dir: std.fs.Dir, reader: anytype, options: PipeOptions)
                 // The data inside the symbolic link.
                 const link_name = file.link_name;
 
-                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 = err,
-                        .file_name = try d.allocator.dupe(u8, file_name),
-                        .link_name = try d.allocator.dupe(u8, link_name),
-                    } });
-                };
+                createDirAndSymlink(dir, link_name, file_name) catch return error.UnableToCreateSymLink;
             },
         }
     }
 }
 
-fn createDirAndFile(dir: std.fs.Dir, file_name: []const u8) !std.fs.File {
-    const fs_file = dir.createFile(file_name, .{ .exclusive = true }) catch |err| {
+const default_mode = std.fs.File.default_mode;
+
+fn fileMode(mode: u32, options: PipeOptions) std.fs.File.Mode {
+    if (!std.fs.has_executable_bit or options.mode_mode == .ignore)
+        return default_mode;
+    const S = std.posix.S;
+    if (mode & S.IXUSR == 0)
+        return default_mode;
+    return default_mode | S.IXUSR | S.IXGRP | S.IXOTH;
+}
+
+test fileMode {
+    if (!std.fs.has_executable_bit) return error.SkipZigTest;
+    try testing.expectEqual(default_mode, fileMode(0o744, PipeOptions{ .mode_mode = .ignore }));
+    try testing.expectEqual(0o777, fileMode(0o744, PipeOptions{}));
+    try testing.expectEqual(0o666, fileMode(0o644, PipeOptions{}));
+    try testing.expectEqual(0o666, fileMode(0o655, PipeOptions{}));
+}
+
+fn createDirAndFile(dir: std.fs.Dir, file_name: []const u8, mode: std.fs.File.Mode) !std.fs.File {
+    const fs_file = dir.createFile(file_name, .{ .exclusive = true, .mode = mode }) 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 try dir.createFile(file_name, .{ .exclusive = true, .mode = mode });
             }
         }
         return err;
@@ -877,9 +815,9 @@ test "create file and symlink" {
     var root = testing.tmpDir(.{});
     defer root.cleanup();
 
-    var file = try createDirAndFile(root.dir, "file1");
+    var file = try createDirAndFile(root.dir, "file1", default_mode);
     file.close();
-    file = try createDirAndFile(root.dir, "a/b/c/file2");
+    file = try createDirAndFile(root.dir, "a/b/c/file2", default_mode);
     file.close();
 
     createDirAndSymlink(root.dir, "a/b/c/file2", "symlink1") catch |err| {
@@ -891,7 +829,7 @@ test "create file and symlink" {
 
     // Danglink symlnik, file created later
     try createDirAndSymlink(root.dir, "../../../g/h/i/file4", "j/k/l/symlink3");
-    file = try createDirAndFile(root.dir, "g/h/i/file4");
+    file = try createDirAndFile(root.dir, "g/h/i/file4", default_mode);
     file.close();
 }
 
@@ -984,9 +922,9 @@ test pipeToFileSystem {
 
     // Save tar from `reader` to the file system `dir`
     pipeToFileSystem(dir, reader, .{
-        .mode_mode = .ignore,
         .strip_components = 1,
         .exclude_empty_directories = true,
+        .mode_mode = .ignore,
     }) catch |err| {
         // Skip on platform which don't support symlinks
         if (err == error.UnableToCreateSymLink) return error.SkipZigTest;
@@ -1005,6 +943,45 @@ test pipeToFileSystem {
     );
 }
 
+test "executable bit" {
+    if (!std.fs.has_executable_bit) return error.SkipZigTest;
+    const S = std.posix.S;
+
+    const data = @embedFile("tar/testdata/example.tar");
+
+    for ([_]PipeOptions.ModeMode{ .ignore, .executable_bit_only }) |opt| {
+        var fbs = std.io.fixedBufferStream(data);
+        const reader = fbs.reader();
+
+        var tmp = testing.tmpDir(.{ .no_follow = true });
+        defer tmp.cleanup();
+
+        pipeToFileSystem(tmp.dir, reader, .{
+            .strip_components = 1,
+            .exclude_empty_directories = true,
+            .mode_mode = opt,
+        }) catch |err| {
+            // Skip on platform which don't support symlinks
+            if (err == error.UnableToCreateSymLink) return error.SkipZigTest;
+            return err;
+        };
+
+        const fs = try tmp.dir.statFile("a/file");
+        try testing.expect(fs.kind == .file);
+        if (opt == .executable_bit_only) {
+            // Executable bit is set for user, group and others
+            try testing.expect(fs.mode & S.IXUSR > 0);
+            try testing.expect(fs.mode & S.IXGRP > 0);
+            try testing.expect(fs.mode & S.IXOTH > 0);
+        }
+        if (opt == .ignore) {
+            try testing.expect(fs.mode & S.IXUSR == 0);
+            try testing.expect(fs.mode & S.IXGRP == 0);
+            try testing.expect(fs.mode & S.IXOTH == 0);
+        }
+    }
+}
+
 fn normalizePath(bytes: []u8) []u8 {
     const canonical_sep = std.fs.path.sep_posix;
     if (std.fs.path.sep == canonical_sep) return bytes;

lib/std/tar/test.zig

@@ -473,14 +473,14 @@ test "should not overwrite existing file" {
     defer root.cleanup();
     try testing.expectError(
         error.PathAlreadyExists,
-        tar.pipeToFileSystem(root.dir, fsb.reader(), .{ .mode_mode = .ignore, .strip_components = 1 }),
+        tar.pipeToFileSystem(root.dir, fsb.reader(), .{ .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 });
+    try tar.pipeToFileSystem(root2.dir, fsb.reader(), .{ .strip_components = 0 });
 }
 
 test "case sensitivity" {
@@ -499,7 +499,7 @@ test "case sensitivity" {
     var root = std.testing.tmpDir(.{});
     defer root.cleanup();
 
-    tar.pipeToFileSystem(root.dir, fsb.reader(), .{ .mode_mode = .ignore, .strip_components = 1 }) catch |err| {
+    tar.pipeToFileSystem(root.dir, fsb.reader(), .{ .strip_components = 1 }) catch |err| {
         // on case insensitive fs we fail on overwrite existing file
         try testing.expectEqual(error.PathAlreadyExists, err);
         return;