Enable recursion into OneDrive #9509
Conversation
src/System.Management.Automation/namespaces/FileSystemProvider.cs
Outdated
Show resolved
Hide resolved
src/System.Management.Automation/namespaces/FileSystemProvider.cs
Outdated
Show resolved
Hide resolved
iSazonov
added
the
CL-General
src/System.Management.Automation/namespaces/FileSystemProvider.cs
Outdated
Show resolved
Hide resolved
|
Impressive fix... relying on very undocumented stuff if you ask me... unless "named surrogates" and "data.dwReserved0 & 0x20000000" is documented somewhere. @SteveL-MSFT?
|
|
@sba923 correct, next version is 7.0-preview.1. As noted in the comments in the change, the use of |
|
@SteveL-MSFT Thanks for the confirmation about 7.0. I had missed the comment with the reference to the The documentation for WIN32_FIND_DATAA documents that symlinks use a reparse point tag of |
| { | ||
| #if !UNIX | ||
| var data = new WIN32_FIND_DATA(); | ||
| using (SafeFileHandle handle = FindFirstFileEx(filePath, FINDEX_INFO_LEVELS.FindExInfoBasic, ref data, FINDEX_SEARCH_OPS.FindExSearchNameMatch, IntPtr.Zero, 0)) |
There was a problem hiding this comment.
Perhaps we could use method with fewer parameters
https://github.com/dotnet/corefx/blob/87089ef4559605f8a34ca8959a0ef9c71aa1e02e/src/System.IO.FileSystem/src/System/IO/FileSystem.Windows.cs#L241
I do very wonder that CoreFX uses IsNameSurrogateReparsePoint() method only for recurse directory remove but not for recurse directory read.
There was a problem hiding this comment.
CoreFx uses FindFirstFileEx() as well, they just name their wrapper FindFirstFile()
There was a problem hiding this comment.
On the second point, I agree it might be an oversight that they don't have symmetric behavior.
|
I've collected my findings about the reparse tags used by OneDrive in this spreadsheet. https://docs.microsoft.com/en-us/windows/desktop/w8cookbook/placeholder-files is definitely outdated, as Windows 10 OneDrive doesn't use I'm not sure where to post the request to document all this. @SteveL-MSFT: can you help? |
|
For now I've opened MicrosoftDocs/feedback#1484 and MicrosoftDocs/feedback#1483 |
| IntPtr hTemplateFile); | ||
|
|
||
| [DllImport(PinvokeDllNames.FindFirstFileDllName, EntryPoint = "FindFirstFileExW", SetLastError = true, CharSet = CharSet.Unicode)] | ||
| private static extern SafeFileHandle FindFirstFileEx(string lpFileName, FINDEX_INFO_LEVELS fInfoLevelId, ref WIN32_FIND_DATA lpFindFileData, FINDEX_SEARCH_OPS fSearchOp, IntPtr lpSearchFilter, int dwAdditionalFlags); |
There was a problem hiding this comment.
This returns a SafeFileHandle, which eventually calls CloseHandle.
According to the FindFirstFileExW documentation, the handle should be closed with FindClose, not CloseHandle.
There was a problem hiding this comment.
Nice catch! Will submit a new PR to address.
5 participants
PR Summary
The current logic of
Get-ChildItem -Recursefor the FileSystemProvider is that if it's a symlink, we don't recurse into it unless-FollowSymLinkis specified. However, OneDrive reparse points aren't symlinks (in that they don't link to the local filesystem). There is another flag (on Windows) to indicate if the reparse point is a named surrogate which means it IS a symlink. So the fix is to check for this flag and if it's not a named surrogate (aka symlink), it is ok to recurse into it.Manually verified with OneDrive.
PR Context
Fix #9461
PR Checklist
.h,.cpp,.cs,.ps1and.psm1files have the correct copyright headerWIP:or[ WIP ]to the beginning of the title (theWIPbot will keep its status check atPendingwhile the prefix is present) and remove the prefix when the PR is ready.