This is the ideal FileType on Windows. You may not like it, but this is what peak performance looks like.

[retep998]

Theoretically this would fix #46484

The current iteration of this PR should not cause existing code to break, but instead merely improves handling around reparse points. Specifically...

• Reparse points are considered to be symbolic links if they have the name surrogate bit set. Name surrogates are reparse points that effectively act like symbolic links, redirecting you to a different directory/file. By checking for this bit instead of specific tags, we become much more general in our handling of reparse points, including those added by third parties.
• If something is a reparse point but does not have the name surrogate bit set, then we ignore the fact that it is a reparse point because it is actually a file or directory directly there, despite having additional handling by drivers due to the reparse point.
• For everything which is not a symbolic link (including non-surrogate reparse points) we report whether it is a directory or a file based on the presence of the directory attribute bit.
• Notably this still preserves invariant that when is_symlink returns true, both is_dir and is_file will return false. The potential for breakage was far too high.
• Adds an unstable FileTypeExt to allow users to determine whether a symbolic link is a directory or a file, since FileType by design is incapable of reporting this information.

[rust-highfive]

r? @Mark-Simulacrum

(rust_highfive has picked a reviewer for you, use r? to override)

[retep998]

cc @roblourens Could you test this to determine whether this actually fixes it?

[Mark-Simulacrum]

struct

[Mark-Simulacrum]

r? @BurntSushi (some random libs team member)

[retep998]

Closed by accident because of github silliness.

[BurntSushi]

cc @rust-lang/libs

This PR fixes a pretty bad bug that impacts any program on Windows that interacts with the file system and OneDrive directories. This was originally reported to me via the VS code folks on ripgrep: BurntSushi/ripgrep#705 --- Basically, the issue here is that OneDrive directories are reported as reparse points, and std currently does not interpret those as directories (or files) even when instructed to follow symbolic links. You can see an example in @roblourens's bug report here: #46484

I managed to work around this bug, but the work-around is gnarly. You basically need to replace all is_file/is_dir calls with a custom Windows specific wrapper. You can see the fix in walkdir here and the fix in ripgrep here.

With all that said, I am by no means a Windows person, so I don't actually know whether this PR is correct. But, it is consistent with my own fix to the problem (which was helpfully informed by @retep998 and @roblourens).

[BurntSushi]

I'm running short on time, but I'm now doubting this solution. I think @retep998 mentioned this to me on IRC, but the problem here is that this change will cause any entry that is a directory to return true for is_dir regardless of whether it is a symbolic link or not. This kind of breaks the various contracts we have setup in std::fs where we often distinguish between following symbolic links (e.g., fs::metadata) and not following symbolic links (e.g., fs::symlink_metadata).

This logic change, for example, ended up requiring changes like this in walkdir where I had assumed that is_dir being true meant that is_symlink would be false: BurntSushi/walkdir@2863f28

[retep998]

The problem is a fundamental difference in how Windows and Unix handle these sorts of attributes. On Windows whether a given path is a file or a directory is a completely separate question from whether it is a reparse point, whereas on Unix files directories and symbolic links are all mutually exclusive. Normally this wouldn't matter so much because you'd typically only have to deal with something being a reparse point or symbolic link when you specifically ask for symlink_metadata instead of metadata, but as it turns out sometimes metadata itself will be told by the OS that something is a reparse point. This leads to a conundrum where the programmer is unable to determine whether the path is a directory or a file because is_dir ignores that when it is a reparse point. However to fix this would require FileType to report that a path is a file or directory even when it is also a reparse point which violates the current assumptions of users of the API in that being a file or directory is mutually exclusive with being a symbolic link/reparse point.

[pitdicker]

I like how matching the attributes is folded into the functions here.

But I don't agree this is the best solution, as it pretty heavily changes the existing behavior. Yes, it matches better with the Windows model of things, but makes it harder to write something cross-platform.

