From 622bd249afa69b602a74a0b8c3dabc10b5648e6b Mon Sep 17 00:00:00 2001 From: Ilya Date: Wed, 28 Apr 2021 00:31:12 +0500 Subject: [PATCH 01/11] Enhance Remove-Item to work with OneDrive (Second) (#15260) --- .../commands/management/Navigation.cs | 2 +- .../engine/Utils.cs | 9 +- .../namespaces/FileSystemProvider.cs | 74 +++-- .../FileSystem.Tests.ps1 | 63 +++- .../Remove-Item.Tests.ps1 | 277 ++++++++++-------- 5 files changed, 277 insertions(+), 148 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 bc443ac10a9..c09ae3f08fd 100644 --- a/src/System.Management.Automation/engine/Utils.cs +++ b/src/System.Management.Automation/engine/Utils.cs @@ -1864,7 +1864,7 @@ internal static string GetFormatStyleString(FormatStyle formatStyle) if (ExperimentalFeature.IsEnabled("PSAnsiRendering")) { - PSStyle psstyle = PSStyle.Instance; + PSStyle psstyle = PSStyle.Instance; switch (formatStyle) { case FormatStyle.Reset: @@ -2104,6 +2104,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 a32dc421748..d6f7100eecb 100644 --- a/src/System.Management.Automation/namespaces/FileSystemProvider.cs +++ b/src/System.Management.Automation/namespaces/FileSystemProvider.cs @@ -1891,9 +1891,14 @@ private void Dir( } bool hidden = false; + bool checkReparsePoint = true; if (!Force) { hidden = (recursiveDirectory.Attributes & FileAttributes.Hidden) != 0; + + // We've already taken the expense of initializing the Attributes property here, + // so we can use that to avoid needing to call IsReparsePointLikeSymlink() later. + checkReparsePoint = (recursiveDirectory.Attributes & FileAttributes.ReparsePoint) != 0; } // if "Hidden" is explicitly specified anywhere in the attribute filter, then override @@ -1907,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 (checkReparsePoint && InternalSymbolicLinkLinkCodeMethods.IsReparsePointLikeSymlink(recursiveDirectory)) { continue; } @@ -3131,22 +3136,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; @@ -8057,7 +8071,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 @@ -8248,28 +8262,50 @@ 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(); - using (var handle = FindFirstFileEx(fileInfo.FullName, FINDEX_INFO_LEVELS.FindExInfoBasic, ref data, FINDEX_SEARCH_OPS.FindExSearchNameMatch, IntPtr.Zero, 0)) + + WIN32_FIND_DATA data = default; + string fullPath = Path.TrimEndingDirectorySeparator(fileInfo.FullName); + using (var handle = FindFirstFileEx(fullPath, FINDEX_INFO_LEVELS.FindExInfoBasic, ref data, FINDEX_SEARCH_OPS.FindExSearchNameMatch, IntPtr.Zero, 0)) { + if (handle.IsInvalid) + { + // Our handle could be invalidated by something else touching the filesystem, + // so ensure we deal with that possibility here + int lastError = Marshal.GetLastWin32Error(); + throw new Win32Exception(lastError); + } + + // We already have the file attribute information from our Win32 call, + // so no need to take the expense of the FileInfo.FileAttributes call + const int FILE_ATTRIBUTE_REPARSE_POINT = 0x0400; + if ((data.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT) == 0) + { + // Not a reparse point. + return false; + } + // The name surrogate bit 0x20000000 is defined in https://docs.microsoft.com/windows/win32/fileio/reparse-point-tags // Name surrogates (0x20000000) are reparse points that point to other named entities local to the filesystem // (like symlinks and mount points). // In the case of OneDrive, they are not name surrogates and would be safe to recurse into. - if (!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) + } + } + } } 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..28c5ed6052d 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,195 @@ # 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 + BeforeAll { + $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 - } + 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 + 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 - } + 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 be called on a file without the Path parameter" { + { Remove-Item $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 rm alias" { + { rm $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 del alias" { + { del $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 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 ri alias" { + { ri $testfilepath } | Should -Not -Throw + + Test-Path $testfilepath | Should -BeFalse + } - It "Should be able to call the ri alias" { - { ri $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 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 -BeTrue - # attempt to remove the file - { Remove-Item $testfilepath -ErrorAction SilentlyContinue } | Should -Not -Throw + # remove using the -force switch on the readonly object + Remove-Item $testfilepath -Force - # validate - Test-Path $testfilepath | Should -BeTrue + # Validate + 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 + } - # remove using the -force switch on the readonly object - Remove-Item $testfilepath -Force + 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" - # Validate - Test-Path $testfilepath | Should -BeFalse - } + # Create a single file that does not match that string + New-Item -Name file1.txt -Path $testpath -ItemType "file" -Value "lorem ipsum" - 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 "file*") -Exclude *.wav -Include *.txt - # 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 + # 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 $testfilepath + # 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 $testfilepath | Should -BeFalse - } + Test-Path (Join-Path -Path $testpath -ChildPath file1.wav) | Should -BeFalse + Test-Path (Join-Path -Path $testpath -ChildPath file2.wav) | 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" + Context "Directory Removal Tests" { + BeforeAll { + $testdirectory = Join-Path -Path $testpath -ChildPath testdir + $testsubdirectory = Join-Path -Path $testdirectory -ChildPath subd + } - # Create a single file that does not match that string - New-Item -Name file1.txt -Path $testpath -ItemType "file" -Value "lorem ipsum" + BeforeEach { + New-Item -Name "testdir" -Path $testpath -ItemType "directory" -Force - # Delete the specific string - Remove-Item (Join-Path -Path $testpath -ChildPath "file*") -Exclude *.wav -Include *.txt + Test-Path $testdirectory | Should -BeTrue + } - # 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 + It "Should be able to remove a directory" { + { Remove-Item $testdirectory -ErrorAction Stop } | Should -Not -Throw - # 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 $testdirectory | Should -BeFalse + } - Test-Path (Join-Path -Path $testpath -ChildPath file1.wav) | Should -BeFalse - Test-Path (Join-Path -Path $testpath -ChildPath file2.wav) | 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" - 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 + $complexDirectory = Join-Path -Path $testsubdirectory -ChildPath $testfile + Test-Path $complexDirectory | Should -BeTrue - Test-Path $testdirectory | Should -BeTrue - } + { Remove-Item $testdirectory -Recurse -ErrorAction Stop } | Should -Not -Throw - 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 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" - 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 + $testdirectoryWithBackSlash = Join-Path -Path $testdirectory -ChildPath ([IO.Path]::DirectorySeparatorChar) + Test-Path $testdirectoryWithBackSlash | Should -BeTrue - { Remove-Item $testdirectory -Recurse} | Should -Not -Throw + { Remove-Item $testdirectoryWithBackSlash -Recurse -ErrorAction Stop } | Should -Not -Throw - Test-Path $testdirectory | Should -BeFalse - } + Test-Path $testdirectoryWithBackSlash | 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 82189e84a7f215a1e7d6db79948f4ca7b156e4d6 Mon Sep 17 00:00:00 2001 From: Ilya Date: Sun, 13 Jun 2021 09:08:06 +0500 Subject: [PATCH 02/11] Enhance Remove-Item to work with OneDrive (Third) --- .../commands/management/Navigation.cs | 2 +- .../namespaces/FileSystemProvider.cs | 5 + .../utils/PathUtils.cs | 101 +++++++++ .../FileSystem.Tests.ps1 | 193 ++++++++++++------ 4 files changed, 239 insertions(+), 62 deletions(-) diff --git a/src/Microsoft.PowerShell.Commands.Management/commands/management/Navigation.cs b/src/Microsoft.PowerShell.Commands.Management/commands/management/Navigation.cs index 655c20182d3..c00c374611e 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 && InternalSymbolicLinkLinkCodeMethods.IsReparsePointLikeSymlink(di)) + if (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 d6f7100eecb..f04842f78fd 100644 --- a/src/System.Management.Automation/namespaces/FileSystemProvider.cs +++ b/src/System.Management.Automation/namespaces/FileSystemProvider.cs @@ -8275,6 +8275,11 @@ internal static bool IsReparsePointLikeSymlink(FileSystemInfo fileInfo) WIN32_FIND_DATA data = default; string fullPath = Path.TrimEndingDirectorySeparator(fileInfo.FullName); + if (fullPath.Length > MAX_PATH) + { + fullPath = PathUtils.EnsureExtendedPrefix(fullPath); + } + using (var handle = FindFirstFileEx(fullPath, FINDEX_INFO_LEVELS.FindExInfoBasic, ref data, FINDEX_SEARCH_OPS.FindExSearchNameMatch, IntPtr.Zero, 0)) { if (handle.IsInvalid) diff --git a/src/System.Management.Automation/utils/PathUtils.cs b/src/System.Management.Automation/utils/PathUtils.cs index 991e3fa63b6..83a2703fa0d 100644 --- a/src/System.Management.Automation/utils/PathUtils.cs +++ b/src/System.Management.Automation/utils/PathUtils.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using System.Globalization; using System.IO; +using System.Runtime.CompilerServices; using System.Text; using System.Management.Automation.Internal; @@ -447,5 +448,105 @@ internal static bool TryDeleteFile(string filepath) return false; } + + #region Helpers for long paths from .Net RUmtime + private const string ExtendedDevicePathPrefix = @"\\?\"; + private const string UncPathPrefix = @"\\"; + private const string UncDevicePrefixToInsert = @"?\UNC\"; + private const string UncExtendedPathPrefix = @"\\?\UNC\"; + private const string DevicePathPrefix = @"\\.\"; + + // \\?\, \\.\, \??\ + private const int DevicePrefixLength = 4; + + /// + /// Returns true if the given character is a valid drive letter + /// + private static bool IsValidDriveChar(char value) + { + return ((value >= 'A' && value <= 'Z') || (value >= 'a' && value <= 'z')); + } + + internal static string EnsureExtendedPrefix(string path) + { + if (IsPartiallyQualified(path) || IsDevice(path)) + return path; + + // Given \\server\share in longpath becomes \\?\UNC\server\share + if (path.StartsWith(UncPathPrefix, StringComparison.OrdinalIgnoreCase)) + return path.Insert(2, UncDevicePrefixToInsert); + + return ExtendedDevicePathPrefix + path; + } + + private static bool IsDevice(string path) + { + return IsExtended(path) + || + ( + path.Length >= DevicePrefixLength + && IsDirectorySeparator(path[0]) + && IsDirectorySeparator(path[1]) + && (path[2] == '.' || path[2] == '?') + && IsDirectorySeparator(path[3]) + ); + } + + private static bool IsExtended(string path) + { + return path.Length >= DevicePrefixLength + && path[0] == '\\' + && (path[1] == '\\' || path[1] == '?') + && path[2] == '?' + && path[3] == '\\'; + } + + /// + /// Returns true if the path specified is relative to the current drive or working directory. + /// Returns false if the path is fixed to a specific drive or UNC path. This method does no + /// validation of the path (URIs will be returned as relative as a result). + /// + /// + /// Handles paths that use the alternate directory separator. It is a frequent mistake to + /// assume that rooted paths (Path.IsPathRooted) are not relative. This isn't the case. + /// "C:a" is drive relative- meaning that it will be resolved against the current directory + /// for C: (rooted, but relative). "C:\a" is rooted and not relative (the current directory + /// will not be used to modify the path). + /// + private static bool IsPartiallyQualified(string path) + { + if (path.Length < 2) + { + // It isn't fixed, it must be relative. There is no way to specify a fixed + // path with one character (or less). + return true; + } + + if (IsDirectorySeparator(path[0])) + { + // There is no valid way to specify a relative path with two initial slashes or + // \? as ? isn't valid for drive relative paths and \??\ is equivalent to \\?\ + return !(path[1] == '?' || IsDirectorySeparator(path[1])); + } + + // The only way to specify a fixed path that doesn't begin with two slashes + // is the drive, colon, slash format- i.e. C:\ + return !((path.Length >= 3) + && (path[1] == Path.VolumeSeparatorChar) + && IsDirectorySeparator(path[2]) + // To match old behavior we'll check the drive character for validity as the path is technically + // not qualified if you don't have a valid drive. "=:\" is the "=" file's default data stream. + && IsValidDriveChar(path[0])); + } + /// + /// True if the given character is a directory separator. + /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private static bool IsDirectorySeparator(char c) + { + return c == Path.DirectorySeparatorChar || c == Path.AltDirectorySeparatorChar; + } + + #endregion } } diff --git a/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 index 1bb9079ef98..4ad91d23e38 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 @@ -92,6 +92,22 @@ Describe "Basic FileSystem Provider Tests" -Tags "CI" { $existsAfter | Should -BeFalse } + It "Verify Remove-Item for file" { + $longDir = 'a' * 250 + $longSubDir = 'b' * 250 + $fileName = "file1.txt" + $topPath = Join-Path $TestDrive $longDir + $longDirPath = Join-Path $topPath $longSubDir + $longFilePath = Join-Path $longDirPath $fileName + $null = New-Item -itemtype file -path $longFilePath -force + + $longFilePath | Should -Exist + + Remove-Item -Path $longFilePath -Force + + $longFilePath | Should -Not -Exist + } + It "Verify Rename-Item for file" { Rename-Item -Path $testFile -NewName $newTestFile -ErrorAction Stop $testFile | Should -Not -Exist @@ -623,31 +639,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 @@ -769,42 +760,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) - } - } - } } @@ -1590,3 +1545,119 @@ Describe "Windows admin tests" -Tag 'RequireAdminOnWindows' { } } } + +Describe "OneDrive filesystem manipulation" -Tags "CI" { + BeforeAll { + # on macOS, the /tmp directory is a symlink, so we'll resolve it here + $TestPath = $TestDrive + if ($IsMacOS) + { + $item = Get-Item $TestPath + $dirName = $item.BaseName + $item = Get-Item $item.PSParentPath -Force + if ($item.LinkType -eq "SymbolicLink") + { + $TestPath = Join-Path $item.Target $dirName + } + } + + $realFile = Join-Path $TestPath "file.txt" + $nonFile = Join-Path $TestPath "not-a-file" + $fileContent = "some text" + $realDir = Join-Path $TestPath "subdir" + $nonDir = Join-Path $TestPath "not-a-dir" + $hardLinkToFile = Join-Path $TestPath "hard-to-file.txt" + $symLinkToFile = Join-Path $TestPath "sym-link-to-file.txt" + $symLinkToDir = Join-Path $TestPath "sym-link-to-dir" + $symLinkToNothing = Join-Path $TestPath "sym-link-to-nowhere" + $dirSymLinkToDir = Join-Path $TestPath "symd-link-to-dir" + $junctionToDir = Join-Path $TestPath "junction-to-dir" + + New-Item -ItemType File -Path $realFile -Value $fileContent > $null + New-Item -ItemType Directory -Path $realDir > $null + + $alphaDir = Join-Path $TestDrive "sub-alpha" + $alphaLink = Join-Path $TestDrive "link-alpha" + $alphaFile1 = Join-Path $alphaDir "AlphaFile1.txt" + $alphaFile2 = Join-Path $alphaDir "AlphaFile2.txt" + $omegaDir = Join-Path $TestDrive "sub-omega" + $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. + $betaFile1 = Join-Path $betaDir "BetaFile1.txt" + $betaFile2 = Join-Path $betaDir "BetaFile2.txt" + $betaFile3 = Join-Path $betaDir "BetaFile3.txt" + $gammaDir = Join-Path $betaDir "sub-gamma" + $uponeLink = Join-Path $gammaDir "upone-link" + $uptwoLink = Join-Path $gammaDir "uptwo-link" + $omegaLink = Join-Path $gammaDir "omegaLink" + + New-Item -ItemType Directory -Path $alphaDir + New-Item -ItemType File -Path $alphaFile1 + New-Item -ItemType File -Path $alphaFile2 + New-Item -ItemType Directory -Path $betaDir + New-Item -ItemType File -Path $betaFile1 + New-Item -ItemType File -Path $betaFile2 + New-Item -ItemType File -Path $betaFile3 + New-Item -ItemType Directory $omegaDir + New-Item -ItemType File -Path $omegaFile1 + New-Item -ItemType File -Path $omegaFile2 + } + + AfterAll { + Remove-Item -Path $alphaLink -Force -ErrorAction SilentlyContinue + Remove-Item -Path $betaLink -Force -ErrorAction SilentlyContinue + } + + BeforeEach { + [System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestOn', $true) + [System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestRecurseOn', $false) + } + + AfterEach { + [System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestRecurseOn', $false) + [System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestOn', $false) + } + + It "Get-ChildItem will recurse into emulated OneDrive directory" -Skip:(-not $IsWindows) { + New-Item -ItemType SymbolicLink -Path $alphaLink -Value $alphaDir -Force + New-Item -ItemType SymbolicLink -Path $betaLink -Value $betaDir -Force + + # '$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 + } + + 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 + + # 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 + } +} From 55e2e4530466c0391f670d64ef2a1e347d923c49 Mon Sep 17 00:00:00 2001 From: Ilya Date: Sun, 13 Jun 2021 09:59:09 +0500 Subject: [PATCH 03/11] Assign RequireAdminOnWindows to new tests --- .../Microsoft.PowerShell.Management/FileSystem.Tests.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 index 4ad91d23e38..70a18ff604f 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 @@ -1546,7 +1546,7 @@ Describe "Windows admin tests" -Tag 'RequireAdminOnWindows' { } } -Describe "OneDrive filesystem manipulation" -Tags "CI" { +Describe "OneDrive filesystem manipulation" -Tags @('CI', 'RequireAdminOnWindows') { BeforeAll { # on macOS, the /tmp directory is a symlink, so we'll resolve it here $TestPath = $TestDrive From bab863bcac6250e8dafa0eb102ebbfb69d2df704 Mon Sep 17 00:00:00 2001 From: Ilya Date: Sun, 13 Jun 2021 10:38:39 +0500 Subject: [PATCH 04/11] Fix CodeFactor issue --- src/System.Management.Automation/utils/PathUtils.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.Management.Automation/utils/PathUtils.cs b/src/System.Management.Automation/utils/PathUtils.cs index 83a2703fa0d..cd7b99ecd9c 100644 --- a/src/System.Management.Automation/utils/PathUtils.cs +++ b/src/System.Management.Automation/utils/PathUtils.cs @@ -4,10 +4,10 @@ using System.Collections.Generic; using System.Globalization; using System.IO; +using System.Management.Automation.Internal; using System.Runtime.CompilerServices; using System.Text; -using System.Management.Automation.Internal; using Dbg = System.Management.Automation.Diagnostics; namespace System.Management.Automation From 3f061626e8debc3c740015dd5a58700454ca76bd Mon Sep 17 00:00:00 2001 From: Ilya Date: Tue, 27 Jul 2021 10:09:29 +0500 Subject: [PATCH 05/11] Fix rebase --- .../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 f04842f78fd..a0667af1d0d 100644 --- a/src/System.Management.Automation/namespaces/FileSystemProvider.cs +++ b/src/System.Management.Automation/namespaces/FileSystemProvider.cs @@ -2067,7 +2067,7 @@ public static string NameString(PSObject instance) { if (instance?.BaseObject is FileSystemInfo fileInfo) { - if (InternalSymbolicLinkLinkCodeMethods.IsReparsePointWithTarget(fileInfo)) + if (InternalSymbolicLinkLinkCodeMethods.IsReparsePointLikeSymlink(fileInfo)) { return $"{PSStyle.Instance.FileInfo.SymbolicLink}{fileInfo.Name}{PSStyle.Instance.Reset} -> {InternalSymbolicLinkLinkCodeMethods.GetTarget(instance)}"; } @@ -2095,7 +2095,7 @@ public static string NameString(PSObject instance) else { return instance?.BaseObject is FileSystemInfo fileInfo - ? InternalSymbolicLinkLinkCodeMethods.IsReparsePointWithTarget(fileInfo) + ? InternalSymbolicLinkLinkCodeMethods.IsReparsePointLikeSymlink(fileInfo) ? $"{fileInfo.Name} -> {InternalSymbolicLinkLinkCodeMethods.GetTarget(instance)}" : fileInfo.Name : string.Empty; From b28f9305fc7abc745eb8a480ba6e707bd37f2326 Mon Sep 17 00:00:00 2001 From: Ilya Date: Tue, 27 Jul 2021 11:04:10 +0500 Subject: [PATCH 06/11] Address Robert's feedbacks --- .../namespaces/FileSystemProvider.cs | 2 +- .../utils/PathUtils.cs | 24 +++++++++---------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/src/System.Management.Automation/namespaces/FileSystemProvider.cs b/src/System.Management.Automation/namespaces/FileSystemProvider.cs index a0667af1d0d..2c6e7fa679f 100644 --- a/src/System.Management.Automation/namespaces/FileSystemProvider.cs +++ b/src/System.Management.Automation/namespaces/FileSystemProvider.cs @@ -8280,7 +8280,7 @@ internal static bool IsReparsePointLikeSymlink(FileSystemInfo fileInfo) fullPath = PathUtils.EnsureExtendedPrefix(fullPath); } - using (var handle = FindFirstFileEx(fullPath, FINDEX_INFO_LEVELS.FindExInfoBasic, ref data, FINDEX_SEARCH_OPS.FindExSearchNameMatch, IntPtr.Zero, 0)) + using (SafeFindHandle handle = FindFirstFileEx(fullPath, FINDEX_INFO_LEVELS.FindExInfoBasic, ref data, FINDEX_SEARCH_OPS.FindExSearchNameMatch, IntPtr.Zero, 0)) { if (handle.IsInvalid) { diff --git a/src/System.Management.Automation/utils/PathUtils.cs b/src/System.Management.Automation/utils/PathUtils.cs index cd7b99ecd9c..723e68b3fc0 100644 --- a/src/System.Management.Automation/utils/PathUtils.cs +++ b/src/System.Management.Automation/utils/PathUtils.cs @@ -449,6 +449,18 @@ internal static bool TryDeleteFile(string filepath) return false; } + internal static string EnsureExtendedPrefix(string path) + { + if (IsPartiallyQualified(path) || IsDevice(path)) + return path; + + // Given \\server\share in longpath becomes \\?\UNC\server\share + if (path.StartsWith(UncPathPrefix, StringComparison.OrdinalIgnoreCase)) + return path.Insert(2, UncDevicePrefixToInsert); + + return ExtendedDevicePathPrefix + path; + } + #region Helpers for long paths from .Net RUmtime private const string ExtendedDevicePathPrefix = @"\\?\"; private const string UncPathPrefix = @"\\"; @@ -467,18 +479,6 @@ private static bool IsValidDriveChar(char value) return ((value >= 'A' && value <= 'Z') || (value >= 'a' && value <= 'z')); } - internal static string EnsureExtendedPrefix(string path) - { - if (IsPartiallyQualified(path) || IsDevice(path)) - return path; - - // Given \\server\share in longpath becomes \\?\UNC\server\share - if (path.StartsWith(UncPathPrefix, StringComparison.OrdinalIgnoreCase)) - return path.Insert(2, UncDevicePrefixToInsert); - - return ExtendedDevicePathPrefix + path; - } - private static bool IsDevice(string path) { return IsExtended(path) From 06ee55004b82587ab00dcdc63566f74e8ca16d08 Mon Sep 17 00:00:00 2001 From: Ilya Date: Wed, 28 Jul 2021 09:45:13 +0500 Subject: [PATCH 07/11] Add comments --- src/System.Management.Automation/engine/Utils.cs | 1 + src/System.Management.Automation/utils/PathUtils.cs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/System.Management.Automation/engine/Utils.cs b/src/System.Management.Automation/engine/Utils.cs index c09ae3f08fd..0f6acff99a1 100644 --- a/src/System.Management.Automation/engine/Utils.cs +++ b/src/System.Management.Automation/engine/Utils.cs @@ -2107,6 +2107,7 @@ public static class InternalTestHooks // 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. + // OneDriveTestSymlinkName defines the symlink name used in tests. internal static bool OneDriveTestOn; internal static bool OneDriveTestRecurseOn; internal static string OneDriveTestSymlinkName = "link-Beta"; diff --git a/src/System.Management.Automation/utils/PathUtils.cs b/src/System.Management.Automation/utils/PathUtils.cs index 723e68b3fc0..026f52ac2a6 100644 --- a/src/System.Management.Automation/utils/PathUtils.cs +++ b/src/System.Management.Automation/utils/PathUtils.cs @@ -449,6 +449,7 @@ internal static bool TryDeleteFile(string filepath) return false; } + #region Helpers for long paths from .Net Runtime internal static string EnsureExtendedPrefix(string path) { if (IsPartiallyQualified(path) || IsDevice(path)) @@ -461,7 +462,6 @@ internal static string EnsureExtendedPrefix(string path) return ExtendedDevicePathPrefix + path; } - #region Helpers for long paths from .Net RUmtime private const string ExtendedDevicePathPrefix = @"\\?\"; private const string UncPathPrefix = @"\\"; private const string UncDevicePrefixToInsert = @"?\UNC\"; From c2b6089e897f64aa07d42da333b76580e8db9a47 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Thu, 5 Aug 2021 10:28:24 -0700 Subject: [PATCH 08/11] Update src/System.Management.Automation/utils/PathUtils.cs --- src/System.Management.Automation/utils/PathUtils.cs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/System.Management.Automation/utils/PathUtils.cs b/src/System.Management.Automation/utils/PathUtils.cs index 026f52ac2a6..45ba787aee2 100644 --- a/src/System.Management.Automation/utils/PathUtils.cs +++ b/src/System.Management.Automation/utils/PathUtils.cs @@ -450,6 +450,11 @@ internal static bool TryDeleteFile(string filepath) } #region Helpers for long paths from .Net Runtime + + // Code here is copied from .NET's internal path helper implementation: + // https://github.com/dotnet/runtime/blob/dcce0f56e10f5ac9539354b049341a2d7c0cdebf/src/libraries/System.Private.CoreLib/src/System/IO/PathInternal.Windows.cs + // It has been left as a verbatim copy. + internal static string EnsureExtendedPrefix(string path) { if (IsPartiallyQualified(path) || IsDevice(path)) From bcdab46ccc4a49764f8e8e46ea1c2681c7accbfe Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Thu, 5 Aug 2021 10:29:05 -0700 Subject: [PATCH 09/11] Update src/System.Management.Automation/engine/Utils.cs Co-authored-by: Steve Lee --- src/System.Management.Automation/engine/Utils.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.Management.Automation/engine/Utils.cs b/src/System.Management.Automation/engine/Utils.cs index 0f6acff99a1..3486f0e72c2 100644 --- a/src/System.Management.Automation/engine/Utils.cs +++ b/src/System.Management.Automation/engine/Utils.cs @@ -2106,7 +2106,7 @@ public static class InternalTestHooks // 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. + // If OneDriveTestRecuseOn is true then we recurse into the symlink as OneDrive should work. // OneDriveTestSymlinkName defines the symlink name used in tests. internal static bool OneDriveTestOn; internal static bool OneDriveTestRecurseOn; From 016267e49cb7f60db2f7623159c711cd00517fb1 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Thu, 5 Aug 2021 10:37:26 -0700 Subject: [PATCH 10/11] Apply suggestions from code review Co-authored-by: Steve Lee --- src/System.Management.Automation/engine/Utils.cs | 4 ++-- .../namespaces/FileSystemProvider.cs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/System.Management.Automation/engine/Utils.cs b/src/System.Management.Automation/engine/Utils.cs index 3486f0e72c2..8c37713234f 100644 --- a/src/System.Management.Automation/engine/Utils.cs +++ b/src/System.Management.Automation/engine/Utils.cs @@ -2105,8 +2105,8 @@ 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 recurse into the symlink as OneDrive should work. + // If OneDriveTestRecurseOn is false then the symlink works as regular symlink. + // If OneDriveTestRecurseOn is true then we recurse into the symlink as OneDrive should work. // OneDriveTestSymlinkName defines the symlink name used in tests. internal static bool OneDriveTestOn; internal static bool OneDriveTestRecurseOn; diff --git a/src/System.Management.Automation/namespaces/FileSystemProvider.cs b/src/System.Management.Automation/namespaces/FileSystemProvider.cs index 2c6e7fa679f..3085d1a44a8 100644 --- a/src/System.Management.Automation/namespaces/FileSystemProvider.cs +++ b/src/System.Management.Automation/namespaces/FileSystemProvider.cs @@ -1898,7 +1898,7 @@ private void Dir( // 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; + checkReparsePoint = recursiveDirectory.Attributes.HasFlag(FileAttributes.ReparsePoint); } // if "Hidden" is explicitly specified anywhere in the attribute filter, then override From 488ceb01fc2e927a9a95f217f8f23c946d174dff Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Thu, 5 Aug 2021 11:02:06 -0700 Subject: [PATCH 11/11] Update src/System.Management.Automation/namespaces/FileSystemProvider.cs --- .../namespaces/FileSystemProvider.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/System.Management.Automation/namespaces/FileSystemProvider.cs b/src/System.Management.Automation/namespaces/FileSystemProvider.cs index 3085d1a44a8..2eb0de56d52 100644 --- a/src/System.Management.Automation/namespaces/FileSystemProvider.cs +++ b/src/System.Management.Automation/namespaces/FileSystemProvider.cs @@ -8070,7 +8070,6 @@ protected override bool ReleaseHandle() private static extern bool FindClose(IntPtr handle); } - // SetLastError is false as the use of this API doesn't not require GetLastError() to be called [DllImport(PinvokeDllNames.FindFirstFileDllName, EntryPoint = "FindFirstFileExW", SetLastError = true, CharSet = CharSet.Unicode)] private static extern SafeFindHandle FindFirstFileEx(string lpFileName, FINDEX_INFO_LEVELS fInfoLevelId, ref WIN32_FIND_DATA lpFindFileData, FINDEX_SEARCH_OPS fSearchOp, IntPtr lpSearchFilter, int dwAdditionalFlags);