From f8e0f7f83f81679b90d95e9f21afd78bad41d2bc Mon Sep 17 00:00:00 2001 From: Ilya Date: Mon, 12 Apr 2021 21:36:16 +0500 Subject: [PATCH 1/7] Enhance Remove-Item to work with OneDrive (#14902) --- .../commands/management/Navigation.cs | 2 +- .../engine/Utils.cs | 9 ++- .../namespaces/FileSystemProvider.cs | 72 ++++++++++++++----- .../FileSystem.Tests.ps1 | 63 +++++++++++++++- 4 files changed, 125 insertions(+), 21 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/engine/Utils.cs b/src/System.Management.Automation/engine/Utils.cs index 94957ced8b8..85fca4d6639 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: @@ -2100,6 +2100,13 @@ 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"; + /// 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 8b90a12044a..b16b63fe7e9 100644 --- a/src/System.Management.Automation/namespaces/FileSystemProvider.cs +++ b/src/System.Management.Automation/namespaces/FileSystemProvider.cs @@ -1890,9 +1890,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 @@ -1906,7 +1914,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 (checkReparsePoint && InternalSymbolicLinkLinkCodeMethods.IsReparsePointLikeSymlink(recursiveDirectory)) { continue; } @@ -2058,7 +2066,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; @@ -3098,22 +3106,31 @@ private void RemoveDirectoryInfoItem(DirectoryInfo directory, bool recurse, bool continueRemoval = ShouldProcess(directory.FullName, action); } - if (directory.Attributes.HasFlag(FileAttributes.ReparsePoint)) + if (InternalSymbolicLinkLinkCodeMethods.IsReparsePointLikeSymlink(directory)) { + void WriteErrorHelper(Exception exception) + { + WriteError(new ErrorRecord(exception, errorId: "DeleteSymbolicLinkFailed", ErrorCategory.WriteError, 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 - directory.Delete(); + if (InternalTestHooks.OneDriveTestOn) + { + WriteErrorHelper(new IOException()); + return; + } + else + { + // Name surrogates should just be detached. + directory.Delete(); + } } catch (Exception e) { 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; @@ -8215,28 +8232,47 @@ 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)) +#if UNIX + // Reparse point on Unix is a symlink. + return IsReparsePoint(fileInfo); +#else + if (InternalTestHooks.OneDriveTestOn && fileInfo.Name == InternalTestHooks.OneDriveTestSymlinkName) { - return false; + return !InternalTestHooks.OneDriveTestRecurseOn; } -#if !UNIX - // It is a reparse point and we should check some reparse point tags. - var data = new WIN32_FIND_DATA(); + + 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) + { + // 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 (!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) diff --git a/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 index f1b352a16bc..1bb9079ef98 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 @@ -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" + $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" @@ -623,6 +623,31 @@ 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 @@ -744,6 +769,42 @@ 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) + } + } + } } From 56c27c5994641e9e7410d5711ce2b855c3f3821d Mon Sep 17 00:00:00 2001 From: Ilya Date: Sat, 17 Apr 2021 19:25:45 +0500 Subject: [PATCH 2/7] Fix trailing backslash in path for FindFirstFileEx --- .../namespaces/FileSystemProvider.cs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/System.Management.Automation/namespaces/FileSystemProvider.cs b/src/System.Management.Automation/namespaces/FileSystemProvider.cs index b16b63fe7e9..0f4d2942b14 100644 --- a/src/System.Management.Automation/namespaces/FileSystemProvider.cs +++ b/src/System.Management.Automation/namespaces/FileSystemProvider.cs @@ -8041,7 +8041,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 = false, CharSet = CharSet.Unicode)] + [DllImport(PinvokeDllNames.FindFirstFileDllName, EntryPoint = "FindFirstFileExW", SetLastError = true, 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 @@ -8244,12 +8244,14 @@ internal static bool IsReparsePointLikeSymlink(FileSystemInfo fileInfo) } WIN32_FIND_DATA data = default; - using (var handle = FindFirstFileEx(fileInfo.FullName, FINDEX_INFO_LEVELS.FindExInfoBasic, ref data, FINDEX_SEARCH_OPS.FindExSearchNameMatch, IntPtr.Zero, 0)) + 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 (handle.IsInvalid) { - // If we can not open the file object we assume it's a symlink. - return true; + // We should never be here. + int lastError = Marshal.GetLastWin32Error(); + throw new Win32Exception(lastError); } // To exclude one extra p/invoke in some scenarios From 39d1e9427bf462633ccfc715e20e8753523a9253 Mon Sep 17 00:00:00 2001 From: Ilya Date: Sat, 17 Apr 2021 18:26:55 +0500 Subject: [PATCH 3/7] Reformat old tests --- .../Remove-Item.Tests.ps1 | 263 +++++++++--------- 1 file changed, 136 insertions(+), 127 deletions(-) diff --git a/test/powershell/Modules/Microsoft.PowerShell.Management/Remove-Item.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Management/Remove-Item.Tests.ps1 index 3e8ccb8fa91..aff404225c3 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Management/Remove-Item.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Management/Remove-Item.Tests.ps1 @@ -1,170 +1,179 @@ # Copyright (c) Microsoft Corporation. # Licensed under the MIT License. -Describe "Remove-Item" -Tags "CI" { - $testpath = $TestDrive - $testfile = "testfile.txt" - $testfilepath = Join-Path -Path $testpath -ChildPath $testfile - Context "File removal Tests" { - BeforeEach { - New-Item -Name $testfile -Path $testpath -ItemType "file" -Value "lorem ipsum" -Force - Test-Path $testfilepath | Should -BeTrue - } - It "Should be able to be called on a regular file without error using the Path parameter" { - { Remove-Item -Path $testfilepath } | Should -Not -Throw - - Test-Path $testfilepath | Should -BeFalse - } +Describe "Remove-Item" -Tags "CI" { + BeforeAll { + $testpath = $TestDrive + $testfile = "testfile.txt" + $testfilepath = Join-Path -Path $testpath -ChildPath $testfile + } - It "Should be able to be called on a file without the Path parameter" { - { Remove-Item $testfilepath } | Should -Not -Throw + Context "File removal Tests" { + BeforeEach { + New-Item -Name $testfile -Path $testpath -ItemType "file" -Value "lorem ipsum" -Force + Test-Path $testfilepath | Should -BeTrue + } - Test-Path $testfilepath | Should -BeFalse - } + It "Should be able to be called on a regular file without error using the Path parameter" { + { Remove-Item -Path $testfilepath } | Should -Not -Throw - It "Should be able to call the rm alias" { - { rm $testfilepath } | Should -Not -Throw + Test-Path $testfilepath | Should -BeFalse + } - Test-Path $testfilepath | Should -BeFalse - } + It "Should be able to be called on a file without the Path parameter" { + { Remove-Item $testfilepath } | Should -Not -Throw - It "Should be able to call the del alias" { - { del $testfilepath } | Should -Not -Throw + Test-Path $testfilepath | Should -BeFalse + } - Test-Path $testfilepath | Should -BeFalse - } + It "Should be able to call the rm alias" { + { rm $testfilepath } | Should -Not -Throw - It "Should be able to call the erase alias" { - { erase $testfilepath } | Should -Not -Throw + Test-Path $testfilepath | Should -BeFalse + } - Test-Path $testfilepath | Should -BeFalse - } + It "Should be able to call the del alias" { + { del $testfilepath } | Should -Not -Throw - It "Should be able to call the ri alias" { - { ri $testfilepath } | Should -Not -Throw + Test-Path $testfilepath | Should -BeFalse + } - Test-Path $testfilepath | Should -BeFalse - } + It "Should be able to call the erase alias" { + { erase $testfilepath } | Should -Not -Throw - It "Should not be able to remove a read-only document without using the force switch" { - # Set to read only - Set-ItemProperty -Path $testfilepath -Name IsReadOnly -Value $true + Test-Path $testfilepath | Should -BeFalse + } - # attempt to remove the file - { Remove-Item $testfilepath -ErrorAction SilentlyContinue } | Should -Not -Throw + It "Should be able to call the ri alias" { + { ri $testfilepath } | Should -Not -Throw - # validate - Test-Path $testfilepath | Should -BeTrue + Test-Path $testfilepath | Should -BeFalse + } - # remove using the -force switch on the readonly object - Remove-Item $testfilepath -Force + It "Should not be able to remove a read-only document without using the force switch" { + # Set to read only + Set-ItemProperty -Path $testfilepath -Name IsReadOnly -Value $true - # Validate - Test-Path $testfilepath | Should -BeFalse - } + # attempt to remove the file + { Remove-Item $testfilepath -ErrorAction SilentlyContinue } | Should -Not -Throw - It "Should be able to remove all files matching a regular expression with the include parameter" { - # Create multiple files with specific string - New-Item -Name file1.txt -Path $testpath -ItemType "file" -Value "lorem ipsum" - New-Item -Name file2.txt -Path $testpath -ItemType "file" -Value "lorem ipsum" - New-Item -Name file3.txt -Path $testpath -ItemType "file" -Value "lorem ipsum" - # Create a single file that does not match that string - already done in BeforeEach + # validate + Test-Path $testfilepath | Should -BeTrue - # Delete the specific string - Remove-Item (Join-Path -Path $testpath -ChildPath "*") -Include file*.txt - # validate that the string under test was deleted, and the nonmatching strings still exist - Test-Path (Join-Path -Path $testpath -ChildPath file1.txt) | Should -BeFalse - Test-Path (Join-Path -Path $testpath -ChildPath file2.txt) | Should -BeFalse - Test-Path (Join-Path -Path $testpath -ChildPath file3.txt) | Should -BeFalse - Test-Path $testfilepath | Should -BeTrue + # remove using the -force switch on the readonly object + Remove-Item $testfilepath -Force - # Delete the non-matching strings - Remove-Item $testfilepath + # Validate + Test-Path $testfilepath | Should -BeFalse + } - Test-Path $testfilepath | Should -BeFalse - } + It "Should be able to remove all files matching a regular expression with the include parameter" { + # Create multiple files with specific string + New-Item -Name file1.txt -Path $testpath -ItemType "file" -Value "lorem ipsum" + New-Item -Name file2.txt -Path $testpath -ItemType "file" -Value "lorem ipsum" + New-Item -Name file3.txt -Path $testpath -ItemType "file" -Value "lorem ipsum" + # Create a single file that does not match that string - already done in BeforeEach + + # Delete the specific string + Remove-Item (Join-Path -Path $testpath -ChildPath "*") -Include file*.txt + # validate that the string under test was deleted, and the nonmatching strings still exist + Test-Path (Join-Path -Path $testpath -ChildPath file1.txt) | Should -BeFalse + Test-Path (Join-Path -Path $testpath -ChildPath file2.txt) | Should -BeFalse + Test-Path (Join-Path -Path $testpath -ChildPath file3.txt) | Should -BeFalse + Test-Path $testfilepath | Should -BeTrue + + # Delete the non-matching strings + Remove-Item $testfilepath + + Test-Path $testfilepath | Should -BeFalse + } - It "Should be able to not remove any files matching a regular expression with the exclude parameter" { - # Create multiple files with specific string - New-Item -Name file1.wav -Path $testpath -ItemType "file" -Value "lorem ipsum" - New-Item -Name file2.wav -Path $testpath -ItemType "file" -Value "lorem ipsum" + It "Should be able to not remove any files matching a regular expression with the exclude parameter" { + # Create multiple files with specific string + New-Item -Name file1.wav -Path $testpath -ItemType "file" -Value "lorem ipsum" + New-Item -Name file2.wav -Path $testpath -ItemType "file" -Value "lorem ipsum" - # Create a single file that does not match that string - New-Item -Name file1.txt -Path $testpath -ItemType "file" -Value "lorem ipsum" + # Create a single file that does not match that string + New-Item -Name file1.txt -Path $testpath -ItemType "file" -Value "lorem ipsum" - # Delete the specific string - Remove-Item (Join-Path -Path $testpath -ChildPath "file*") -Exclude *.wav -Include *.txt + # Delete the specific string + Remove-Item (Join-Path -Path $testpath -ChildPath "file*") -Exclude *.wav -Include *.txt - # validate that the string under test was deleted, and the nonmatching strings still exist - Test-Path (Join-Path -Path $testpath -ChildPath file1.wav) | Should -BeTrue - Test-Path (Join-Path -Path $testpath -ChildPath file2.wav) | Should -BeTrue - Test-Path (Join-Path -Path $testpath -ChildPath file1.txt) | Should -BeFalse + # validate that the string under test was deleted, and the nonmatching strings still exist + Test-Path (Join-Path -Path $testpath -ChildPath file1.wav) | Should -BeTrue + Test-Path (Join-Path -Path $testpath -ChildPath file2.wav) | Should -BeTrue + Test-Path (Join-Path -Path $testpath -ChildPath file1.txt) | Should -BeFalse - # Delete the non-matching strings - Remove-Item (Join-Path -Path $testpath -ChildPath file1.wav) - Remove-Item (Join-Path -Path $testpath -ChildPath file2.wav) + # Delete the non-matching strings + Remove-Item (Join-Path -Path $testpath -ChildPath file1.wav) + Remove-Item (Join-Path -Path $testpath -ChildPath file2.wav) - Test-Path (Join-Path -Path $testpath -ChildPath file1.wav) | Should -BeFalse - Test-Path (Join-Path -Path $testpath -ChildPath file2.wav) | Should -BeFalse - } + Test-Path (Join-Path -Path $testpath -ChildPath file1.wav) | Should -BeFalse + Test-Path (Join-Path -Path $testpath -ChildPath file2.wav) | Should -BeFalse + } } Context "Directory Removal Tests" { - $testdirectory = Join-Path -Path $testpath -ChildPath testdir - $testsubdirectory = Join-Path -Path $testdirectory -ChildPath subd - BeforeEach { - New-Item -Name "testdir" -Path $testpath -ItemType "directory" -Force + BeforeAll { + $testdirectory = Join-Path -Path $testpath -ChildPath testdir + $testsubdirectory = Join-Path -Path $testdirectory -ChildPath subd + } - Test-Path $testdirectory | Should -BeTrue - } + BeforeEach { + New-Item -Name "testdir" -Path $testpath -ItemType "directory" -Force - It "Should be able to remove a directory" { - { Remove-Item $testdirectory } | Should -Not -Throw + Test-Path $testdirectory | Should -BeTrue + } + + It "Should be able to remove a directory" { + { Remove-Item $testdirectory } | Should -Not -Throw - Test-Path $testdirectory | Should -BeFalse - } + Test-Path $testdirectory | Should -BeFalse + } - It "Should be able to recursively delete subfolders" { - New-Item -Name "subd" -Path $testdirectory -ItemType "directory" - New-Item -Name $testfile -Path $testsubdirectory -ItemType "file" -Value "lorem ipsum" + It "Should be able to recursively delete subfolders" { + New-Item -Name "subd" -Path $testdirectory -ItemType "directory" + New-Item -Name $testfile -Path $testsubdirectory -ItemType "file" -Value "lorem ipsum" - $complexDirectory = Join-Path -Path $testsubdirectory -ChildPath $testfile - Test-Path $complexDirectory | Should -BeTrue + $complexDirectory = Join-Path -Path $testsubdirectory -ChildPath $testfile + Test-Path $complexDirectory | Should -BeTrue - { Remove-Item $testdirectory -Recurse} | Should -Not -Throw + { Remove-Item $testdirectory -Recurse } | Should -Not -Throw - Test-Path $testdirectory | Should -BeFalse - } + Test-Path $testdirectory | Should -BeFalse + } } Context "Alternate Data Streams should be supported on Windows" { - BeforeAll { - if (!$IsWindows) { - return - } - $fileName = "ADStest.txt" - $streamName = "teststream" - $dirName = "ADStestdir" - $fileContent =" This is file content." - $streamContent = "datastream content here" - $streamfile = Join-Path -Path $testpath -ChildPath $fileName - $streamdir = Join-Path -Path $testpath -ChildPath $dirName - - $null = New-Item -Path $streamfile -ItemType "File" -force - Add-Content -Path $streamfile -Value $fileContent - Add-Content -Path $streamfile -Stream $streamName -Value $streamContent - $null = New-Item -Path $streamdir -ItemType "Directory" -Force - Add-Content -Path $streamdir -Stream $streamName -Value $streamContent - } - It "Should completely remove a datastream from a file" -Skip:(!$IsWindows) { - Get-Item -Path $streamfile -Stream $streamName | Should -Not -BeNullOrEmpty - Remove-Item -Path $streamfile -Stream $streamName - Get-Item -Path $streamfile -Stream $streamName -ErrorAction SilentlyContinue | Should -BeNullOrEmpty - } - It "Should completely remove a datastream from a directory" -Skip:(!$IsWindows) { - Get-Item -Path $streamdir -Stream $streamName | Should -Not -BeNullOrEmpty - Remove-Item -Path $streamdir -Stream $streamName - Get-Item -Path $streamdir -Stream $streamname -ErrorAction SilentlyContinue | Should -BeNullOrEmpty - } + BeforeAll { + if (!$IsWindows) { + return + } + $fileName = "ADStest.txt" + $streamName = "teststream" + $dirName = "ADStestdir" + $fileContent =" This is file content." + $streamContent = "datastream content here" + $streamfile = Join-Path -Path $testpath -ChildPath $fileName + $streamdir = Join-Path -Path $testpath -ChildPath $dirName + + $null = New-Item -Path $streamfile -ItemType "File" -force + Add-Content -Path $streamfile -Value $fileContent + Add-Content -Path $streamfile -Stream $streamName -Value $streamContent + $null = New-Item -Path $streamdir -ItemType "Directory" -Force + Add-Content -Path $streamdir -Stream $streamName -Value $streamContent + } + + It "Should completely remove a datastream from a file" -Skip:(!$IsWindows) { + Get-Item -Path $streamfile -Stream $streamName | Should -Not -BeNullOrEmpty + Remove-Item -Path $streamfile -Stream $streamName + Get-Item -Path $streamfile -Stream $streamName -ErrorAction SilentlyContinue | Should -BeNullOrEmpty + } + + It "Should completely remove a datastream from a directory" -Skip:(!$IsWindows) { + Get-Item -Path $streamdir -Stream $streamName | Should -Not -BeNullOrEmpty + Remove-Item -Path $streamdir -Stream $streamName + Get-Item -Path $streamdir -Stream $streamname -ErrorAction SilentlyContinue | Should -BeNullOrEmpty + } } } From 1bc1de8e36ae5fe5b4ebd42e4bff81f2814f3ccc Mon Sep 17 00:00:00 2001 From: Ilya Date: Sat, 17 Apr 2021 18:37:41 +0500 Subject: [PATCH 4/7] Add new test --- .../Remove-Item.Tests.ps1 | 22 ++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/test/powershell/Modules/Microsoft.PowerShell.Management/Remove-Item.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Management/Remove-Item.Tests.ps1 index aff404225c3..28c5ed6052d 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Management/Remove-Item.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Management/Remove-Item.Tests.ps1 @@ -126,7 +126,7 @@ Describe "Remove-Item" -Tags "CI" { } It "Should be able to remove a directory" { - { Remove-Item $testdirectory } | Should -Not -Throw + { Remove-Item $testdirectory -ErrorAction Stop } | Should -Not -Throw Test-Path $testdirectory | Should -BeFalse } @@ -138,16 +138,32 @@ Describe "Remove-Item" -Tags "CI" { $complexDirectory = Join-Path -Path $testsubdirectory -ChildPath $testfile Test-Path $complexDirectory | Should -BeTrue - { Remove-Item $testdirectory -Recurse } | Should -Not -Throw + { Remove-Item $testdirectory -Recurse -ErrorAction Stop } | Should -Not -Throw Test-Path $testdirectory | Should -BeFalse } + + It "Should be able to recursively delete a directory with a trailing backslash" { + New-Item -Name "subd" -Path $testdirectory -ItemType "directory" + New-Item -Name $testfile -Path $testsubdirectory -ItemType "file" -Value "lorem ipsum" + + $complexDirectory = Join-Path -Path $testsubdirectory -ChildPath $testfile + Test-Path $complexDirectory | Should -BeTrue + + $testdirectoryWithBackSlash = Join-Path -Path $testdirectory -ChildPath ([IO.Path]::DirectorySeparatorChar) + Test-Path $testdirectoryWithBackSlash | Should -BeTrue + + { Remove-Item $testdirectoryWithBackSlash -Recurse -ErrorAction Stop } | Should -Not -Throw + + Test-Path $testdirectoryWithBackSlash | Should -BeFalse + Test-Path $testdirectory | Should -BeFalse + } } Context "Alternate Data Streams should be supported on Windows" { BeforeAll { if (!$IsWindows) { - return + return } $fileName = "ADStest.txt" $streamName = "teststream" From 6bbea77b256b4cf27ccf9229a9d3a969d7c38da0 Mon Sep 17 00:00:00 2001 From: Ilya Date: Tue, 27 Apr 2021 22:06:20 +0500 Subject: [PATCH 5/7] Update src/System.Management.Automation/namespaces/FileSystemProvider.cs Co-authored-by: Robert Holt --- .../namespaces/FileSystemProvider.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/System.Management.Automation/namespaces/FileSystemProvider.cs b/src/System.Management.Automation/namespaces/FileSystemProvider.cs index 0f4d2942b14..1a25ad1a931 100644 --- a/src/System.Management.Automation/namespaces/FileSystemProvider.cs +++ b/src/System.Management.Automation/namespaces/FileSystemProvider.cs @@ -8254,8 +8254,8 @@ internal static bool IsReparsePointLikeSymlink(FileSystemInfo fileInfo) throw new Win32Exception(lastError); } - // To exclude one extra p/invoke in some scenarios - // we don't check fileInfo.FileAttributes + // 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) { From 5eec940b89a316ec3a33f1c6a43cc61f99c3d88b Mon Sep 17 00:00:00 2001 From: Ilya Date: Tue, 27 Apr 2021 22:09:27 +0500 Subject: [PATCH 6/7] Update src/System.Management.Automation/namespaces/FileSystemProvider.cs Co-authored-by: Robert Holt --- .../namespaces/FileSystemProvider.cs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/System.Management.Automation/namespaces/FileSystemProvider.cs b/src/System.Management.Automation/namespaces/FileSystemProvider.cs index 1a25ad1a931..6c6efa49e40 100644 --- a/src/System.Management.Automation/namespaces/FileSystemProvider.cs +++ b/src/System.Management.Automation/namespaces/FileSystemProvider.cs @@ -1895,11 +1895,8 @@ private void Dir( { 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. + // 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; } From 64f89c27b3084b87dbcb0fb9f0587349591481eb Mon Sep 17 00:00:00 2001 From: Ilya Date: Tue, 27 Apr 2021 22:43:24 +0500 Subject: [PATCH 7/7] Update src/System.Management.Automation/namespaces/FileSystemProvider.cs Co-authored-by: Robert Holt --- .../namespaces/FileSystemProvider.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/System.Management.Automation/namespaces/FileSystemProvider.cs b/src/System.Management.Automation/namespaces/FileSystemProvider.cs index 6c6efa49e40..95a2d89fb31 100644 --- a/src/System.Management.Automation/namespaces/FileSystemProvider.cs +++ b/src/System.Management.Automation/namespaces/FileSystemProvider.cs @@ -8246,7 +8246,8 @@ internal static bool IsReparsePointLikeSymlink(FileSystemInfo fileInfo) { if (handle.IsInvalid) { - // We should never be here. + // 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); }