What do you think about only changing the final match condition, so that it reports a reparse point as a directory or file if the reparse point is not recognized as a symlink or directory junction? I think that would also solve the bug with OneDrive, and not be a breaking change.

[BurntSushi]

@pitdicker Interesting. I also believe that would solve the OneDrive bug.

[retep998]

What if there is a reparse point which does act like a symbolic link, but is not recognized as a symlink or directory junction? What would happen if someone called remove_dir_all on that? It would see the reparse point as just a directory and not a symlink, causing it to recursively delete the contents instead of just deleting the reparse point itself, which would be absolutely horrible!

I think the best we could do is change how we construct FileType based on whether we are getting metadata or link_metadata. For the former we would ignore if something is a reparse point and just report whether it is a directory or a file, but for the latter we would report all reparse points as symlinks.

[BurntSushi]

@retep998 So I still feel like we're missing something here. If a reparse point is considered a symlink, and OneDrive directories are reparse points, then that means we're treating OneDrive directories as a symlinks. But that doesn't seem right to me? They aren't necessarily symlinked to something else on the file system. For example, if I were removing a OneDrive directory, I would expect it to recursively descend and remove it where as I wouldn't expect that behavior with symlinks (unless the routine was specifically instructed to follow symlinks).

What else are reparse points used for on Windows?

[mattico]

