8000 Revert "Enhance Remove-Item to work with OneDrive (Second)" by rjmholt · Pull Request #15546 · PowerShell/PowerShell · GitHub
[go: up one dir, main page]
Skip to content

Revert "Enhance Remove-Item to work with OneDrive (Second)" #15546

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
merged 1 commit into from
Jun 11, 2021
Merged

Revert "Enhance Remove-Item to work with OneDrive (Second)" #15546

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion src/Microsoft.PowerShell.Commands.Management/commands/management/Navigation.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2699,7 +2699,7 @@ protected override void ProcessRecord()
try
{
System.IO.DirectoryInfo di = new(providerPath);
if (di != null && InternalSymbolicLinkLinkCodeMethods.IsReparsePointLikeSymlink(di))
if (di != null && (di.Attributes & System.IO.FileAttributes.ReparsePoint) != 0)
{
shouldRecurse = false;
treatAsFile = true;
Expand Down
9 changes: 1 addition & 8 deletions src/System.Management.Automation/engine/Utils.cs
View file
Open in desktop
< 8000 td id="diff-08b9d853f19b7b63dfb6b0fa7b634db55f2a20aeed01ffb5855b3e5b0da3431bL2107" data-line-number="2107" class="blob-num blob-num-deletion js-linkable-line-number">
Original file line number Diff line number Diff line change
Expand Up @@ -1863,7 +1863,7 @@ internal static string GetFormatStyleString(FormatStyle formatStyle)

if (ExperimentalFeature.IsEnabled("PSAnsiRendering"))
{
PSStyle psstyle = PSStyle.Instance;
PSStyle psstyle = PSStyle.Instance;
switch (formatStyle)
{
case FormatStyle.Reset:
Expand Down Expand Up @@ -2100,13 +2100,6 @@ public static class InternalTestHooks

internal static bool ThrowExdevErrorOnMoveDirectory;

// To emulate OneDrive behavior we use the hard-coded symlink.
// If OneDriveTestRecuseOn is false then the symlink works as regular symlink.
// If OneDriveTestRecuseOn is true then we resurce into the symlink as OneDrive should work.
internal static bool OneDriveTestOn;
internal static bool OneDriveTestRecurseOn;
internal static string OneDriveTestSymlinkName = "link-Beta";

/// <summary>This member is used for internal test purposes.</summary>
public static void SetTestHook(string property, object value)
{
Expand Down
76 changes: 20 additions & 56 deletions src/System.Management.Automation/namespaces/FileSystemProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1891,14 +1891,9 @@ private void Dir(
}

bool hidden = false;
bool checkReparsePoint = true;
if (!Force)
{
hidden = (recursiveDirectory.Attributes & FileAttributes.Hidden) != 0;

// We've already taken the expense of initializing the Attributes property here,
// so we can use that to avoid needing to call IsReparsePointLikeSymlink() later.
checkReparsePoint = (recursiveDirectory.Attributes & FileAttributes.ReparsePoint) != 0;
}

// if "Hidden" is explicitly specified anywhere in the attribute filter, then override
Expand All @@ -1912,7 +1907,7 @@ private void Dir(
// c) it is not a reparse point with a target (not OneDrive or an AppX link).
if (tracker == null)
{
if (checkReparsePoint && InternalSymbolicLinkLinkCodeMethods.IsReparsePointLikeSymlink(recursiveDirectory))
if (InternalSymbolicLinkLinkCodeMethods.IsReparsePointWithTarget(recursiveDirectory))
{
continue;
}
Expand Down Expand Up @@ -2064,7 +2059,7 @@ string ToModeString(FileSystemInfo fileSystemInfo)
public static string NameString(PSObject instance)
{
return instance?.BaseObject is FileSystemInfo fileInfo
? InternalSymbolicLinkLinkCodeMethods.IsReparsePointLikeSymlink(fileInfo)
? InternalSymbolicLinkLinkCodeMethods.IsReparsePointWithTarget(fileInfo)
? $"{fileInfo.Name} -> {InternalSymbolicLinkLinkCodeMethods.GetTarget(instance)}"
: fileInfo.Name
: string.Empty;
Expand Down Expand Up @@ -3104,31 +3099,22 @@ private void RemoveDirectoryInfoItem(DirectoryInfo directory, bool recurse, bool
continueRemoval = ShouldProcess(directory.FullName, action);
}

if (InternalSymbolicLinkLinkCodeMethods.IsReparsePointLikeSymlink(directory))
if (directory.Attributes.HasFlag(FileAttributes.ReparsePoint))
{
void WriteErrorHelper(Exception exception)
{
WriteError(new ErrorRecord(exception, errorId: "DeleteSymbolicLinkFailed", ErrorCategory.WriteError, directory));
}

try
{
if (InternalTestHooks.OneDriveTestOn)
{
WriteErrorHelper(new IOException());
return;
}
else
{
// Name surrogates should just be detached.
directory.Delete();
}
// TODO:
// Different symlinks seem to vary by behavior.
// In particular, OneDrive symlinks won't remove without recurse,
// but the .NET API here does not allow us to distinguish them.
// We may need to revisit using p/Invokes here to get the right behavior
directory.Delete();
}
catch (Exception e)
{
string error = StringUtil.Format(FileSystemProviderStrings.CannotRemoveItem, directory.FullName, e.Message);
var exception = new IOException(error, e);
WriteErrorHelper(exception);
WriteError(new ErrorRecord(exception, errorId: "DeleteSymbolicLinkFailed", ErrorCategory.WriteError, directory));
}

return;
Expand Down Expand Up @@ -8039,7 +8025,7 @@ protected override bool ReleaseHandle()
}

// SetLastError is false as the use of this API doesn't not require GetLastError() to be called
[DllImport(PinvokeDllNames.FindFirstFileDllName, EntryPoint = "FindFirstFileExW", SetLastError = true, CharSet = CharSet.Unicode)]
[DllImport(PinvokeDllNames.FindFirstFileDllName, EntryPoint = "FindFirstFileExW", SetLastError = false, CharSet = CharSet.Unicode)]
private static extern SafeFindHandle FindFirstFileEx(string lpFileName, FINDEX_INFO_LEVELS fInfoLevelId, ref WIN32_FIND_DATA lpFindFileData, FINDEX_SEARCH_OPS fSearchOp, IntPtr lpSearchFilter, int dwAdditionalFlags);

internal enum FINDEX_INFO_LEVELS : uint
Expand Down Expand Up @@ -8230,50 +8216,28 @@ internal static bool IsReparsePoint(FileSystemInfo fileInfo)
return fileInfo.Attributes.HasFlag(System.IO.FileAttributes.ReparsePoint);
}

internal static bool IsReparsePointLikeSymlink(FileSystemInfo fileInfo)
internal static bool IsReparsePointWithTarget(FileSystemInfo fileInfo)
{
#if UNIX
// Reparse point on Unix is a symlink.
return IsReparsePoint(fileInfo);
#else
if (InternalTestHooks.OneDriveTestOn && fileInfo.Name == InternalTestHooks.OneDriveTestSymlinkName)
if (!IsReparsePoint(fileInfo))
{
return !InternalTestHooks.OneDriveTestRecurseOn;
return false;
}

WIN32_FIND_DATA data = default;
string fullPath = Path.TrimEndingDirectorySeparator(fileInfo.FullName);
using (var handle = FindFirstFileEx(fullPath, FINDEX_INFO_LEVELS.FindExInfoBasic, ref data, FINDEX_SEARCH_OPS.FindExSearchNameMatch, IntPtr.Zero, 0))
#if !UNIX
// It is a reparse point and we should check some reparse point tags.
var data = new WIN32_FIND_DATA();
using (var handle = FindFirstFileEx(fileInfo.FullName, FINDEX_INFO_LEVELS.FindExInfoBasic, ref data, FINDEX_SEARCH_OPS.FindExSearchNameMatch, IntPtr.Zero, 0))
{
if (handle.IsInvalid)
{
// Our handle could be invalidated by something else touching the filesystem,
// so ensure we deal with that possibility here
int lastError = Marshal.GetLastWin32Error();
throw new Win32Exception(lastError);
}

// We already have the file attribute information from our Win32 call,
// so no need to take the expense of the FileInfo.FileAttributes call
const int FILE_ATTRIBUTE_REPARSE_POINT = 0x0400;
if ((data.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) == 0)
{
// Not a reparse point.
return false;
}

// The name surrogate bit 0x20000000 is defined in https://docs.microsoft.com/windows/win32/fileio/reparse-point-tags
// Name surrogates (0x20000000) are reparse points that point to other named entities local to the filesystem
// (like symlinks and mount points).
// In the case of OneDrive, they are not name surrogates and would be safe to recurse into.
if ((data.dwReserved0 & 0x20000000) == 0 && (data.dwReserved0 != IO_REPARSE_TAG_APPEXECLINK))
if (!handle.IsInvalid && (data.dwReserved0 & 0x20000000) == 0 && (data.dwReserved0 != IO_REPARSE_TAG_APPEXECLINK))
{
return false;
}
}

return true;
#endif
return true;
}

internal static bool WinIsHardLink(FileSystemInfo fileInfo)
Expand Down
63 changes: 1 addition & 62 deletions test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -570,7 +570,7 @@ Describe "Hard link and symbolic link tests" -Tags "CI", "RequireAdminOnWindows"
$omegaFile1 = Join-Path $omegaDir "OmegaFile1"
$omegaFile2 = Join-Path $omegaDir "OmegaFile2"
$betaDir = Join-Path $alphaDir "sub-Beta"
$betaLink = Join-Path $alphaDir "link-Beta" # Don't change! The name is hard-coded in PowerShell for OneDrive tests.
$betaLink = Join-Path $alphaDir "link-Beta"
$betaFile1 = Join-Path $betaDir "BetaFile1.txt"
$betaFile2 = Join-Path $betaDir "BetaFile2.txt"
$betaFile3 = Join-Path $betaDir "BetaFile3.txt"
Expand Down Expand Up @@ -623,31 +623,6 @@ Describe "Hard link and symbolic link tests" -Tags "CI", "RequireAdminOnWindows"
$ci = Get-ChildItem $alphaLink -Recurse -Name
$ci.Count | Should -BeExactly 7 # returns 10 - unexpectly recurce in link-alpha\link-Beta. See https://github.com/PowerShell/PowerShell/issues/11614
}
It "Get-ChildItem will recurse into emulated OneDrive directory" -Skip:(-not $IsWindows) {
# The test depends on the files created in previous test:
#New-Item -ItemType SymbolicLink -Path $alphaLink -Value $alphaDir
#New-Item -ItemType SymbolicLink -Path $betaLink -Value $betaDir

[System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestOn', $true)
[System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestRecurseOn', $false)
try
{
# '$betaDir' is a symlink - we don't follow symlinks
# This emulates PowerShell 6.2 and below behavior.
$ci = Get-ChildItem -Path $alphaDir -Recurse
$ci.Count | Should -BeExactly 7

# Now we follow the symlink like on OneDrive.
[System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestRecurseOn', $true)
$ci = Get-ChildItem -Path $alphaDir -Recurse
$ci.Count | Should -BeExactly 10
}
finally
{
[System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestRecurseOn', $false)
[System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestOn', $false)
}
}
It "Get-ChildItem will recurse into symlinks given -FollowSymlink, avoiding link loops" {
New-Item -ItemType Directory -Path $gammaDir
New-Item -ItemType SymbolicLink -Path $uponeLink -Value $betaDir
Expand Down Expand Up @@ -769,42 +744,6 @@ Describe "Hard link and symbolic link tests" -Tags "CI", "RequireAdminOnWindows"
$childB.Count | Should -BeExactly $childA.Count
$childB.Name | Should -BeExactly $childA.Name
}
It "Remove-Item will recurse into emulated OneDrive directory" -Skip:(-not $IsWindows) {
$alphaDir = Join-Path $TestDrive "sub-alpha2"
$alphaLink = Join-Path $TestDrive "link-alpha2"
$alphaFile1 = Join-Path $alphaDir "AlphaFile1.txt"
$betaDir = Join-Path $alphaDir "sub-Beta"
$betaLink = Join-Path $alphaDir "link-Beta"
$betaFile1 = Join-Path $betaDir "BetaFile1.txt"

New-Item -ItemType Directory -Path $alphaDir > $null
New-Item -ItemType File -Path $alphaFile1 > $null
New-Item -ItemType Directory -Path $betaDir > $null
New-Item -ItemType File -Path $betaFile1 > $null

New-Item -ItemType SymbolicLink -Path $alphaLink -Value $alphaDir > $null
New-Item -ItemType SymbolicLink -Path $betaLink -Value $betaDir > $null

[System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestOn', $true)
[System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestRecurseOn', $false)
try
{
# With the test hook turned on we don't remove '$betaDir' symlink.
# This emulates PowerShell 7.1 and below behavior.
{ Remove-Item -Path $betaLink -Recurse -ErrorAction Stop } | Should -Throw -ErrorId "DeleteSymbolicLinkFailed,Microsoft.PowerShell.Commands.RemoveItemCommand"

# Now we emulate OneDrive and follow the symlink like on OneDrive.
[System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestRecurseOn', $true)
Remove-Item -Path $betaLink -Recurse
Test-Path -Path $betaLink | Should -BeFalse
}
finally
{
[System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestRecurseOn', $false)
[System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestOn', $false)
}
}

}
}

Expand Down
Loading
0