Skip to content

File.stat: Support detection of Kind.sym_link on Windows - #18326

Merged
Vexu merged 2 commits into
ziglang:masterfrom
squeek502:stat-symlink
Dec 22, 2023
Merged

File.stat: Support detection of Kind.sym_link on Windows#18326
Vexu merged 2 commits into
ziglang:masterfrom
squeek502:stat-symlink

Conversation

@squeek502

@squeek502squeek502 commented Dec 20, 2023

Copy link
Copy Markdown
Member

Requires an extra NtQueryInformationFile call when FILE_ATTRIBUTE_REPARSE_POINT is set to determine if it's actually a symlink or some other kind of reparse point (https://learn.microsoft.com/en-us/windows/win32/fileio/reparse-point-tags). This is something that File.Metadata.kind was already doing, so the same technique is used in stat.

Also, replace the std.os.windows.DeviceIoControl call in metadata with NtQueryInformationFile (NtQueryInformationFile is what gets called during kernel32.GetFileInformationByHandleEx with FileAttributeTagInfo, verified using NtTrace).

Addresses the last bit of #18290 that #18323 doesn't.

@daurnimatordaurnimator left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Isn't this sort of like AT_SYMLINK_NOFOLLOW on unices? Should we introduce that flag as an argument across all OSes?

@squeek502

squeek502 commented Dec 20, 2023

Copy link
Copy Markdown
MemberAuthor

Isn't this sort of like AT_SYMLINK_NOFOLLOW on unices? Should we introduce that flag as an argument across all OSes?

This is in File.stat, meaning you already have an open file descriptor. AT_SYMLINK_NOFOLLOW is not a possible flag to fstat AFAICT.

Note, though, that the inability to specify not to follow symlinks when calling Dir.statFile/Dir.openFile is a separate issue alluded to in the added test in this PR. EDIT: #18327

@squeek502squeek502 changed the title File.stat: Support detection of Kind.sym_link on WindowsFile.stat: Support detection of Kind.sym_link on WindowsDec 20, 2023
Requires an extra NtQueryInformationFile call when FILE_ATTRIBUTE_REPARSE_POINT is set to determine if it's actually a symlink or some other kind of reparse point (https://learn.microsoft.com/en-us/windows/win32/fileio/reparse-point-tags). This is something that `File.Metadata.kind` was already doing, so the same technique is used in `stat`.
Also, replace the std.os.windows.DeviceIoControl call in `metadata` with NtQueryInformationFile (NtQueryInformationFile is what gets called during kernel32.GetFileInformationByHandleEx with FileAttributeTagInfo, verified using NtTrace).
@squeek502

squeek502 commented Dec 20, 2023

Copy link
Copy Markdown
MemberAuthor

Looks like the added test uncovered a OpenDirOptions.no_follow bug on MacOS (openat is returning ENOTDIR).

EDIT: O_NOFOLLOW doesn't seem like the right flag in general to be using for no_follow as described in OpenDirOptions:

zig/lib/std/fs/Dir.zig

Lines 1336 to 1337 in f36ac22

/// `true` means it won't dereference the symlinks.
no_follow: bool=false,

https://linux.die.net/man/2/open

O_NOFOLLOW
If pathname is a symbolic link, then the open fails.

Seems likely that Linux will fail, too.

EDIT#2: Seems like O_PATH in combo with O_NOFOLLOW is needed:

EDIT#3: openDirZ does use O_PATH, but it's defined as 0 in darwin.zig:

zig/lib/std/fs/Dir.zig

Lines 1410 to 1413 in f36ac22

constsymlink_flags: u32=if (args.no_follow) posix.O.NOFOLLOWelse0x0;
if (!args.iterate) {
constO_PATH=if (@hasDecl(posix.O, "PATH")) posix.O.PATHelse0;
returnself.openDirFlagsZ(sub_path_c, posix.O.DIRECTORY|posix.O.RDONLY|posix.O.CLOEXEC|O_PATH|symlink_flags);

zig/lib/std/c/darwin.zig

Lines 1369 to 1370 in f36ac22

pubconstO=struct {
pubconstPATH=0x0000;

EDIT#4: If the intention of OpenDirOptions.no_follow is to get a file descriptor of the symlink itself, it seems like that cannot be implemented on MacOS/Darwin/OpenBSD, as there is no O_PATH flag for the open syscall on those platforms.

@perillo

Copy link
Copy Markdown
Contributor

@squeek502 Thanks for the PR.

Just a few question:

  1. From the Windows documentation, there are additionally parse points can be used to emulate POSIX .
    As an example https://en.wikipedia.org/wiki/NTFS_reparse_point#Unix_domain_socket_(socket). Does it make sense to implement it?

  2. In FileMetadata, as you wrote in Fix Stat.ctime docs, and correct its value on Windows #18323, the implementation is correct. However, what is the reason why there is no change time?

@squeek502

squeek502 commented Dec 21, 2023

Copy link
Copy Markdown
MemberAuthor
  1. From the Windows documentation, there are additionally parse points can be used to emulate POSIX .
    As an example https://en.wikipedia.org/wiki/NTFS_reparse_point#Unix_domain_socket_(socket). Does it make sense to implement it?

If there's an example of how to go about getting a HANDLE to such a reparse point, that'd be helpful. Or if there's an example from another standard library that handles this case (Rust doesn't, for example; it only knows about file/directory/symlink).

  1. In FileMetadata, as you wrote in Fix Stat.ctime docs, and correct its value on Windows #18323, the implementation is correct. However, what is the reason why there is no change time?

Likely just an oversight, the context for the Metadata implementation is here: #10486

…ow behavior
The `no_follow` behavior happened to allow opening a file descriptor of a symlink itself on Windows, but that behavior may change in the future. Instead, we implement the opening of the symlink as a file descriptor manually (and per-platform) in the test case.
@Vexu
Vexu merged commit d787b78 into ziglang:masterDec 22, 2023
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@squeek502@perillo@daurnimator@Vexu