8000 Fix longpath directory removal by rjmholt · Pull Request #15467 · PowerShell/PowerShell · GitHub
[go: up one dir, main page]
Skip to content

Fix longpath directory removal #15467

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed

Fix longpath directory removal #15467

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a1c2b1f
Ensure long paths are formatted correctly for Windows
rjmholt May 26, 2021
e6025d1
Ensure non-existent files error correctly
rjmholt May 26, 2021
12cbbe3
Rearrange testing to improve sensitivity to OneDrive issues
rjmholt May 26, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/Microsoft.PowerShell.Commands.Management/commands/management/Navigation.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2698,8 +2698,9 @@ protected override void ProcessRecord()
{
try
{
System.IO.DirectoryInfo di = new(providerPath);
if (di != null && InternalSymbolicLinkLinkCodeMethods.IsReparsePointLikeSymlink(di))
var directoryInfo = new System.IO.DirectoryInfo(providerPath);
if (directoryInfo.Exists
&& InternalSymbolicLinkLinkCodeMethods.IsReparsePointLikeSymlink(directoryInfo))
Comment on lines +2701 to +2703
Copy link
Collaborator

Choose a reason for hiding this comment

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

Here di is never null and we can remove the null check but we also have no need to do extra Exist check because providerPath is based on resolvedPath.

{
shouldRecurse = false;
treatAsFile = true;
Expand Down
6 changes: 6 additions & 0 deletions src/System.Management.Automation/namespaces/FileSystemProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8243,6 +8243,12 @@ internal static bool IsReparsePointLikeSymlink(FileSystemInfo fileInfo)

WIN32_FIND_DATA data = default;
string fullPath = Path.TrimEndingDirectorySeparator(fileInfo.FullName);

if (fullPath.Length > MAX_PATH)
{
fullPath = "\\\\?\\" + fullPath;
Copy link
Collaborator

Choose a reason for hiding this comment

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

There are 2 things we need to check here before we can add the prefix.

  • The prefix isn't already present - I'm not sure if FileSystemInfo.FullName preserves the \\?\ if it was first created with that prefix
  • When dealing with a UNC path we need to do \\?\UNC\{server}\{share}, essentially replace the first \\ with \\?\UNC

I'm also not sure if pwsh/.NET is meant to support the \\.\ DosDevice prefix as well as \\?\, if so then it also needs to check for that prefix.

Finally I think that the prefix should always be added rather than when the path exceeds MAX_PATH. This ensures the code always runs the same no matter what path is being used and any future bugs that might occur occur in all situations (thus gets picked up more easily/in betas) rather than just in the edge cases.

}

using (var handle = FindFirstFileEx(fullPath, FINDEX_INFO_LEVELS.FindExInfoBasic, ref data, FINDEX_SEARCH_OPS.FindExSearchNameMatch, IntPtr.Zero, 0))
{
if (handle.IsInvalid)
Expand Down
176 changes: 116 additions & 60 deletions test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -623,31 +623,6 @@ Describe "Hard link and symbolic link tests" -Tags "CI", "RequireAdminOnWindows"
$ci = Get-ChildItem $alphaLink -Recurse -Name
$ci.Count | Should -BeExactly 7 # returns 10 - unexpectly recurce in link-alpha\link-Beta. See https://github.com/PowerShell/PowerShell/issues/11614
}
It "Get-ChildItem will recurse into emulated OneDrive directory" -Skip:(-not $IsWindows) {
# The test depends on the files created in previous test:
#New-Item -ItemType SymbolicLink -Path $alphaLink -Value $alphaDir
#New-Item -ItemType SymbolicLink -Path $betaLink -Value $betaDir

[System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestOn', $true)
[System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestRecurseOn', $false)
try
{
# '$betaDir' is a symlink - we don't follow symlinks
# This emulates PowerShell 6.2 and below behavior.
$ci = Get-ChildItem -Path $alphaDir -Recurse
$ci.Count | Should -BeExactly 7

# Now we follow the symlink like on OneDrive.
[System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestRecurseOn', $true)
$ci = Get-ChildItem -Path $alphaDir -Recurse
$ci.Count | Should -BeExactly 10
}
finally
{
[System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestRecurseOn', $false)
[System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestOn', $false)
}
}
It "Get-ChildItem will recurse into symlinks given -FollowSymlink, avoiding link loops" {
New-Item -ItemType Directory -Path $gammaDir
New-Item -ItemType SymbolicLink -Path $uponeLink -Value $betaDir
Expand Down Expand Up @@ -769,41 +744,6 @@ Describe "Hard link and symbolic link tests" -Tags "CI", "RequireAdminOnWindows"
$childB.Count | Should -BeExactly $childA.Count
$childB.Name | Should -BeExactly $childA.Name
}
It "Remove-Item will recurse into emulated OneDrive directory" -Skip:(-not $IsWindows) {
$alphaDir = Join-Path $TestDrive "sub-alpha2"
$alphaLink = Join-Path $TestDrive "link-alpha2"
$alphaFile1 = Join-Path $alphaDir "AlphaFile1.txt"
$betaDir = Join-Path $alphaDir "sub-Beta"
$betaLink = Join-Path $alphaDir "link-Beta"
$betaFile1 = Join-Path $betaDir "BetaFile1.txt"

New-Item -ItemType Directory -Path $alphaDir > $null
New-Item -ItemType File -Path $alphaFile1 > $null
New-Item -ItemType Directory -Path $betaDir > $null
New-Item -ItemType File -Path $betaFile1 > $null

New-Item -ItemType SymbolicLink -Path $alphaLink -Value $alphaDir > $null
New-Item -ItemType SymbolicLink -Path $betaLink -Value $betaDir > $null

[System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestOn', $true)
[System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestRecurseOn', $false)
try
{
# With the test hook turned on we don't remove '$betaDir' symlink.
# This emulates PowerShell 7.1 and below behavior.
{ Remove-Item -Path $betaLink -Recurse -ErrorAction Stop } | Should -Throw -ErrorId "DeleteSymbolicLinkFailed,Microsoft.PowerShell.Commands.RemoveItemCommand"

# Now we emulate OneDrive and follow the symlink like on OneDrive.
[System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestRecurseOn', $true)
Remove-Item -Path $betaLink -Recurse
Test-Path -Path $betaLink | Should -BeFalse
}
finally
{
[System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestRecurseOn', $false)
[System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('OneDriveTestOn', $false)
}
}

}
}
Expand Down Expand Up @@ -1590,3 +1530,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
}
}
0