From d472ff981fb711d71f85642957895134ca371b29 Mon Sep 17 00:00:00 2001 From: Ilya Date: Sat, 17 Apr 2021 17:48:35 +0500 Subject: [PATCH 1/3] Remove trailing slash in path passed to FindFirstFileEx --- .../namespaces/FileSystemProvider.cs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/System.Management.Automation/namespaces/FileSystemProvider.cs b/src/System.Management.Automation/namespaces/FileSystemProvider.cs index 8b90a12044a..7062c344afc 100644 --- a/src/System.Management.Automation/namespaces/FileSystemProvider.cs +++ b/src/System.Management.Automation/namespaces/FileSystemProvider.cs @@ -8024,7 +8024,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 @@ -8224,13 +8224,21 @@ internal static bool IsReparsePointWithTarget(FileSystemInfo fileInfo) #if !UNIX // It is a reparse point and we should check some reparse point tags. var data = new WIN32_FIND_DATA(); + string fullPath = Path.TrimEndingDirectorySeparator(fileInfo.FullName); using (var handle = FindFirstFileEx(fileInfo.FullName, FINDEX_INFO_LEVELS.FindExInfoBasic, ref data, FINDEX_SEARCH_OPS.FindExSearchNameMatch, IntPtr.Zero, 0)) { + if (handle.IsInvalid) + { + // We should never be here. + int lastError = Marshal.GetLastWin32Error(); + throw new Win32Exception(lastError); + } + // 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; } From 85127c3ca3fdd8acba789e37e2271ef42fb557de Mon Sep 17 00:00:00 2001 From: Ilya Date: Sat, 17 Apr 2021 18:26:55 +0500 Subject: [PATCH 2/3] 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 4027f426442c39762146aa5106754c3f206041d5 Mon Sep 17 00:00:00 2001 From: Ilya Date: Sat, 17 Apr 2021 18:37:41 +0500 Subject: [PATCH 3/3] 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..be5a2a51bfb 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 $testsubdirectory -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"