Skip to content

Revert "Enhance Remove-Item to work with OneDrive (#14902)" #15253

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
TravisEz13 merged 1 commit into from
Apr 16, 2021
Merged

Revert "Enhance Remove-Item to work with OneDrive (#14902)" #15253

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
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;
Copy link
Collaborator

@JamesWTruher JamesWTruher Apr 16, 2021

Choose a reason for hiding this comment

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

extra white space?

Copy link
Collaborator

@iSazonov iSazonov Apr 17, 2021

Choose a reason for hiding this comment

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

Yes, will remove in follow PR.

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
72 changes: 18 additions & 54 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 @@ -1890,17 +1890,9 @@ private void Dir(
}

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

// Performance optimization.
// Since we have already checked Attributes for Hidden we have already did a p/invoke
// and initialized Attributes property.
// So here we can check for ReparsePoint without new p/invoke.
// If it is not a reparse point we skip one p/invoke in IsReparsePointLikeSymlink() below.
checkReparsePoint = (recursiveDirectory.Attributes & FileAttributes.ReparsePoint) != 0;
}

// if "Hidden" is explicitly specified anywhere in the attribute filter, then override
Expand All @@ -1914,7 +1906,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 @@ -2066,7 +2058,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 @@ -3106,31 +3098,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 @@ -8232,47 +8215,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;
#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)
{
// If we can not open the file object we assume it's a symlink.
return true;
}

// To exclude one extra p/invoke in some scenarios
// we don't check fileInfo.FileAttributes
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