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

Preview 7 Improper LinkTarget returned for a Directory Junction on Windows #57575

Closed
fitdev opened this issue Aug 17, 2021 · 10 comments · Fixed by #57996
Closed

Preview 7 Improper LinkTarget returned for a Directory Junction on Windows #57575

fitdev opened this issue Aug 17, 2021 · 10 comments · Fixed by #57996
Assignees
Milestone

Comments

@fitdev
Copy link

fitdev commented Aug 17, 2021

When a directory is a junction (as opposed to being a regular directory or a symlink) on Windows, as of Preview 7, DirectoryInfo.LinkTarget returns a non-null value, which is wrong. Considering a structure like this:

C:\Test\RealFolder
C:\Test\JunctionToRealFolder -> C:\Test\RealFolder
C:\Test\SymlinkToRealFolder -> C:\Test\RealFolder

DirectoryInfo.LinkTarget for C:\Test\JunctionToRealFolder returns \Test\RealFolder (instead of C:\Test\RealFolder)
DirectoryInfo.LinkTarget for C:\Test\SymlinkToRealFolder returns C:\Test\RealFolder (which is correct)

Strangely, DirectoryInfo.ResolveLinkTarget(returnFinalTarget: true) returns the correct target in all cases (Junction and Symlink).

I suspect the issue comes that you have an incorrect check for a reparse tag kind in your code:

if ((rdb.ReparseTag & Interop.Kernel32.IOReparseOptions.IO_REPARSE_TAG_SYMLINK) == 0)

Should instead be:

if (rdb.ReparseTag != Interop.Kernel32.IOReparseOptions.IO_REPARSE_TAG_SYMLINK)

Since you say you support symlinks only ATM.

@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.IO untriaged New issue has not been triaged by the area owner labels Aug 17, 2021
@ghost
Copy link

ghost commented Aug 17, 2021

Tagging subscribers to this area: @dotnet/area-system-io
See info in area-owners.md if you want to be subscribed.

Issue Details

Main Issue

When a directory is a junction (as opposed to being a regular directory or a symlink) on Windows, as of Preview 7, DirectoryInfo.LinkTarget returns a non-null value, which is wrong. Considering a structure like this:

C:\Test\RealFolder
C:\Test\JunctionToRealFolder -> C:\Test\RealFolder
C:\Test\SymlinkToRealFolder -> C:\Test\RealFolder

DirectoryInfo.LinkTarget for C:\Test\JunctionToRealFolder returns \Test\RealFolder (instead of C:\Test\RealFolder)
DirectoryInfo.LinkTarget for C:\Test\SymlinkToRealFolder returns C:\Test\RealFolder (which is correct)

Strangely, DirectoryInfo.ResolveLinkTarget(returnFinalTarget: true) returns the correct target in all cases (Junction and Symlink).

I suspect the issue comes that you have an incorrect check for a reparse tag kind in your code:

if ((rdb.ReparseTag & Interop.Kernel32.IOReparseOptions.IO_REPARSE_TAG_SYMLINK) == 0)

Should instead be:

if (rdb.ReparseTag != Interop.Kernel32.IOReparseOptions.IO_REPARSE_TAG_SYMLINK)

Since you say you support symlinks only ATM.

Smaller issue

On Windows, there is an inconsistency of final targets returned depending on which APIs are used, and whether or not virtual drives created with subst command are used in the path (both the link and the target residing on such virtual drives):

The DirectoryInfo.ResolveLinkTarget(returnFinalTarget: true) API (and other APIs you added that resolve final target) resolve the final target to non-virtual drives, while the DirectoryInfo.LinkTarget (and other similar APIs that resolve the immediate target), when called repeatedly while LinkTarget != null stop at the virtual drive level.

So, given a structure like this:

C:\RootDirectoryForVirtualDriveX
X:\SomeRealFolder
X:\SymlinkToRealFolderOnX -> X:\SomeRealFolder

Calling DirectoryInfo.ResolveLinkTarget(returnFinalTarget: true) on X:\SymlinkToRealFolderOnX will result in: C:\RootDirectoryForVirtualDriveX\SomeRealFolder

While calling DirectoryInfo.LinkTarget (repeatedly while the result is non-null) on X:\SymlinkToRealFolderOnX will result in: X:\SomeRealFolder

Even though, logically, and for consistency results should be the same.

Author: fitdev
Assignees: -
Labels:

area-System.IO, untriaged

Milestone: -

@adamsitnik
Copy link
Member

@carlossanlop @jozkee PTAL

