commit d72983da44b7ebe967868ba5236a2c4b05e84510 (tree)
parent 1264469a41e89e72635c3b60b4cd3dca17876964
Author: Ryan Liptak <squeek502@hotmail.com>
Date: Tue, 16 Dec 2025 20:02:05 -0800
File.OpenFlags: Add `allow_directory` and default it to true
This is one way of addressing/closing https://github.com/ziglang/zig/issues/16738
Previously, there was a mismatch between the default behaviors on Windows vs other platforms, where Windows was implicitly using .NON_DIRECTORY_FILE for its `openFile` implementation which caused `error.IsDir` when opening a directory, while on other platforms there is no equivalent flag for the `open` syscall. This meant that `openFile` on a path of a directory would fail on Windows but succeed on other platforms.
Adding `allow_directory` to `File.OpenFlags` serves two purposes:
1. It provides a cross-platform way to get the `.NON_DIRECTORY_FILE` behavior in the most efficient available way for the platform (on Windows, no extra syscalls are required, on other systems, an extra `fstat` is required)
2. It allows `statFile` to be implemented on top of `openFile` on Windows while still allowing `statFile` to work on directory paths. Before this commit, `statFile` on a directory path on Windows failed with `error.IsDir`
Note: The second purpose could have been addressed in different ways (bespoke call to NtCreateFile in the `statFile` implementation to avoid passing `NON_DIRECTORY_FILE`, or just never pass `NON_DIRECTORY_FILE` in the `openFile` implementation), so the first purpose is the more relevant/motivating force behind this change.
The default being `true` is intended to cut down on the number of syscalls as much as possible when using the default flags.
Diffstat:
4 files changed, 81 insertions(+), 38 deletions(-)
diff --git a/lib/std/Io/File.zig b/lib/std/Io/File.zig
@@ -100,6 +100,18 @@ pub const Lock = enum {
pub const OpenFlags = struct {
mode: OpenMode = .read_only,
+ /// Determines the behavior when opening a path that refers to a directory.
+ /// If set to true, directories may be opened, but `error.IsDir` is still
+ /// possible in certain scenarios, e.g. attempting to open a directory with
+ /// write permissions.
+ /// If set to false, `error.IsDir` will always be returned when opening a directory.
+ ///
+ /// When set to false:
+ /// * On Windows, the behavior is implemented without any extra syscalls.
+ /// * On other operating systems, the behavior is implemented with an additional
+ /// `fstat` syscall.
+ allow_directory: bool = true,
+
/// Open the file with an advisory lock to coordinate with other processes
/// accessing it at the same time. An exclusive lock will prevent other
/// processes from acquiring a lock. A shared lock will prevent other
@@ -226,7 +238,9 @@ pub const OpenError = error{
/// The file is too large to be opened. This error is unreachable
/// for 64-bit targets, as well as when opening directories.
FileTooBig,
- /// The path refers to directory but the `DIRECTORY` flag was not provided.
+ /// Either:
+ /// * The path refers to a directory and write permissions were requested.
+ /// * The path refers to a directory and `allow_directory` was set to false.
IsDir,
/// A new path cannot be created because the device has no room for the new file.
/// This error is only reachable when the `CREAT` flag is provided.
diff --git a/lib/std/Io/Threaded.zig b/lib/std/Io/Threaded.zig
@@ -1621,14 +1621,8 @@ fn dirMakePath(
// stat the file and return an error if it's not a directory
// this is important because otherwise a dangling symlink
// could cause an infinite loop
- check_dir: {
- // workaround for windows, see https://github.com/ziglang/zig/issues/16738
- const fstat = dirStatFile(t, dir, component.path, .{}) catch |stat_err| switch (stat_err) {
- error.IsDir => break :check_dir,
- else => |e| return e,
- };
- if (fstat.kind != .directory) return error.NotDir;
- }
+ const fstat = dirStatFile(t, dir, component.path, .{});
+ if (fstat.kind != .directory) return error.NotDir;
},
error.FileNotFound => |e| {
component = it.previous() orelse return e;
@@ -1750,16 +1744,10 @@ fn dirMakeOpenPathWindows(
// stat the file and return an error if it's not a directory
// this is important because otherwise a dangling symlink
// could cause an infinite loop
- check_dir: {
- // workaround for windows, see https://github.com/ziglang/zig/issues/16738
- const fstat = dirStatFileWindows(t, dir, component.path, .{
- .follow_symlinks = options.follow_symlinks,
- }) catch |stat_err| switch (stat_err) {
- error.IsDir => break :check_dir,
- else => |e| return e,
- };
- if (fstat.kind != .directory) return error.NotDir;
- }
+ const fstat = dirStatFileWindows(t, dir, component.path, .{
+ .follow_symlinks = options.follow_symlinks,
+ });
+ if (fstat.kind != .directory) return error.NotDir;
component = it.next().?;
continue;
@@ -2791,6 +2779,18 @@ fn dirOpenFilePosix(
};
errdefer posix.close(fd);
+ if (!flags.allow_directory) {
+ const is_dir = is_dir: {
+ const stat = fileStat(t, .{ .handle = fd }) catch |err| switch (err) {
+ // The directory-ness is either unknown or unknowable
+ error.Streaming => break :is_dir false,
+ else => |e| return e,
+ };
+ break :is_dir stat.kind == .directory;
+ };
+ if (is_dir) return error.IsDir;
+ }
+
if (have_flock and !have_flock_open_flags and flags.lock != .none) {
const lock_nonblocking: i32 = if (flags.lock_nonblocking) posix.LOCK.NB else 0;
const lock_flags = switch (flags.lock) {
@@ -2936,7 +2936,7 @@ pub fn dirOpenFileWtf16(
.OPEN,
.{
.IO = if (flags.follow_symlinks) .SYNCHRONOUS_NONALERT else .ASYNCHRONOUS,
- .NON_DIRECTORY_FILE = true,
+ .NON_DIRECTORY_FILE = !flags.allow_directory,
.OPEN_REPARSE_POINT = !flags.follow_symlinks,
},
null,
@@ -3052,9 +3052,8 @@ fn dirOpenFileWasi(
while (true) {
switch (wasi.path_open(dir.handle, lookup_flags, sub_path.ptr, sub_path.len, oflags, base, inheriting, fdflags, &fd)) {
.SUCCESS => {
- errdefer posix.close(fd);
current_thread.endSyscall();
- return .{ .handle = fd };
+ break;
},
.INTR => {
try current_thread.checkCancel();
@@ -3088,6 +3087,21 @@ fn dirOpenFileWasi(
},
}
}
+ errdefer posix.close(fd);
+
+ if (!flags.allow_directory) {
+ const is_dir = is_dir: {
+ const stat = fileStat(t, .{ .handle = fd }) catch |err| switch (err) {
+ // The directory-ness is either unknown or unknowable
+ error.Streaming => break :is_dir false,
+ else => |e| return e,
+ };
+ break :is_dir stat.kind == .directory;
+ };
+ if (is_dir) return error.IsDir;
+ }
+
+ return .{ .handle = fd };
}
const dirOpenDir = switch (native_os) {
@@ -4577,7 +4591,7 @@ fn dirSymLinkWindows(
.access_mask = w.SYNCHRONIZE | w.GENERIC_READ | w.GENERIC_WRITE,
.dir = dir,
.creation = w.FILE_CREATE,
- .filter = if (flags.is_directory) .dir_only else .file_only,
+ .filter = if (flags.is_directory) .dir_only else .non_directory_only,
}) catch |err| switch (err) {
error.IsDir => return error.PathAlreadyExists,
error.NotDir => return error.Unexpected,
diff --git a/lib/std/fs/test.zig b/lib/std/fs/test.zig
@@ -750,14 +750,26 @@ test "Dir.statFile" {
try testWithAllSupportedPathTypes(struct {
fn impl(ctx: *TestContext) !void {
const io = ctx.io;
- const test_file_name = try ctx.transformPath("test_file");
+ {
+ const test_file_name = try ctx.transformPath("test_file");
- try expectError(error.FileNotFound, ctx.dir.statFile(io, test_file_name, .{}));
+ try expectError(error.FileNotFound, ctx.dir.statFile(io, test_file_name, .{}));
- try ctx.dir.writeFile(io, .{ .sub_path = test_file_name, .data = "" });
+ try ctx.dir.writeFile(io, .{ .sub_path = test_file_name, .data = "" });
- const stat = try ctx.dir.statFile(io, test_file_name, .{});
- try expectEqual(File.Kind.file, stat.kind);
+ const stat = try ctx.dir.statFile(io, test_file_name, .{});
+ try expectEqual(.file, stat.kind);
+ }
+ {
+ const test_dir_name = try ctx.transformPath("test_dir");
+
+ try expectError(error.FileNotFound, ctx.dir.statFile(io, test_dir_name, .{}));
+
+ try ctx.dir.makeDir(io, test_dir_name);
+
+ const stat = try ctx.dir.statFile(io, test_dir_name, .{});
+ try expectEqual(.directory, stat.kind);
+ }
}
}.impl);
}
@@ -840,10 +852,15 @@ test "file operations on directories" {
handle.close(io);
} else {
// Note: The `.mode = .read_write` is necessary to ensure the error occurs on all platforms.
- // TODO: Add a read-only test as well, see https://github.com/ziglang/zig/issues/5732
try expectError(error.IsDir, ctx.dir.openFile(io, test_dir_name, .{ .mode = .read_write }));
}
+ {
+ const handle = try ctx.dir.openFile(io, test_dir_name, .{ .allow_directory = true, .mode = .read_only });
+ handle.close(io);
+ }
+ try expectError(error.IsDir, ctx.dir.openFile(io, test_dir_name, .{ .allow_directory = false, .mode = .read_only }));
+
if (ctx.path_type == .absolute and comptime PathType.absolute.isSupported(builtin.os)) {
try expectError(error.IsDir, fs.createFileAbsolute(test_dir_name, .{}));
try expectError(error.IsDir, fs.deleteFileAbsolute(test_dir_name));
diff --git a/lib/std/os/windows.zig b/lib/std/os/windows.zig
@@ -2310,17 +2310,15 @@ pub const OpenFileOptions = struct {
sa: ?*SECURITY_ATTRIBUTES = null,
share_access: FILE.SHARE = .VALID_FLAGS,
creation: FILE.CREATE_DISPOSITION,
- /// If true, tries to open path as a directory.
- /// Defaults to false.
- filter: Filter = .file_only,
+ filter: Filter = .non_directory_only,
/// If false, tries to open path as a reparse point without dereferencing it.
/// Defaults to true.
follow_symlinks: bool = true,
pub const Filter = enum {
/// Causes `OpenFile` to return `error.IsDir` if the opened handle would be a directory.
- file_only,
- /// Causes `OpenFile` to return `error.NotDir` if the opened handle would be a file.
+ non_directory_only,
+ /// Causes `OpenFile` to return `error.NotDir` if the opened handle is not a directory.
dir_only,
/// `OpenFile` does not discriminate between opening files and directories.
any,
@@ -2328,10 +2326,10 @@ pub const OpenFileOptions = struct {
};
pub fn OpenFile(sub_path_w: []const u16, options: OpenFileOptions) OpenError!HANDLE {
- if (mem.eql(u16, sub_path_w, &[_]u16{'.'}) and options.filter == .file_only) {
+ if (mem.eql(u16, sub_path_w, &[_]u16{'.'}) and options.filter == .non_directory_only) {
return error.IsDir;
}
- if (mem.eql(u16, sub_path_w, &[_]u16{ '.', '.' }) and options.filter == .file_only) {
+ if (mem.eql(u16, sub_path_w, &[_]u16{ '.', '.' }) and options.filter == .non_directory_only) {
return error.IsDir;
}
@@ -2366,7 +2364,7 @@ pub fn OpenFile(sub_path_w: []const u16, options: OpenFileOptions) OpenError!HAN
options.creation,
.{
.DIRECTORY_FILE = options.filter == .dir_only,
- .NON_DIRECTORY_FILE = options.filter == .file_only,
+ .NON_DIRECTORY_FILE = options.filter == .non_directory_only,
.IO = if (options.follow_symlinks) .SYNCHRONOUS_NONALERT else .ASYNCHRONOUS,
.OPEN_REPARSE_POINT = !options.follow_symlinks,
},
@@ -3040,7 +3038,7 @@ pub fn CreateSymbolicLink(
},
.dir = dir,
.creation = .CREATE,
- .filter = if (is_directory) .dir_only else .file_only,
+ .filter = if (is_directory) .dir_only else .non_directory_only,
}) catch |err| switch (err) {
error.IsDir => return error.PathAlreadyExists,
error.NotDir => return error.Unexpected,