(This is the first time I've read about this but I believe this summary is correct)

@BurntSushi
When NTFS attempts to open a file or directory with a reparse point it looks up a driver based on its reparse tag, and that driver handles the request. These drivers can do anything and can be registered by user programs, so a reparse point doesn't have any defined semantics per se.

The good news is that the surrogate bit in the reparse tag specifies if "the file or directory represents another named entity in the system". That sounds like symlink behavior to me!

I made a program to test this behavior. Here are the results on my computer with Files On-Demand enabled:

PATH	REPARSE TAG	IsReparseTagNameSurrogate
D:\symlink	10100000000000000000000000001100	true
D:\symlink_dir	10100000000000000000000000001100	true
D:\junction	10100000000000000000000000000011	true
D:\mountpoint	10100000000000000000000000000011	true
D:\OneDrive	10010000000000000111000000011010	false
D:\hardlink	hard links do not use reparse points	and they don't act like symlinks

So the flag works as I'd expect. Therefore, I'd argue the correct way to implement is_symlink is using this flag:

fn IsReparseTagNameSurrogate(reparse_tag: ULONG) -> bool {
    (reparse_tag & 0x20000000) == 0x20000000
}
pub fn is_symlink(&self) -> bool {
    self.is_reparse_point() && IsReparseTagNameSurrogate(self.reparse_tag)
}

Edit:
Funny thing is, none of the code I found while researching does this correctly. Even Microsoft did it wrong and has an issue open to fix it.

[BurntSushi]

@mattico Nice find! If that is indeed the case then perhaps that is the secret sauce we need to handle reparse points. @roblourens @retep998 What do you think?

[mattico]

@roblourens I disagree that mount points need to be handled differently than symlinks.

Mount Points are basically identical to Junctions, except that mount points can point into volumes that aren't mounted with a drive letter. The documentation indicates that they were added to aid users who are using more volumes than can be assigned to 26 drive letters. I'd argue, therefore, that the purpose of Mount Points is not to be "a junction except that applications will pretend it is a regular directory", but to be "a junction, but I ran out of drive letters".

Second, I believe that IsReparseTagNameSurrogate is the way that Windows applications are supposed to determine what to do with reparse tags, in general. It's what del /s uses to determine if it should recurse into a reparse tag to delete, for example. Matching specific reparse tags opens up a can of worms where new reparse tags don't work as expected (that's the whole reason we're having this discussion!) For example: if Microsoft adds "mount point except it works for ReFs volumes", whoops need to update is_symlink again. If third-party software adds a mount-point-like reparse tag, that would also unexpectedly work differently than regular mount points.

(anyway, I'm not super concerned about this because NTFS mount points are rarely used so as long as it's documented... meh)

---

Also if we should be able to differentiate between a symlinked file and directory, seems like that be available cross platform, not just for windows.

Doesn't std::fs::FileType work for this? type.is_dir() && type.is_symlink(), or does lstat never set S_IFDIR when S_IFLNK is true? I notice that the current windows implementation doesn't work like this, but I'm a bit surprised about that.

Note that, despite being pub, is_symlink_dir is not exposed to users since they cannot access the sys modules. I'd agree that there should be a std::os::windows::fs::FileTypeExt, so users who care can tell the difference between a directory symbolic link and a junction or whatever.

[retep998]

Doesn't std::fs::FileType work for this? type.is_dir() && type.is_symlink(), or does lstat never set S_IFDIR when S_IFLNK is true?

According to posix, to get the file type, you can mask st_mode with S_IFMT and then compare that for equality with the file type constants. This is an inherently mutually exclusive operation therefore anything that uses posix style lstat is unable to tell whether a symlink is a file or a directory,

In addition, such posixy platforms have a single function for creating symlinks. There is no distinction between a directory symlink and a file symlink in the API (unlike Windows where CreateSymbolicLink can take SYMBOLIC_LINK_FLAG_DIRECTORY), nor does the programmer ever have to care about there being a distinction because deleting a symlink is done the same way regardless of what it points to (unlike Windows where you need to know the difference to know whether to call RemoveDirectory or DeleteFile).

[shepmaster]

Ping from triage, @BurntSushi !

[BurntSushi]

@rust-lang/libs I'd like to nominate this PR for merging, since I think it is in a good place.

Allow me to briefly summarize this PR. An issue was reported recently on Windows where is_dir returned false when it should have returned true. In particular, this was on a directory in a OneDrive mount on Windows. Windows records this as a reparse point, but FileType on Windows won't interpret it as a directory. The key here is that reparse points aren't necessarily symbolic links, but as @mattico points out, it is possible to tell whether a reparse point is a symlink or not.

So what does this PR do? It effectively rewrites FileType detection logic on Windows to be consistent with the aforementioned facts. In particular, it changes things like directory detection to correspond to, "has the directory bit set, and isn't a symlink," which seems correct to me. The specific behavior this changes is that is_dir will now return true in more cases than it previously did, but it should never return false where it previously returned true. While this is a change in behavior, I see this as an important bug fix. In particular, working around this problem with current Rust is particularly gruesome. You can see the requisite patches to both walkdir and ripgrep. If this PR is merged, then I believe I can drop both of those patches and everyone will be happy again.

The reason why this issue is coming up now is because it seems like there was a relatively recent change to cause OneDrive's "files on demand" feature to use reparse points. You can see that others are also grappling with a similar issue:

• Support OneDrive folders on recent Windows 10 syncthing/syncthing#4542
• Support OneDrive by properly handling symlinks gohugoio/hugo#4079
• os: IsDir doesn't work with Windows OneDrive golang/go#22579 (This is roughly analogous to our bug, but in Go land. There is a patch with a fix that is roughly equivalent to this PR, although this PR seems more correct since it uses the surrogate bits on a reparse point to determine whether it is a symbolic link or not. The Go patch instead tries to exhaustively enumerate all possible reparse points that are symbolic links.)
• windows: fix non-symlink reparse points libuv/libuv#1522 (This is a patch to libuv that proceeds in a slightly different manner. Namely, it seems to fix this in lstat directly by interpreting a path as a regular file when it's a reparse point only if reading it as a link fails. This seems like a bit of a higher level fix to me and probably not what we want for Rust's standard library. This did in turn fix VS Code itself though.)
• Julia issue with new OneDrive placeholder files in Windows Fall Creator Update JuliaLang/julia#24483 (It looks like this will get fixed once they update libuv?)
• OneDrive files not visible to Bash microsoft/WSL#1941 (Fix logged here? https://docs.microsoft.com/en-us/windows/wsl/release-notes#build-16226)

In light of the above I think we are on very solid ground when we say that the existing behavior is incorrect and should be fixed. Applications and libraries can work around it (like I have), but it is a monstrous pain. :-)

