Skip to content
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

Dir.makePath infinite loops when a path component is a symlink #17330

Open
Jarred-Sumner opened this issue Sep 30, 2023 · 3 comments
Open

Dir.makePath infinite loops when a path component is a symlink #17330

Jarred-Sumner opened this issue Sep 30, 2023 · 3 comments
Labels
bug Observed behavior contradicts documented or intended behavior standard library This issue involves writing Zig code for the standard library.
Milestone

Comments

@Jarred-Sumner
Copy link
Contributor

Zig Version

0.12.0-dev.163+6780a6bbf

Steps to Reproduce and Observed Behavior

  1. Paste into repro.zig
const std = @import("std");
test {
    try std.fs.cwd().symLink("../../boop", "folder", .{});
    try std.fs.cwd().makePath("folder/infinite-loop");
}
  1. zig test repro.zig
  2. Infinite loop

Expected Behavior

It shouldn't infinite loop

It is a valid question whether it should follow the symlink and "continue" creating the directory through the symlink, joining the path. There is an argument for either case.

But it shouldn't infinite loop

@Jarred-Sumner Jarred-Sumner added the bug Observed behavior contradicts documented or intended behavior label Sep 30, 2023
@Vexu Vexu added the standard library This issue involves writing Zig code for the standard library. label Sep 30, 2023
@Vexu Vexu added this to the 0.13.0 milestone Sep 30, 2023
@rootbeer
Copy link
Contributor

