Skip to content

Commit

Permalink
feat: Avoid overriding existing files
Browse files Browse the repository at this point in the history
This solution is not perfect. There are two places where, if someone is
fast enough, existing files could be overwritten.
  • Loading branch information
Hejsil committed Mar 7, 2025
1 parent 57079d8 commit f7c24f9
Show file tree
Hide file tree
Showing 5 changed files with 217 additions and 49 deletions.
31 changes: 31 additions & 0 deletions src/Diagnostics.zig
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ failures: struct {
hash_mismatches: std.ArrayListUnmanaged(HashMismatch),
downloads: std.ArrayListUnmanaged(DownloadFailed),
downloads_with_status: std.ArrayListUnmanaged(DownloadFailedWithStatus),
path_already_exists: std.ArrayListUnmanaged(PathAlreadyExists),
},

pub fn init(allocator: std.mem.Allocator) Diagnostics {
Expand All @@ -45,6 +46,7 @@ pub fn init(allocator: std.mem.Allocator) Diagnostics {
.hash_mismatches = .{},
.downloads = .{},
.downloads_with_status = .{},
.path_already_exists = .{},
},
};
}
Expand Down Expand Up @@ -239,6 +241,15 @@ pub fn report(diagnostics: *Diagnostics, writer: anytype, opt: ReportOptions) !v
});
try writer.print("└── No version found: {s}\n", .{@errorName(no_version.err)});
}
for (diagnostics.failures.path_already_exists.items) |err| {
try writer.print("{s} {s}{s}{s}\n", .{
failure,
esc.bold,
err.name,
esc.reset,
});
try writer.print("└── Path already exists: {s}\n", .{err.path});
}
for (diagnostics.successes.donate.items) |package| {
try writer.print("{s} {s}{s} {s}{s}\n", .{
success,
Expand Down Expand Up @@ -432,6 +443,17 @@ pub fn downloadFailedWithStatus(
});
}

pub fn pathAlreadyExists(diagnostics: *Diagnostics, failure: PathAlreadyExists) !void {
diagnostics.lock.lock();
defer diagnostics.lock.unlock();

const arena = diagnostics.arena.allocator();
return diagnostics.failures.path_already_exists.append(diagnostics.gpa(), .{
.name = try arena.dupe(u8, failure.name),
.path = try arena.dupe(u8, failure.path),
});
}

pub const Package = struct {
name: []const u8,
};
Expand Down Expand Up @@ -484,6 +506,15 @@ pub const DownloadFailedWithStatus = struct {
status: std.http.Status,
};

pub const PathAlreadyExists = struct {
name: []const u8,
path: []const u8,
};

pub const Error = error{
DiagnosticsReported,
};

test {
_ = Escapes;
_ = Target;
Expand Down
138 changes: 98 additions & 40 deletions src/PackageManager.zig
Original file line number Diff line number Diff line change
Expand Up @@ -155,8 +155,7 @@ pub fn installMany(pm: *PackageManager, package_names: []const []const u8) !void
// If this fails, packages can be left in a partially updated state
for (downloads.jobs.items) |downloaded| {
downloaded.result catch |err| switch (err) {
DiagnosticError.DiagnosticsInvalidHash => continue,
DiagnosticError.DiagnosticsDownloadFailed => continue,
Diagnostics.Error.DiagnosticsReported => continue,
else => |e| return e,
};

Expand Down Expand Up @@ -246,7 +245,7 @@ fn downloadAndExtractPackage(
.url = package.install.url,
.err = err,
});
return DiagnosticError.DiagnosticsDownloadFailed;
return Diagnostics.Error.DiagnosticsReported;
};
if (download_result.status != .ok) {
try pm.diagnostics.downloadFailedWithStatus(.{
Expand All @@ -255,7 +254,7 @@ fn downloadAndExtractPackage(
.url = package.install.url,
.status = download_result.status,
});
return DiagnosticError.DiagnosticsDownloadFailed;
return Diagnostics.Error.DiagnosticsReported;
}

const actual_hash = std.fmt.bytesToHex(download_result.hash, .lower);
Expand All @@ -266,7 +265,7 @@ fn downloadAndExtractPackage(
.expected_hash = package.install.hash,
.actual_hash = &actual_hash,
});
return DiagnosticError.DiagnosticsInvalidHash;
return Diagnostics.Error.DiagnosticsReported;
}
}