[alexcrichton]

Thanks for the extensive report @BurntSushi! Out of curiosity, do you have an example of situations that don't want this sort of logic? Are there examples programs which will break as a result of this change, for example relying on the fact that is_dir is returning false?

I'm inclined to merge given the fallout here, but would just like to understand the potential fallout here!

[BurntSushi]

@alexcrichton I actually can't think of any case (and haven't explicitly come across one), but I think someone more familiar with Windows should probably answer that. cc @roblourens @retep998 @mattico

[mattico]

(I'm going to equate "use of a non-surrogate reparse point" with "the OneDrive folder" for expediency)

I expect these functions are mostly used for recursive directory traversal. Users may have noticed that the OneDrive folder is special and isn't affected by these methods. I can imagine some cases where this behavior could be relied upon:

• A user wants {backup, disk usage accounting} software to ignore the OneDrive folder since it's already backed up or since the files may not be stored locally on the computer.
• A user notices that the OneDrive folder doesn't get deleted by their recursive delete function and starts to rely on this behavior. (fs::remove_dir_all currently stops with an error once it gets to the OneDrive folder, but I suppose an outside implementation may keep going)

The deletion scenario is slightly scary because it opens the door to data loss. I don't think it's an issue in practice because:

1. OneDrive files are "in the cloud" so you only lose files which haven't finished syncing (and some bandwidth/time).
2. The OneDrive folder is stored in the user's home directory (by default), so I doubt it would be caught up in a remove_dir_all unless you're deliberately or accidentally deleting all of a user's data.
3. Users expect the OneDrive folder to behave like a normal directory until it doesn't. If they're recursively deleting they must have been willing to delete the OneDrive folder, at least by accident.

[retep998]

I added an unstable FileTypeExt which allows users to finally determine whether a symbolic link is also a directory or a file. FileType will always return false for both is_dir and is_file when is_symlink is true, yet users need to know this critical piece of information. The fact that remove_dir_all needs this information should be reason enough to expose it.

This might need an FCP to be added, along with a tracking issue.

[BurntSushi]

@retep998 If there is any controversy around FileTypeExt (although it seems reasonable to me, and it is an unstable API), then it might be wise to split that out into a separate PR. :-)

[retep998]

A user wants {backup, disk usage accounting} software to ignore the OneDrive folder since it's already backed up or since the files may not be stored locally on the computer.

In this case I argue software should be paying attention to attributes like FILE_ATTRIBUTE_OFFLINE or FILE_ATTRIBUTE_RECALL_ON_DATA_ACCESS or FILE_ATTRIBUTE_RECALL_ON_OPEN.

[alexcrichton]

Ok thanks for the info @mattico! This seems fine to land by me

[BurntSushi]

@alexcrichton I think an unstable API was added to this since I last commented. Should we get tickboxes from the std team to land it? (I think the new unstable API is probably uncontroversial and is a Windows specific API.)

[alexcrichton]

@BurntSushi nah unstable APIs can land at any time, I think this may want to hold off on waiting for beta to branch (to help give it maximal time to bake on nightly), but other than that I think this can be r+'d whenever.

[BurntSushi]

@bors r+

[bors]

📌 Commit 9269e83 has been approved by BurntSushi

[BurntSushi]

Thanks again @retep998! 🎉

[bors]

⌛ Testing commit 9269e83 with merge b298607...

[bors]

☀️ Test successful - status-appveyor, status-travis
Approved by: BurntSushi
Pushing b298607 to master...

[dan-da]

Can this be marked stable? I need it when copying around windows directory trees, to be able to correctly re-create symlinks.

The alternative approach is to check the type of the link target, but that fails or gives wrong result if:

1. The link target does not exist.
2. The link target type is different from the link type.