I reproduced this, but this comment in makePath predicts the problem: https://github.com/ziglang/zig/blob/937e8cb7051a3de537e11c2d52946f772f7449c3/lib/std/fs.zig#L1469C1-L1474C52

    while (true) {
        self.makeDir(component.path) catch |err| switch (err) {
            error.PathAlreadyExists => {
                // TODO 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

Note that valid symlinks don't have this problem, just dangling ones.

@squeek502
Copy link
Collaborator

squeek502 commented Sep 30, 2023

Should be fixed by #16878, but that PR is to-an-extent blocked by #17034

rootbeer added a commit to rootbeer/zig that referenced this issue Oct 12, 2023
If a directory in a makePath() call already exists, use statFile() to
check what it is.   If its a Directory, that's okay.  If its "not found",
that implies its a dangling symlink, other kinds of files are assumed
to be not okay.

Also add several more tests for makePath.

Fixes ziglang#17330
rootbeer added a commit to rootbeer/zig that referenced this issue Oct 12, 2023
If a directory in a makePath() call already exists, use statFile() to
check what it is.   If its a Directory, that's okay.  If its "not found",
that implies its a dangling symlink, other kinds of files are assumed
to be not okay.

Also add several more tests for makePath.

Fixes ziglang#17330
rootbeer added a commit to rootbeer/zig that referenced this issue Oct 12, 2023
If a directory in a makePath() call already exists, use statFile() to
check what it is.   If its a Directory, that's okay.  If its "not found",
that implies its a dangling symlink, other kinds of files are assumed
to be not okay.

Also add several more tests for makePath.

Fixes ziglang#17330
@squeek502
Copy link
Collaborator

squeek502 commented Jan 5, 2024

Info dump of what I've found when looking into this so far on Windows:


NtCreateFile without FILE_OPEN_REPARSE_POINT will successfully create the target of a dangling directory symlink

This modified test from #17499 succeeds on Windows (makePath will create both doesnotexist and doesnotexist/never-created), but we want it to fail since it's trying to go through a dangling symlink:

test "makepath existing dangling symlink" {
    var tmp = tmpDir(.{});
    defer tmp.cleanup();

    const symlinkName = "dangling-symlink";
    const symlinkTarget = "./doesnotexist";
    const pathThroughLink = "dangling-symlink/never-created";

    try tmp.dir.symLink(symlinkTarget, symlinkName, .{ .is_directory = true });
    try tmp.dir.makePath(pathThroughLink);
}

With FILE_OPEN_REPARSE_POINT set, I believe it will work as intended for the purposes of makePath (the NtCreateFile of dangling-symlink will return OBJECT_NAME_COLLISION which gets translated to error.PathAlreadyExists).


Dir.statFile on a dangling directory symlink will error with error.IsDir since FILE_NON_DIRECTORY_FILE is used when opening the file

This ties into #16738, since Dir.statFile always uses FILE_NON_DIRECTORY_FILE, meaning that it seems to be impossible to get anything but error.IsDir from a directory symlink, dangling or not. If neither FILE_DIRECTORY_FILE/FILE_NON_DIRECTORY_FILE is passed (i.e. std.os.windows.OpenFileOptions.filter = .any), then it returns OBJECT_NAME_NOT_FOUND for a dangling directory symlink as expected, though.

For avoiding the infinite loop in the OP, we need to be able to detect dangling symlinks, so the current implementation of statFile won't work to solve this on Windows.


Weird side-note about the mkdir util on Windows

The mkdir Windows implementation has some surprising behavior around this

mkdir creates intermediate directories by default. With the same setup as the test above:

C:\Users\Ryan\Programming\tmp> mklink /D dangling-symlink .\doesnotexist
symbolic link created for dangling-symlink <<===>> .\doesnotexist

and a test.bat with the following:

mkdir dangling-symlink\never-created

then executing the test.bat via NtTrace like so:

> nttrace test.bat > trace.log

Gives us the following expected NtCreateFile call:

NtCreateFile( FileHandle=0x95c56fdfd0, DesiredAccess=SYNCHRONIZE|0x1, ObjectAttributes=0x48:"dangling-symlink\never-created", IoStatusBlock=0x95c56fdf40, AllocationSize=null, FileAttributes=0x80, ShareAccess=3, CreateDisposition=2, CreateOptions=0x00204021, EaBuffer=null, EaLength=0 ) => 0xc000003a [3 'The system cannot find the path specified.']

(it has FILE_OPEN_REPARSE_POINT set [0x200000] and it returns OBJECT_PATH_NOT_FOUND)

but also these unexpected NtCreateFile calls afterwards:

NtCreateFile( FileHandle=0x95c56fdfd0, DesiredAccess=SYNCHRONIZE|0x1, ObjectAttributes="\??\C:\Users", IoStatusBlock=0x95c56fdf40, AllocationSize=null, FileAttributes=0x80, ShareAccess=3, CreateDisposition=2, CreateOptions=0x00204021, EaBuffer=null, EaLength=0 ) => 0xc0000035 [183 'Cannot create a file when that file already exists.']
NtCreateFile( FileHandle=0x95c56fdfd0, DesiredAccess=SYNCHRONIZE|0x1, ObjectAttributes="\??\C:\Users\Ryan", IoStatusBlock=0x95c56fdf40, AllocationSize=null, FileAttributes=0x80, ShareAccess=3, CreateDisposition=2, CreateOptions=0x00204021, EaBuffer=null, EaLength=0 ) => 0xc0000035 [183 'Cannot create a file when that file already exists.']
NtCreateFile( FileHandle=0x95c56fdfd0, DesiredAccess=SYNCHRONIZE|0x1, ObjectAttributes="\??\C:\Users\Ryan\Programming", IoStatusBlock=0x95c56fdf40, AllocationSize=null, FileAttributes=0x80, ShareAccess=3, CreateDisposition=2, CreateOptions=0x00204021, EaBuffer=null, EaLength=0 ) => 0xc0000035 [183 'Cannot create a file when that file already exists.']
NtCreateFile( FileHandle=0x95c56fdfd0, DesiredAccess=SYNCHRONIZE|0x1, ObjectAttributes="\??\C:\Users\Ryan\Programming\tmp", IoStatusBlock=0x95c56fdf40, AllocationSize=null, FileAttributes=0x80, ShareAccess=3, CreateDisposition=2, CreateOptions=0x00204021, EaBuffer=null, EaLength=0 ) => 0xc0000035 [183 'Cannot create a file when that file already exists.']
NtCreateFile( FileHandle=0x95c56fdfd0, DesiredAccess=SYNCHRONIZE|0x1, ObjectAttributes="\??\C:\Users\Ryan\Programming\tmp\dangling-symlink", IoStatusBlock=0x95c56fdf40, AllocationSize=null, FileAttributes=0x80, ShareAccess=3, CreateDisposition=2, CreateOptions=0x00204021, EaBuffer=null, EaLength=0 ) => 0xc0000035 [183 'Cannot create a file when that file already exists.']
NtCreateFile( FileHandle=0x95c56fdfd0, DesiredAccess=SYNCHRONIZE|0x1, ObjectAttributes="\??\C:\Users\Ryan\Programming\tmp\dangling-symlink\never-created", IoStatusBlock=0x95c56fdf40, AllocationSize=null, FileAttributes=0x80, ShareAccess=3, CreateDisposition=2, CreateOptions=0x00204021, EaBuffer=null, EaLength=0 ) => 0xc000003a [3 'The system cannot find the path specified.']

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Observed behavior contradicts documented or intended behavior standard library This issue involves writing Zig code for the standard library.
Projects
None yet
Development

No branches or pull requests

4 participants