Expand Down Expand Up @@ -310,20 +309,45 @@ fn installExtractedPackage(
for (package.install.install_bin) |install_field| {
const the_install = Package.Install.fromString(install_field);
const path = try std.fs.path.join(arena, &.{ paths.bin_subpath, the_install.to });
try installBin(the_install, from_dir, pm.bin_dir);
locations.appendAssumeCapacity(path);
}
for (package.install.install_lib) |install_field| {
const the_install = Package.Install.fromString(install_field);
const path = try std.fs.path.join(arena, &.{ paths.lib_subpath, the_install.to });
try installGeneric(the_install, from_dir, pm.lib_dir);
installBin(the_install, from_dir, pm.bin_dir) catch |err| switch (err) {
error.PathAlreadyExists => {
try pm.diagnostics.pathAlreadyExists(.{
.name = package.name,
.path = path,
});
return Diagnostics.Error.DiagnosticsReported;
},
else => |e| return e,
};
locations.appendAssumeCapacity(path);
}
for (package.install.install_share) |install_field| {
const the_install = Package.Install.fromString(install_field);
const path = try std.fs.path.join(arena, &.{ paths.share_subpath, the_install.to });
try installGeneric(the_install, from_dir, pm.share_dir);
locations.appendAssumeCapacity(path);

const GenericInstall = struct {
dir: std.fs.Dir,
path: []const u8,
installs: []const []const u8,
};

const generic_installs = [_]GenericInstall{
.{ .dir = pm.lib_dir, .path = paths.lib_subpath, .installs = package.install.install_lib },
.{ .dir = pm.share_dir, .path = paths.share_subpath, .installs = package.install.install_share },
};
for (generic_installs) |install| {
for (install.installs) |install_field| {
const the_install = Package.Install.fromString(install_field);
const path = try std.fs.path.join(arena, &.{ install.path, the_install.to });
installGeneric(the_install, from_dir, install.dir) catch |err| switch (err) {
error.PathAlreadyExists => {
try pm.diagnostics.pathAlreadyExists(.{
.name = package.name,
.path = path,
});
return Diagnostics.Error.DiagnosticsReported;
},
else => |e| return e,
};
locations.appendAssumeCapacity(path);
}
}

// Caller ensures that package is not installed
Expand All @@ -335,15 +359,7 @@ fn installExtractedPackage(
}

fn installBin(the_install: Package.Install, from_dir: std.fs.Dir, to_dir: std.fs.Dir) !void {
try from_dir.copyFile(the_install.from, to_dir, the_install.to, .{});

const installed_file = try to_dir.openFile(the_install.to, .{});
defer installed_file.close();

const metadata = try installed_file.metadata();
var permissions = metadata.permissions();
permissions.inner.unixSet(.user, .{ .execute = true });
try installed_file.setPermissions(permissions);
return installFile(the_install, from_dir, to_dir, .{ .executable = true });
}

fn installGeneric(the_install: Package.Install, from_dir: std.fs.Dir, to_dir: std.fs.Dir) !void {
Expand All @@ -353,20 +369,28 @@ fn installGeneric(the_install: Package.Install, from_dir: std.fs.Dir, to_dir: st
.directory => {
var child_from_dir = try from_dir.openDir(the_install.from, .{ .iterate = true });
defer child_from_dir.close();
var child_to_dir = try to_dir.makeOpenPath(the_install.to, .{});
defer child_to_dir.close();

try fs.copyTree(child_from_dir, child_to_dir);
},
.sym_link, .file => {
const install_base_name = std.fs.path.basename(the_install.to);
const child_to_dir_path = std.fs.path.dirname(the_install.to) orelse ".";
var child_to_dir = try to_dir.makeOpenPath(child_to_dir_path, .{});
defer child_to_dir.close();

try from_dir.copyFile(the_install.from, child_to_dir, install_base_name, .{});
},
var tmp_dir = try fs.tmpDir(child_to_dir, .{});
defer tmp_dir.deleteAndClose();

try fs.copyTree(child_from_dir, tmp_dir.dir);

if (fs.exists(child_to_dir, install_base_name))
return error.PathAlreadyExists;

// RACE: If something is fast enough, it could write to `the_install.to` before
// `rename` completes. In that case, their content will be overwritten. To
// prevent this, we would need to copy to a temp file, then `renameat2` with
// `RENAME_NOREPLACE`

try child_to_dir.rename(&tmp_dir.name, install_base_name);
},
.sym_link, .file => return installFile(the_install, from_dir, to_dir, .{}),
.block_device,
.character_device,
.named_pipe,
Expand All @@ -379,6 +403,46 @@ fn installGeneric(the_install: Package.Install, from_dir: std.fs.Dir, to_dir: st
}
}

const InstallFileOptions = struct {
executable: bool = false,
};

fn installFile(
the_install: Package.Install,
from_dir: std.fs.Dir,
to_dir: std.fs.Dir,
options: InstallFileOptions,
) !void {
const install_base_name = std.fs.path.basename(the_install.to);
const child_to_dir_path = std.fs.path.dirname(the_install.to) orelse ".";
var child_to_dir = try to_dir.makeOpenPath(child_to_dir_path, .{});
defer child_to_dir.close();

var from_file = try from_dir.openFile(the_install.from, .{});
defer from_file.close();

var to_file = try child_to_dir.atomicFile(install_base_name, .{});
defer to_file.deinit();

_ = try from_file.copyRangeAll(0, to_file.file, 0, std.math.maxInt(u64));

if (options.executable) {
const metadata = try to_file.file.metadata();
var permissions = metadata.permissions();
permissions.inner.unixSet(.user, .{ .execute = true });
try to_file.file.setPermissions(permissions);
}

if (fs.exists(child_to_dir, install_base_name))
return error.PathAlreadyExists;

// RACE: If something is fast enough, it could write to `the_install.to` before `finish`
// completes the rename. In that case, their content will be overwritten. To prevent
// this, we would need to copy to a temp file, then `renameat2` with `RENAME_NOREPLACE`

try to_file.finish();
}

pub fn uninstallMany(pm: *PackageManager, package_names: []const []const u8) !void {
var packages_to_uninstall = try pm.packagesToUninstall(package_names);
defer packages_to_uninstall.deinit();
Expand Down Expand Up @@ -499,8 +563,7 @@ fn updatePackages(pm: *PackageManager, package_names: []const []const u8, option
// If this fails, packages can be left in a partially updated state
for (downloads.jobs.items) |downloaded| {
downloaded.result catch |err| switch (err) {
DiagnosticError.DiagnosticsInvalidHash => continue,
DiagnosticError.DiagnosticsDownloadFailed => continue,
Diagnostics.Error.DiagnosticsReported => continue,
else => |e| return e,
};

Expand Down Expand Up @@ -583,11 +646,6 @@ const DownloadAndExtractJob = struct {
}
};

const DiagnosticError = error{
DiagnosticsDownloadFailed,
DiagnosticsInvalidHash,
};

test {
_ = Diagnostics;
_ = InstalledPackage;
Expand Down
9 changes: 9 additions & 0 deletions src/fs.zig
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,15 @@ pub fn copyTree(from_dir: std.fs.Dir, to_dir: std.fs.Dir) !void {
};
}

pub fn exists(dir: std.fs.Dir, path: []const u8) bool {
_ = dir.statFile(path) catch |err| switch (err) {
error.FileNotFound => return false,
else => return true,
};

return true;
}

test {
_ = Progress;

Expand Down
8 changes: 2 additions & 6 deletions src/main.zig
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ pub fn main() !u8 {
defer std.process.argsFree(gpa, args);

mainFull(.{ .gpa = gpa, .args = args[1..] }) catch |err| switch (err) {
DiagnosticsError.DiagnosticFailure => return 1,
Diagnostics.Error.DiagnosticsReported => return 1,
else => |e| {
if (builtin.mode == .Debug)
return e;
Expand All @@ -20,10 +20,6 @@ pub fn main() !u8 {
return 0;
}

pub const DiagnosticsError = error{
DiagnosticFailure,
};

pub const MainOptions = struct {
gpa: std.mem.Allocator,
args: []const []const u8,
Expand Down Expand Up @@ -82,7 +78,7 @@ pub fn mainFull(options: MainOptions) !void {
try diag.reportToFile(program.stderr);

if (diag.hasFailed())
return DiagnosticsError.DiagnosticFailure;
return Diagnostics.Error.DiagnosticsReported;

return res;
}
Expand Down
Loading

0 comments on commit f7c24f9

Please sign in to comment.