Skip to content

zig fmt: Fix relative paths with . and .. on Windows as well as forward slashes #5187

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.

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
merged 3 commits into from
Apr 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 7 additions & 3 deletions lib/std/io/buffered_atomic_file.zig
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,12 @@ pub const BufferedAtomicFile = struct {

/// TODO when https://github.com/ziglang/zig/issues/2761 is solved
/// this API will not need an allocator
/// TODO integrate this with Dir API
pub fn create(allocator: *mem.Allocator, dest_path: []const u8) !*BufferedAtomicFile {
pub fn create(
allocator: *mem.Allocator,
dir: fs.Dir,
dest_path: []const u8,
atomic_file_options: fs.Dir.AtomicFileOptions,
) !*BufferedAtomicFile {
var self = try allocator.create(BufferedAtomicFile);
self.* = BufferedAtomicFile{
.atomic_file = undefined,
Expand All @@ -26,7 +30,7 @@ pub const BufferedAtomicFile = struct {
};
errdefer allocator.destroy(self);

self.atomic_file = try fs.cwd().atomicFile(dest_path, .{});
self.atomic_file = try dir.atomicFile(dest_path, atomic_file_options);
errdefer self.atomic_file.deinit();

self.file_stream = self.atomic_file.file.outStream();
Expand Down
19 changes: 12 additions & 7 deletions lib/std/os/windows.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1260,15 +1260,9 @@ pub fn wToPrefixedFileW(s: []const u16) ![PATH_MAX_WIDE:0]u16 {
pub fn sliceToPrefixedSuffixedFileW(s: []const u8, comptime suffix: []const u16) ![PATH_MAX_WIDE + suffix.len:0]u16 {
// TODO https://github.com/ziglang/zig/issues/2765
var result: [PATH_MAX_WIDE + suffix.len:0]u16 = undefined;
// > File I/O functions in the Windows API convert "/" to "\" as part of
// > converting the name to an NT-style name, except when using the "\\?\"
// > prefix as detailed in the following sections.
// from https://docs.microsoft.com/en-us/windows/desktop/FileIO/naming-a-file#maximum-path-length-limitation
// Because we want the larger maximum path length for absolute paths, we
// disallow forward slashes in zig std lib file functions on Windows.
for (s) |byte| {
switch (byte) {
'/', '*', '?', '"', '<', '>', '|' => return error.BadPathName,
'*', '?', '"', '<', '>', '|' => return error.BadPathName,
else => {},
}
}
Expand All @@ -1279,6 +1273,17 @@ pub fn sliceToPrefixedSuffixedFileW(s: []const u8, comptime suffix: []const u16)
};
const end_index = start_index + try std.unicode.utf8ToUtf16Le(result[start_index..], s);
if (end_index + suffix.len > result.len) return error.NameTooLong;
// > File I/O functions in the Windows API convert "/" to "\" as part of
// > converting the name to an NT-style name, except when using the "\\?\"
// > prefix as detailed in the following sections.
// from https://docs.microsoft.com/en-us/windows/desktop/FileIO/naming-a-file#maximum-path-length-limitation
// Because we want the larger maximum path length for absolute paths, we
// convert forward slashes to backward slashes here.
for (result[0..end_index]) |*elem| {
if (elem.* == '/') {
elem.* = '\\';
}
}
mem.copy(u16, result[end_index..], suffix);
result[end_index + suffix.len] = 0;
return result;
Expand Down
17 changes: 12 additions & 5 deletions src-self-hosted/stage2.zig
Original file line number Diff line number Diff line change
Expand Up @@ -318,11 +318,18 @@ const FmtError = error{
} || fs.File.OpenError;

fn fmtPath(fmt: *Fmt, file_path: []const u8, check_mode: bool) FmtError!void {
if (fmt.seen.exists(file_path)) return;
try fmt.seen.put(file_path);
// get the real path here to avoid Windows failing on relative file paths with . or .. in them
var real_path = fs.realpathAlloc(fmt.allocator, file_path) catch |err| {
try stderr.print("unable to open '{}': {}\n", .{ file_path, err });
fmt.any_error = true;
return;
};
defer fmt.allocator.free(real_path);

if (fmt.seen.exists(real_path)) return;
try fmt.seen.put(real_path);

const max = std.math.maxInt(usize);
const source_code = fs.cwd().readFileAlloc(fmt.allocator, file_path, max) catch |err| switch (err) {
const source_code = fs.cwd().readFileAlloc(fmt.allocator, real_path, self_hosted_main.max_src_size) catch |err| switch (err) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't real_path an absolute path? Why call fs.cwd()?

error.IsDir, error.AccessDenied => {
// TODO make event based (and dir.next())
var dir = try fs.cwd().openDir(file_path, .{ .iterate = true });
Expand Down Expand Up @@ -370,7 +377,7 @@ fn fmtPath(fmt: *Fmt, file_path: []const u8, check_mode: bool) FmtError!void {
fmt.any_error = true;
}
} else {
const baf = try io.BufferedAtomicFile.create(fmt.allocator, file_path);
const baf = try io.BufferedAtomicFile.create(fmt.allocator, fs.cwd(), real_path, .{});
defer baf.destroy();

const anything_changed = try std.zig.render(fmt.allocator, baf.stream(), tree);
Expand Down