Skip to content

Enable recursion into OneDrive #9509

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.

Sign up for GitHub

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

Merged
adityapatwardhan merged 3 commits into from
May 7, 2019
Merged

Enable recursion into OneDrive #9509

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d014114
Enable recursion into OneDrive
SteveL-MSFT May 1, 2019
1df6183
address Ilya's feedback
SteveL-MSFT May 2, 2019
5032769
removed unused method
SteveL-MSFT May 2, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 57 additions & 1 deletion src/System.Management.Automation/namespaces/FileSystemProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1847,9 +1847,11 @@ private void Dir(
// a) the user has asked to with the -FollowSymLinks switch parameter and
// b) the directory pointed to by the symlink has not already been visited,
// preventing symlink loops.
// c) it is not a name surrogate making it not a symlink
if (tracker == null)
{
if (InternalSymbolicLinkLinkCodeMethods.IsReparsePoint(recursiveDirectory))
if (InternalSymbolicLinkLinkCodeMethods.IsReparsePoint(recursiveDirectory) &&
InternalSymbolicLinkLinkCodeMethods.IsNameSurrogateReparsePoint(recursiveDirectory.FullName))
{
continue;
}
Expand Down Expand Up @@ -7705,6 +7707,8 @@ public static class InternalSymbolicLinkLinkCodeMethods

private const string NonInterpretedPathPrefix = @"\??\";

private const int MAX_PATH = 260;

[Flags]
// dwDesiredAccess of CreateFile
internal enum FileDesiredAccess : uint
Expand Down Expand Up @@ -7845,6 +7849,39 @@ internal static extern IntPtr CreateFile(
FileAttributes dwFlagsAndAttributes,
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);
Copy link

@KalleOlaviNiemitalo KalleOlaviNiemitalo Aug 21, 2019

Choose a reason for hiding this comment

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

This returns a SafeFileHandle, which eventually calls CloseHandle.
According to the FindFirstFileExW documentation, the handle should be closed with FindClose, not CloseHandle.

Copy link
Member Author

@SteveL-MSFT SteveL-MSFT Aug 21, 2019

Choose a reason for hiding this comment

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

Nice catch! Will submit a new PR to address.

Copy link
Member Author

@SteveL-MSFT SteveL-MSFT Aug 21, 2019

Choose a reason for hiding this comment

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

#10405


internal enum FINDEX_INFO_LEVELS : uint
{
FindExInfoStandard = 0x0u,
FindExInfoBasic = 0x1u,
FindExInfoMaxInfoLevel = 0x2u,
}

internal enum FINDEX_SEARCH_OPS : uint
{
FindExSearchNameMatch = 0x0u,
FindExSearchLimitToDirectories = 0x1u,
FindExSearchLimitToDevices = 0x2u,
FindExSearchMaxSearchOp = 0x3u,
}

[StructLayout(LayoutKind.Sequential, CharSet = CharSet.Unicode)]
internal unsafe struct WIN32_FIND_DATA
{
internal uint dwFileAttributes;
internal System.Runtime.InteropServices.ComTypes.FILETIME ftCreationTime;
internal System.Runtime.InteropServices.ComTypes.FILETIME ftLastAccessTime;
internal System.Runtime.InteropServices.ComTypes.FILETIME ftLastWriteTime;
internal uint nFileSizeHigh;
internal uint nFileSizeLow;
internal uint dwReserved0;
internal uint dwReserved1;
internal fixed char cFileName[MAX_PATH];
internal fixed char cAlternateFileName[14];
}

/// <summary>
/// Gets the target of the specified reparse point.
/// </summary>
Expand Down Expand Up @@ -7992,6 +8029,25 @@ internal static bool IsReparsePoint(FileSystemInfo fileInfo)
: Platform.NonWindowsIsSymLink(fileInfo);
}

internal static bool IsNameSurrogateReparsePoint(string filePath)
{
#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))
Copy link
Collaborator

@iSazonov iSazonov May 4, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

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.

Copy link
Member Author

@SteveL-MSFT SteveL-MSFT May 4, 2019

Choose a reason for hiding this comment

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

CoreFx uses FindFirstFileEx() as well, they just name their wrapper FindFirstFile()

Copy link
Member Author

@SteveL-MSFT SteveL-MSFT May 4, 2019

Choose a reason for hiding this comment

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

On the second point, I agree it might be an oversight that they don't have symmetric behavior.

{
// Name surrogates are reparse points that point to other named entities local to the filesystem (like symlinks)
// In the case of OneDrive, they are not surrogates and would be safe to recurse into.
// This code is equivalent to the IsReparseTagNameSurrogate macro: https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/content/ntifs/nf-ntifs-isreparsetagnamesurrogate
if (!handle.IsInvalid && (data.dwReserved0 & 0x20000000) == 0)
{
return false;
}
}
#endif
// true means the reparse point is a symlink
return true;
}

internal static bool WinIsHardLink(FileSystemInfo fileInfo)
{
bool isHardLink = false;
Expand Down