From d3addeb3daa604beda5c05689539ee147c07d4e5 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Mon, 6 Jul 2020 16:39:08 -0700 Subject: [PATCH] Fix `Move-Item` to support cross-mount moves on Unix (#13044) --- .../engine/Utils.cs | 2 + .../namespaces/FileSystemProvider.cs | 70 ++++++++++++++----- .../FileSystem.Tests.ps1 | 24 +++++++ 3 files changed, 78 insertions(+), 18 deletions(-) diff --git a/src/System.Management.Automation/engine/Utils.cs b/src/System.Management.Automation/engine/Utils.cs index 12374148fa..05b8800b4e 100644 --- a/src/System.Management.Automation/engine/Utils.cs +++ b/src/System.Management.Automation/engine/Utils.cs @@ -2080,6 +2080,8 @@ namespace System.Management.Automation.Internal internal static bool ShowMarkdownOutputBypass; + internal static bool ThrowExdevErrorOnMoveDirectory; + /// 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 9c454ce68f..a24b48c0ef 100644 --- a/src/System.Management.Automation/namespaces/FileSystemProvider.cs +++ b/src/System.Management.Automation/namespaces/FileSystemProvider.cs @@ -51,6 +51,12 @@ namespace Microsoft.PowerShell.Commands ISecurityDescriptorCmdletProvider, ICmdletProviderSupportsHelp { +#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; +#endif + // 4MB gives the best results without spiking the resources on the remote connection for file transfers between pssessions. // NOTE: The script used to copy file data from session (PSCopyFromSessionHelper) has a // maximum fragment size value for security. If FILETRANSFERSIZE changes make sure the @@ -6001,15 +6007,7 @@ namespace Microsoft.PowerShell.Commands try { - if (!IsSameVolume(directory.FullName, destination)) - { - CopyAndDelete(directory, destination, force); - } - else - { - // Move the file - directory.MoveTo(destination); - } + MoveDirectoryInfoUnchecked(directory, destination, force); WriteItemObject( directory, @@ -6027,14 +6025,7 @@ namespace Microsoft.PowerShell.Commands directory.Attributes = directory.Attributes & ~(FileAttributes.ReadOnly | FileAttributes.Hidden); - if (!IsSameVolume(directory.FullName, destination)) - { - CopyAndDelete(directory, destination, force); - } - else - { - directory.MoveTo(destination); - } + MoveDirectoryInfoUnchecked(directory, destination, force); WriteItemObject(directory, directory.FullName, true); } @@ -6072,6 +6063,47 @@ namespace Microsoft.PowerShell.Commands } } + /// + /// Implements the file move operation for directories without handling any error scenarios. + /// In particular, this attempts to rename or copy+delete the file, + /// but passes any exceptional behavior through to the caller. + /// + /// The directory to move. + /// The destination path to move the directory to. + /// If true, force move the directory, overwriting anything at the destination. + 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); + } + + directory.MoveTo(destinationPath); + } + catch (IOException e) when (e.HResult == UNIX_ERRNO_EXDEV) + { + // 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) { if (!ItemExists(destination)) @@ -6107,13 +6139,15 @@ namespace Microsoft.PowerShell.Commands } } - private bool IsSameVolume(string source, string destination) +#if !UNIX + private 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 diff --git a/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 index e38bf0c617..015ad53ccd 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 @@ -131,12 +131,36 @@ Describe "Basic FileSystem Provider Tests" -Tags "CI" { "$destDir/$testDir/$testFile" | Should -Exist } + It "Verify Move-Item across devices for directory" { + [System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('ThrowExdevErrorOnMoveDirectory', $true) + try + { + $dir = (New-Item -Path TestDrive:/dir -ItemType Directory -ErrorAction Stop).FullName + $file = (New-Item -Path "$dir/file.txt" -Value "HELLO" -ErrorAction Stop).Name + $destination = "$TestDrive/destination" + + Move-Item -Path $dir -Destination $destination -ErrorAction Stop + + $dir | Should -Not -Exist + $destination | Should -Exist + "$destination/$file" | Should -Exist + } + finally + { + [System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('ThrowExdevErrorOnMoveDirectory', $false) + } + } + It "Verify Move-Item will not move to an existing file" { { Move-Item -Path $testDir -Destination $testFile -ErrorAction Stop } | Should -Throw -ErrorId "MoveDirectoryItemIOError,Microsoft.PowerShell.Commands.MoveItemCommand" $error[0].Exception | Should -BeOfType System.IO.IOException $testDir | Should -Exist } + It "Verify Move-Item throws correct error for non-existent source" { + { Move-Item -Path /does/not/exist -Destination $testFile -ErrorAction Stop } | Should -Throw -ErrorId 'PathNotFound,Microsoft.PowerShell.Commands.MoveItemCommand' + } + It "Verify Move-Item as substitute for Rename-Item" { $newFile = Move-Item -Path $testFile -Destination $newTestFile -PassThru $fileExists = Test-Path $newTestFile