From 731ab10b85a13321fb69561c0c6c49fedbf4e39b Mon Sep 17 00:00:00 2001 From: Ilya Date: Thu, 25 Feb 2021 00:51:20 +0500 Subject: [PATCH 1/8] Enhance Remove-Item to work with OneDrive --- .../commands/management/Navigation.cs | 2 +- .../namespaces/FileSystemProvider.cs | 43 +++++++++++-------- 2 files changed, 26 insertions(+), 19 deletions(-) diff --git a/src/Microsoft.PowerShell.Commands.Management/commands/management/Navigation.cs b/src/Microsoft.PowerShell.Commands.Management/commands/management/Navigation.cs index 23aa621266d..655c20182d3 100644 --- a/src/Microsoft.PowerShell.Commands.Management/commands/management/Navigation.cs +++ b/src/Microsoft.PowerShell.Commands.Management/commands/management/Navigation.cs @@ -2699,7 +2699,7 @@ protected override void ProcessRecord() try { System.IO.DirectoryInfo di = new(providerPath); - if (di != null && (di.Attributes & System.IO.FileAttributes.ReparsePoint) != 0) + if (di != null && InternalSymbolicLinkLinkCodeMethods.IsReparsePointLikeSymlink(di)) { shouldRecurse = false; treatAsFile = true; diff --git a/src/System.Management.Automation/namespaces/FileSystemProvider.cs b/src/System.Management.Automation/namespaces/FileSystemProvider.cs index db619e3d15e..522b4eb5a27 100644 --- a/src/System.Management.Automation/namespaces/FileSystemProvider.cs +++ b/src/System.Management.Automation/namespaces/FileSystemProvider.cs @@ -1912,7 +1912,7 @@ private void Dir( // c) it is not a reparse point with a target (not OneDrive or an AppX link). if (tracker == null) { - if (InternalSymbolicLinkLinkCodeMethods.IsReparsePointWithTarget(recursiveDirectory)) + if (InternalSymbolicLinkLinkCodeMethods.IsReparsePointLikeSymlink(recursiveDirectory)) { continue; } @@ -2064,7 +2064,7 @@ string ToModeString(FileSystemInfo fileSystemInfo) public static string NameString(PSObject instance) { return instance?.BaseObject is FileSystemInfo fileInfo - ? InternalSymbolicLinkLinkCodeMethods.IsReparsePointWithTarget(fileInfo) + ? InternalSymbolicLinkLinkCodeMethods.IsReparsePointLikeSymlink(fileInfo) ? $"{fileInfo.Name} -> {InternalSymbolicLinkLinkCodeMethods.GetTarget(instance)}" : fileInfo.Name : string.Empty; @@ -3104,15 +3104,11 @@ private void RemoveDirectoryInfoItem(DirectoryInfo directory, bool recurse, bool continueRemoval = ShouldProcess(directory.FullName, action); } - if (directory.Attributes.HasFlag(FileAttributes.ReparsePoint)) + if (InternalSymbolicLinkLinkCodeMethods.IsReparsePointLikeSymlink(directory)) { try { - // 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 + // Name surrogates should just be detached. directory.Delete(); } catch (Exception e) @@ -8244,28 +8240,39 @@ internal static bool IsReparsePoint(FileSystemInfo fileInfo) return fileInfo.Attributes.HasFlag(System.IO.FileAttributes.ReparsePoint); } - internal static bool IsReparsePointWithTarget(FileSystemInfo fileInfo) + internal static bool IsReparsePointLikeSymlink(FileSystemInfo fileInfo) { - if (!IsReparsePoint(fileInfo)) - { - return false; - } -#if !UNIX - // It is a reparse point and we should check some reparse point tags. - var data = new WIN32_FIND_DATA(); +#if UNIX + // Reparse point on Unix is a symlink. + return IsReparsePoint(fileInfo); +#else + WIN32_FIND_DATA data = default; using (var handle = FindFirstFileEx(fileInfo.FullName, FINDEX_INFO_LEVELS.FindExInfoBasic, ref data, FINDEX_SEARCH_OPS.FindExSearchNameMatch, IntPtr.Zero, 0)) { + if (handle.IsInvalid) + { + // It is our convention - if we can not open the file object we process it like a symlink. + return true; + } + + if ((data.dwReserved0 & 0x0400) == 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 (!handle.IsInvalid && (data.dwReserved0 & 0x20000000) == 0 && (data.dwReserved0 != IO_REPARSE_TAG_APPEXECLINK)) + if ((data.dwReserved0 & 0x20000000) == 0 && (data.dwReserved0 != IO_REPARSE_TAG_APPEXECLINK)) { return false; } } -#endif + return true; +#endif } internal static bool WinIsHardLink(FileSystemInfo fileInfo) From 25963886d875a28981394eca907f86df711dc540 Mon Sep 17 00:00:00 2001 From: Ilya Date: Thu, 25 Feb 2021 10:59:30 +0500 Subject: [PATCH 2/8] Fix reparse point check --- .../namespaces/FileSystemProvider.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/System.Management.Automation/namespaces/FileSystemProvider.cs b/src/System.Management.Automation/namespaces/FileSystemProvider.cs index 522b4eb5a27..5f2fe193d72 100644 --- a/src/System.Management.Automation/namespaces/FileSystemProvider.cs +++ b/src/System.Management.Automation/namespaces/FileSystemProvider.cs @@ -8255,7 +8255,10 @@ internal static bool IsReparsePointLikeSymlink(FileSystemInfo fileInfo) return true; } - if ((data.dwReserved0 & 0x0400) == 0) + // 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; From 8c81afe84b432c2fd1b67154e0b5f7a14a566387 Mon Sep 17 00:00:00 2001 From: Ilya Date: Thu, 25 Feb 2021 12:02:46 +0500 Subject: [PATCH 3/8] Add performance optimization --- .../namespaces/FileSystemProvider.cs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/System.Management.Automation/namespaces/FileSystemProvider.cs b/src/System.Management.Automation/namespaces/FileSystemProvider.cs index 5f2fe193d72..de552e8e70c 100644 --- a/src/System.Management.Automation/namespaces/FileSystemProvider.cs +++ b/src/System.Management.Automation/namespaces/FileSystemProvider.cs @@ -1896,9 +1896,17 @@ 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 @@ -1912,7 +1920,7 @@ private void Dir( // c) it is not a reparse point with a target (not OneDrive or an AppX link). if (tracker == null) { - if (InternalSymbolicLinkLinkCodeMethods.IsReparsePointLikeSymlink(recursiveDirectory)) + if (checkReparsePoint && InternalSymbolicLinkLinkCodeMethods.IsReparsePointLikeSymlink(recursiveDirectory)) { continue; } From ec6ddd2e019878a1f33f9b787309626cd6666ed3 Mon Sep 17 00:00:00 2001 From: Ilya Date: Thu, 25 Feb 2021 13:26:42 +0500 Subject: [PATCH 4/8] Add new test for Get-ChildItem on emulated OneDrive --- .../engine/Utils.cs | 4 +++- .../namespaces/FileSystemProvider.cs | 5 +++++ .../FileSystem.Tests.ps1 | 20 +++++++++++++++++++ 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/System.Management.Automation/engine/Utils.cs b/src/System.Management.Automation/engine/Utils.cs index fa1ef13572e..e03cbd616b9 100644 --- a/src/System.Management.Automation/engine/Utils.cs +++ b/src/System.Management.Automation/engine/Utils.cs @@ -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: @@ -2099,6 +2099,8 @@ public static class InternalTestHooks internal static bool ThrowExdevErrorOnMoveDirectory; + internal static bool EmulateOneDrive; + /// This member is used for internal test purposes. public static void SetTestHook(string property, object value) { diff --git a/src/System.Management.Automation/namespaces/FileSystemProvider.cs b/src/System.Management.Automation/namespaces/FileSystemProvider.cs index de552e8e70c..1c608708549 100644 --- a/src/System.Management.Automation/namespaces/FileSystemProvider.cs +++ b/src/System.Management.Automation/namespaces/FileSystemProvider.cs @@ -8254,6 +8254,11 @@ internal static bool IsReparsePointLikeSymlink(FileSystemInfo fileInfo) // Reparse point on Unix is a symlink. return IsReparsePoint(fileInfo); #else + if (InternalTestHooks.EmulateOneDrive) + { + return fileInfo.Name != "link-Beta"; + } + WIN32_FIND_DATA data = default; using (var handle = FindFirstFileEx(fileInfo.FullName, FINDEX_INFO_LEVELS.FindExInfoBasic, ref data, FINDEX_SEARCH_OPS.FindExSearchNameMatch, IntPtr.Zero, 0)) { diff --git a/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 index 0e4adaeec70..f86b7dc41b4 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 @@ -616,6 +616,26 @@ 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 + try + { + # '$betaDir' is a symlink - we don't follow symlinks + $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('EmulateOneDrive', $true) + $ci = Get-ChildItem -Path $alphaDir -Recurse + $ci.Count | Should -BeExactly 10 + } + finally + { + [System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('EmulateOneDrive', $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 From f3391402c35e38afe57d8abd62e5c92c9dd91b46 Mon Sep 17 00:00:00 2001 From: Ilya Date: Thu, 25 Feb 2021 17:09:11 +0500 Subject: [PATCH 5/8] Add new test for Remove-Item on emulated OneDrive --- .../engine/Utils.cs | 7 ++- .../namespaces/FileSystemProvider.cs | 9 +++- .../FileSystem.Tests.ps1 | 45 +++++++++++++++++-- 3 files changed, 55 insertions(+), 6 deletions(-) diff --git a/src/System.Management.Automation/engine/Utils.cs b/src/System.Management.Automation/engine/Utils.cs index e03cbd616b9..4683fcb24ff 100644 --- a/src/System.Management.Automation/engine/Utils.cs +++ b/src/System.Management.Automation/engine/Utils.cs @@ -2099,7 +2099,12 @@ public static class InternalTestHooks internal static bool ThrowExdevErrorOnMoveDirectory; - internal static bool EmulateOneDrive; + // 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 OneDriveTestRecuseOn; + internal static string OneDriveTestSymlinkName = "link-Beta"; /// This member is used for internal test purposes. public static void SetTestHook(string property, object value) diff --git a/src/System.Management.Automation/namespaces/FileSystemProvider.cs b/src/System.Management.Automation/namespaces/FileSystemProvider.cs index 1c608708549..18ca2aaf49e 100644 --- a/src/System.Management.Automation/namespaces/FileSystemProvider.cs +++ b/src/System.Management.Automation/namespaces/FileSystemProvider.cs @@ -3101,6 +3101,11 @@ private void RemoveDirectoryInfoItem(DirectoryInfo directory, bool recurse, bool { Dbg.Diagnostics.Assert(directory != null, "Caller should always check directory"); + if (InternalTestHooks.OneDriveTestOn && !InternalTestHooks.OneDriveTestRecuseOn && directory.Name == InternalTestHooks.OneDriveTestSymlinkName) + { + return; + } + bool continueRemoval = true; // We only want to confirm the removal if this is the root of the @@ -8254,9 +8259,9 @@ internal static bool IsReparsePointLikeSymlink(FileSystemInfo fileInfo) // Reparse point on Unix is a symlink. return IsReparsePoint(fileInfo); #else - if (InternalTestHooks.EmulateOneDrive) + if (InternalTestHooks.OneDriveTestOn && fileInfo.Name == InternalTestHooks.OneDriveTestSymlinkName) { - return fileInfo.Name != "link-Beta"; + return !InternalTestHooks.OneDriveTestRecuseOn; } WIN32_FIND_DATA data = default; diff --git a/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 index f86b7dc41b4..c53e713bc2c 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 @@ -563,7 +563,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" + $betaLink = Join-Path $alphaDir "link-Beta" # Don't change! The name is hard-code in PowerShell for OneDrive tests. $betaFile1 = Join-Path $betaDir "BetaFile1.txt" $betaFile2 = Join-Path $betaDir "BetaFile2.txt" $betaFile3 = Join-Path $betaDir "BetaFile3.txt" @@ -620,6 +620,9 @@ Describe "Hard link and symbolic link tests" -Tags "CI", "RequireAdminOnWindows" # 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('OneDriveTestRecuseOn', $false) try { # '$betaDir' is a symlink - we don't follow symlinks @@ -627,13 +630,14 @@ Describe "Hard link and symbolic link tests" -Tags "CI", "RequireAdminOnWindows" $ci.Count | Should -BeExactly 7 # Now we follow the symlink like on OneDrive - [System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('EmulateOneDrive', $true) + [System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestRecuseOn', $true) $ci = Get-ChildItem -Path $alphaDir -Recurse $ci.Count | Should -BeExactly 10 } finally { - [System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('EmulateOneDrive', $false) + [System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestRecuseOn', $false) + [System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestOn', $false) } } It "Get-ChildItem will recurse into symlinks given -FollowSymlink, avoiding link loops" { @@ -757,6 +761,41 @@ 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('OneDriveTestRecuseOn', $false) + try + { + # '$betaDir' is a symlink - we don't follow symlinks + { Remove-Item -Path $alphaDir -Recurse -ErrorAction Stop } | Should -Throw -ErrorId "DeleteSymbolicLinkFailed,Microsoft.PowerShell.Commands.RemoveItemCommand" + + # Now we follow the symlink like on OneDrive + [System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestRecuseOn', $true) + Remove-Item -Path $alphaDir -Recurse + Test-Item -Path $alphaDir | Should -BeFalse + } + finally + { + [System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestRecuseOn', $false) + [System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestOn', $false) + } + } + } } From cd7f760f68e6cd282e08fa2f19b057dbf06bccba Mon Sep 17 00:00:00 2001 From: Ilya Date: Thu, 25 Feb 2021 18:43:01 +0500 Subject: [PATCH 6/8] Fix tests 2 --- .../namespaces/FileSystemProvider.cs | 21 +++++++++++++------ .../FileSystem.Tests.ps1 | 11 +++++----- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/src/System.Management.Automation/namespaces/FileSystemProvider.cs b/src/System.Management.Automation/namespaces/FileSystemProvider.cs index 18ca2aaf49e..f87656ef88a 100644 --- a/src/System.Management.Automation/namespaces/FileSystemProvider.cs +++ b/src/System.Management.Automation/namespaces/FileSystemProvider.cs @@ -3101,10 +3101,11 @@ private void RemoveDirectoryInfoItem(DirectoryInfo directory, bool recurse, bool { Dbg.Diagnostics.Assert(directory != null, "Caller should always check directory"); - if (InternalTestHooks.OneDriveTestOn && !InternalTestHooks.OneDriveTestRecuseOn && directory.Name == InternalTestHooks.OneDriveTestSymlinkName) - { - return; - } + //if (InternalTestHooks.OneDriveTestOn && !InternalTestHooks.OneDriveTestRecuseOn && directory.Name == InternalTestHooks.OneDriveTestSymlinkName) + //{ + // WriteError(new ErrorRecord(exception: null, errorId: "DeleteSymbolicLinkFailed", ErrorCategory.WriteError, directory)); + // return; + //} bool continueRemoval = true; @@ -3121,8 +3122,16 @@ private void RemoveDirectoryInfoItem(DirectoryInfo directory, bool recurse, bool { try { - // Name surrogates should just be detached. - directory.Delete(); + if (InternalTestHooks.OneDriveTestOn) + { + WriteError(new ErrorRecord(new IOException(), errorId: "DeleteSymbolicLinkFailed", ErrorCategory.WriteError, directory)); + return; + } + else + { + // Name surrogates should just be detached. + directory.Delete(); + } } catch (Exception e) { diff --git a/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 index c53e713bc2c..78a6fb44c29 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 @@ -781,13 +781,14 @@ Describe "Hard link and symbolic link tests" -Tags "CI", "RequireAdminOnWindows" [System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestRecuseOn', $false) try { - # '$betaDir' is a symlink - we don't follow symlinks - { Remove-Item -Path $alphaDir -Recurse -ErrorAction Stop } | Should -Throw -ErrorId "DeleteSymbolicLinkFailed,Microsoft.PowerShell.Commands.RemoveItemCommand" + # With the test hook turned on we don't remove '$betaDir' symlink. + # This emulate PowerShell 7.1 and below behavior. + { Remove-Item -Path $betaLink -Recurse -ErrorAction Stop } | Should -Throw -ErrorId "DeleteSymbolicLinkFailed,Microsoft.PowerShell.Commands.RemoveItemCommand" - # Now we follow the symlink like on OneDrive + # Now we emulate OneDrive and follow the symlink like on OneDrive. [System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestRecuseOn', $true) - Remove-Item -Path $alphaDir -Recurse - Test-Item -Path $alphaDir | Should -BeFalse + Remove-Item -Path $betaLink -Recurse + Test-Path -Path $betaLink | Should -BeFalse } finally { From 382d5a68639b907aa1cd585e8cbb864665f221bc Mon Sep 17 00:00:00 2001 From: Ilya Date: Thu, 25 Feb 2021 20:18:03 +0500 Subject: [PATCH 7/8] Cleanups --- .../namespaces/FileSystemProvider.cs | 15 +++++++-------- .../FileSystem.Tests.ps1 | 5 +++-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/System.Management.Automation/namespaces/FileSystemProvider.cs b/src/System.Management.Automation/namespaces/FileSystemProvider.cs index f87656ef88a..e9bf2d7bcbe 100644 --- a/src/System.Management.Automation/namespaces/FileSystemProvider.cs +++ b/src/System.Management.Automation/namespaces/FileSystemProvider.cs @@ -3101,12 +3101,6 @@ private void RemoveDirectoryInfoItem(DirectoryInfo directory, bool recurse, bool { Dbg.Diagnostics.Assert(directory != null, "Caller should always check directory"); - //if (InternalTestHooks.OneDriveTestOn && !InternalTestHooks.OneDriveTestRecuseOn && directory.Name == InternalTestHooks.OneDriveTestSymlinkName) - //{ - // WriteError(new ErrorRecord(exception: null, errorId: "DeleteSymbolicLinkFailed", ErrorCategory.WriteError, directory)); - // return; - //} - bool continueRemoval = true; // We only want to confirm the removal if this is the root of the @@ -3120,11 +3114,16 @@ private void RemoveDirectoryInfoItem(DirectoryInfo directory, bool recurse, bool if (InternalSymbolicLinkLinkCodeMethods.IsReparsePointLikeSymlink(directory)) { + void WriteErrorHelper(Exception exception) + { + WriteError(new ErrorRecord(exception, errorId: "DeleteSymbolicLinkFailed", ErrorCategory.WriteError, directory)); + } + try { if (InternalTestHooks.OneDriveTestOn) { - WriteError(new ErrorRecord(new IOException(), errorId: "DeleteSymbolicLinkFailed", ErrorCategory.WriteError, directory)); + WriteErrorHelper(new IOException()); return; } else @@ -3137,7 +3136,7 @@ private void RemoveDirectoryInfoItem(DirectoryInfo directory, bool recurse, bool { string error = StringUtil.Format(FileSystemProviderStrings.CannotRemoveItem, directory.FullName, e.Message); var exception = new IOException(error, e); - WriteError(new ErrorRecord(exception, errorId: "DeleteSymbolicLinkFailed", ErrorCategory.WriteError, directory)); + WriteErrorHelper(exception); } return; diff --git a/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 index 78a6fb44c29..e9144853bb3 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 @@ -626,10 +626,11 @@ Describe "Hard link and symbolic link tests" -Tags "CI", "RequireAdminOnWindows" 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 + # Now we follow the symlink like on OneDrive. [System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestRecuseOn', $true) $ci = Get-ChildItem -Path $alphaDir -Recurse $ci.Count | Should -BeExactly 10 @@ -782,7 +783,7 @@ Describe "Hard link and symbolic link tests" -Tags "CI", "RequireAdminOnWindows" try { # With the test hook turned on we don't remove '$betaDir' symlink. - # This emulate PowerShell 7.1 and below behavior. + # 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. From 41df0b6d422426c9321a05ea268633b1b32f6f48 Mon Sep 17 00:00:00 2001 From: Ilya Date: Thu, 25 Feb 2021 23:33:45 +0500 Subject: [PATCH 8/8] Address feedback --- src/System.Management.Automation/engine/Utils.cs | 2 +- .../namespaces/FileSystemProvider.cs | 4 ++-- .../FileSystem.Tests.ps1 | 14 +++++++------- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/System.Management.Automation/engine/Utils.cs b/src/System.Management.Automation/engine/Utils.cs index 4683fcb24ff..f5a022e9e36 100644 --- a/src/System.Management.Automation/engine/Utils.cs +++ b/src/System.Management.Automation/engine/Utils.cs @@ -2103,7 +2103,7 @@ public static class InternalTestHooks // 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 OneDriveTestRecuseOn; + internal static bool OneDriveTestRecurseOn; internal static string OneDriveTestSymlinkName = "link-Beta"; /// This member is used for internal test purposes. diff --git a/src/System.Management.Automation/namespaces/FileSystemProvider.cs b/src/System.Management.Automation/namespaces/FileSystemProvider.cs index e9bf2d7bcbe..f8fd90bc3d7 100644 --- a/src/System.Management.Automation/namespaces/FileSystemProvider.cs +++ b/src/System.Management.Automation/namespaces/FileSystemProvider.cs @@ -8269,7 +8269,7 @@ internal static bool IsReparsePointLikeSymlink(FileSystemInfo fileInfo) #else if (InternalTestHooks.OneDriveTestOn && fileInfo.Name == InternalTestHooks.OneDriveTestSymlinkName) { - return !InternalTestHooks.OneDriveTestRecuseOn; + return !InternalTestHooks.OneDriveTestRecurseOn; } WIN32_FIND_DATA data = default; @@ -8277,7 +8277,7 @@ internal static bool IsReparsePointLikeSymlink(FileSystemInfo fileInfo) { if (handle.IsInvalid) { - // It is our convention - if we can not open the file object we process it like a symlink. + // If we can not open the file object we assume it's a symlink. return true; } diff --git a/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 index e9144853bb3..b41c6d16a08 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 @@ -563,7 +563,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-code in PowerShell for OneDrive tests. + $betaLink = Join-Path $alphaDir "link-Beta" # Don't change! The name is hard-coded in PowerShell for OneDrive tests. $betaFile1 = Join-Path $betaDir "BetaFile1.txt" $betaFile2 = Join-Path $betaDir "BetaFile2.txt" $betaFile3 = Join-Path $betaDir "BetaFile3.txt" @@ -622,7 +622,7 @@ Describe "Hard link and symbolic link tests" -Tags "CI", "RequireAdminOnWindows" #New-Item -ItemType SymbolicLink -Path $betaLink -Value $betaDir [System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestOn', $true) - [System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestRecuseOn', $false) + [System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestRecurseOn', $false) try { # '$betaDir' is a symlink - we don't follow symlinks @@ -631,13 +631,13 @@ Describe "Hard link and symbolic link tests" -Tags "CI", "RequireAdminOnWindows" $ci.Count | Should -BeExactly 7 # Now we follow the symlink like on OneDrive. - [System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestRecuseOn', $true) + [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('OneDriveTestRecuseOn', $false) + [System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestRecurseOn', $false) [System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestOn', $false) } } @@ -779,7 +779,7 @@ Describe "Hard link and symbolic link tests" -Tags "CI", "RequireAdminOnWindows" New-Item -ItemType SymbolicLink -Path $betaLink -Value $betaDir > $null [System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestOn', $true) - [System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestRecuseOn', $false) + [System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestRecurseOn', $false) try { # With the test hook turned on we don't remove '$betaDir' symlink. @@ -787,13 +787,13 @@ Describe "Hard link and symbolic link tests" -Tags "CI", "RequireAdminOnWindows" { 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('OneDriveTestRecuseOn', $true) + [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('OneDriveTestRecuseOn', $false) + [System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestRecurseOn', $false) [System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestOn', $false) } }