@adamsitnik adamsitnik added this to the 6.0.0 milestone Aug 17, 2021
@carlossanlop
Copy link
Member

Thanks for your report, @fitdev

The "main issue" should get moved to 7.0. As you correctly stated, we only support symlinks at the moment.

I'm moving the "smaller issue" into a separate issue, and we can then determine if it should be fixed for 6.0 or 7.0.

@carlossanlop carlossanlop changed the title Preview 7 Improper LinkTarget returned for a Directory Junction on Windows and Inconsistent FinalTarget results Preview 7 Improper LinkTarget returned for a Directory Junction on Windows Aug 17, 2021
@carlossanlop carlossanlop modified the milestones: 6.0.0, 7.0.0 Aug 17, 2021
@carlossanlop carlossanlop removed the untriaged New issue has not been triaged by the area owner label Aug 17, 2021
@fitdev
Copy link
Author

fitdev commented Aug 17, 2021

@carlossanlop Thanks for looking into this. However this issue was not about supporting Junctions. It was about a bug in Preview 7 and your current code that basically returns a target (i.e. non-null string) even if the directory is a Junction. Instead it should return null then, and certainly NOT the totally erroneous path as it does currently.

I mean if DotNet 6 does not support Junctions, then it should not support them at all, i.e. LinkTarget and ResolveLinkTarget should all return null if directory is a junction, i.e. treat it as a regular directory. But right now it seems to be semi-supported in a very buggy way (no way to distinguish between is this a symlink or a junction dir, since in both cases LinkTarget is not null).

@carlossanlop
Copy link
Member

Good point, @fitdev. I'll mark it for 6.0 and return null for junctions.

@carlossanlop carlossanlop modified the milestones: 7.0.0, 6.0.0 Aug 17, 2021
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Aug 24, 2021
@carlossanlop
Copy link
Member

@fitdev I had a conversation with @jozkee and there's no need to return null in LinkTarget if the link is a junction. The fix ensures both public APIs ResolveLinkTarget(bool) and LinkTarget can resolve both symlinks and junctions, because internally, they both call the same method: GetImmediateLinkTarget.
Even if currently we don't offer a public API to determine what kind of link the file represents, the user wants to be able to succesfully resolve the target. This scenario is much more important.

@fitdev
Copy link
Author

fitdev commented Aug 24, 2021

If it would work correctly, I suppose. But currently it does not work correctly, see my original issue comment:

DirectoryInfo.LinkTarget for C:\Test\JunctionToRealFolder returns \Test\RealFolder (instead of C:\Test\RealFolder)

Better to just return null until Junctions are properly supported instead of an improper path like this.

@carlossanlop
Copy link
Member

Noted. With the bug fix, the LinkTarget API should now return the correct path for your example: C:\Test\RealFolder.

@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Aug 27, 2021
@fitdev
Copy link
Author

fitdev commented Aug 28, 2021

@carlossanlop I noticed you merged your changes into main yesterday. However, I see that you still use the old check to determine reparse tag kind:

((data.dwReserved0 & Interop.Kernel32.IOReparseOptions.IO_REPARSE_TAG_SYMLINK) == 0 &&
(data.dwReserved0 & Interop.Kernel32.IOReparseOptions.IO_REPARSE_TAG_MOUNT_POINT) == 0))

Shouldn't these be:

 ((data.dwReserved0 != Interop.Kernel32.IOReparseOptions.IO_REPARSE_TAG_SYMLINK) && 
  (data.dwReserved0 != Interop.Kernel32.IOReparseOptions.IO_REPARSE_TAG_MOUNT_POINT))) 

I mean otherwise, supposing reparse tag was say 0x80000021 (IO_REPARSE_TAG_ONEDRIVE), then 0x80000021 & 0xA000000C would be 0x80000000, which is not 0! Therefore routine will not return null (as it should have, since reparse tag is neither IO_REPARSE_TAG_SYMLINK or IO_REPARSE_TAG_MOUNT_POINT).

@jozkee
Copy link
Member

jozkee commented Sep 9, 2021

I mean otherwise, supposing reparse tag was say 0x80000021 (IO_REPARSE_TAG_ONEDRIVE), then 0x80000021 & 0xA000000C would be 0x80000000, which is not 0! Therefore routine will not return null (as it should have, since reparse tag is neither IO_REPARSE_TAG_SYMLINK or IO_REPARSE_TAG_MOUNT_POINT).

The fix for that will be in .NET 6-RC2 release.
Thanks again, @fitdev.

@ghost ghost locked as resolved and limited conversation to collaborators Oct 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants