Skip to content

std.tar don't overwrite files on unpack #19081

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.

Sign up for GitHub

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

Merged
andrewrk merged 5 commits into from
Feb 26, 2024
Merged

std.tar don't overwrite files on unpack #19081

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b84301c
std.tar don't overwrite existing file
ianic Feb 24, 2024
96e4d56
std.tar add case sensitive file name test
ianic Feb 25, 2024
30a319b
std.tar improve error reporting
ianic Feb 25, 2024
f086ea8
std.tar skip test on windows
ianic Feb 25, 2024
65e5c46
std.tar fix refactored function
ianic Feb 25, 2024
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
74 changes: 36 additions & 38 deletions lib/std/tar.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -544,31 +544,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();
}
},
Expand All @@ -579,21 +563,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),
} });
Expand All @@ -604,6 +577,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;
Expand Down
115 changes: 115 additions & 0 deletions lib/std/tar/test.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -369,3 +369,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);
}
Binary file added lib/std/tar/testdata/18089.tar
View file
Open in desktop
Binary file not shown.
Binary file added lib/std/tar/testdata/overwrite_file.tar
View file
Open in desktop
Binary file not shown.
Binary file added lib/std/tar/testdata/pipe_to_file_system_test.tar
View file
Open in desktop
Binary file not shown.