Skip to content

Fix Move-Item for FileSystemProvider to use copy-delete instead of move for DFS paths #14913

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

Merged
anmenaga merged 8 commits into from
Mar 8, 2021
Merged

Fix Move-Item for FileSystemProvider to use copy-delete instead of move for DFS paths #14913

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
543c73d
Fix `Move-Item` for FileSystemProvider to use copy-delete instead of …
SteveL-MSFT Feb 27, 2021
2513c2f
Change fix to use CopyAndDelete only if AccessDenied error comes up
SteveL-MSFT Mar 1, 2021
18d2472
remove unncessary check for Windows if move is to same volume
SteveL-MSFT Mar 2, 2021
27fb53a
change to try copydelete on any IOException as different HResult can …
SteveL-MSFT Mar 3, 2021
4ea4112
address Ilya's feedback by not falling back on all IOException types
SteveL-MSFT Mar 3, 2021
3de7bd2
update tests
SteveL-MSFT Mar 3, 2021
5c804aa
Unified Unix and Windows paths, check HRESULT for 0x80070005 AccessDe…
SteveL-MSFT Mar 4, 2021
4723a93
Fix test
SteveL-MSFT Mar 4, 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
32 changes: 6 additions & 26 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 @@ -54,7 +54,10 @@ public sealed partial class FileSystemProvider : NavigationCmdletProvider,
#if UNIX
// This is the errno returned by the rename() syscall
// when an item is attempted to be renamed across filesystem mount boundaries.
private const int UNIX_ERRNO_EXDEV = 18;
private const int MOVE_FAILED_ERROR = 18;
#else
// 0x80070005 ACCESS_DENIED is returned when trying to move files across volumes like DFS
private const int MOVE_FAILED_ERROR = -2147024891;
#endif

// 4MB gives the best results without spiking the resources on the remote connection for file transfers between pssessions.
Expand Down Expand Up @@ -6071,35 +6074,22 @@ private void MoveDirectoryInfoItem(
/// <param name="force">If true, force move the directory, overwriting anything at the destination.</param>
private void MoveDirectoryInfoUnchecked(DirectoryInfo directory, string destinationPath, bool force)
{
#if UNIX
try
{
if (InternalTestHooks.ThrowExdevErrorOnMoveDirectory)
{
throw new IOException("Invalid cross-device link", hresult: UNIX_ERRNO_EXDEV);
throw new IOException("Invalid cross-device link", hresult: MOVE_FAILED_ERROR);
}

directory.MoveTo(destinationPath);
}
catch (IOException e) when (e.HResult == UNIX_ERRNO_EXDEV)
catch (IOException e) when (e.HResult == MOVE_FAILED_ERROR)
{
// Rather than try to ascertain whether we can rename a directory ahead of time,
// it's both faster and more correct to try to rename it and fall back to copy/deleting it
// See also: https://github.com/coreutils/coreutils/blob/439741053256618eb651e6d43919df29625b8714/src/mv.c#L212-L216
CopyAndDelete(directory, destinationPath, force);
}
#else
// On Windows, being able to rename vs copy/delete a file
// is just a question of the drive
if (IsSameWindowsVolume(directory.FullName, destinationPath))
{
directory.MoveTo(destinationPath);
}
else
{
CopyAndDelete(directory, destinationPath, force);
}
#endif
}

private void CopyAndDelete(DirectoryInfo directory, string destination, bool force)
Expand Down Expand Up @@ -6137,16 +6127,6 @@ private void CopyAndDelete(DirectoryInfo directory, string destination, bool for
}
}

#if !UNIX
private static bool IsSameWindowsVolume(string source, string destination)
{
FileInfo src = new FileInfo(source);
FileInfo dest = new FileInfo(destination);

return (src.Directory.Root.Name == dest.Directory.Root.Name);
}
#endif

#endregion MoveItem

#endregion NavigationCmdletProvider members
Expand Down
9 changes: 8 additions & 1 deletion 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 @@ -287,6 +287,13 @@ Describe "Basic FileSystem Provider Tests" -Tags "CI" {
$newItemPath = Join-Path $protectedPath "foo"
$shouldSkip = -not (Test-Path $protectedPath)
}

if ($IsWindows) {
$fqaccessdenied = "MoveDirectoryItemUnauthorizedAccessError,Microsoft.PowerShell.Commands.MoveItemCommand"
}
else {
$fqaccessdenied = "MoveDirectoryItemIOError,Microsoft.PowerShell.Commands.MoveItemCommand"
}
}

It "Access-denied test for <cmdline>" -Skip:(-not $IsWindows -or $shouldSkip) -TestCases @(
Expand All @@ -296,7 +303,7 @@ Describe "Basic FileSystem Provider Tests" -Tags "CI" {
# @{cmdline = "New-Item -Type File -Path $newItemPath -ErrorAction Stop"; expectedError = "NewItemUnauthorizedAccessError,Microsoft.PowerShell.Commands.NewItemCommand"}
@{cmdline = "Get-ChildItem $protectedPath -ErrorAction Stop"; expectedError = "DirUnauthorizedAccessError,Microsoft.PowerShell.Commands.GetChildItemCommand"}
@{cmdline = "Rename-Item -Path $protectedPath -NewName bar -ErrorAction Stop"; expectedError = "RenameItemIOError,Microsoft.PowerShell.Commands.RenameItemCommand"},
@{cmdline = "Move-Item -Path $protectedPath -Destination bar -ErrorAction Stop"; expectedError = "MoveDirectoryItemIOError,Microsoft.PowerShell.Commands.MoveItemCommand"}
@{cmdline = "Move-Item -Path $protectedPath -Destination bar -ErrorAction Stop"; expectedError = $fqaccessdenied}
) {
param ($cmdline, $expectedError)

Expand Down