From d9708093ce0cc83d5593e3061fa18d856e21c7cf Mon Sep 17 00:00:00 2001 From: Jeff Bienstadt Date: Sun, 26 Mar 2017 18:24:33 -0700 Subject: [PATCH 1/7] Replace string-compare-based test for copying to same file with more accurate, cross-platform test (#1930) Changes per code review * Cleaned up comments in .cpp file * Renamed IsSameFile to IsSameFileSystemItem * Removed comment about PosixSemantics * Added PosixSemantics to CreateFile call * Provide additional information in exception when paths point to same destination. * Added tests --- .../CoreCLR/CorePsPlatform.cs | 12 +- .../namespaces/FileSystemProvider.cs | 51 ++++- src/libpsl-native/src/CMakeLists.txt | 1 + .../src/issamefilesystemitem.cpp | 48 +++++ src/libpsl-native/src/issamefilesystemitem.h | 11 ++ .../FileSystem.Tests.ps1 | 176 +++++++++++++++++- 6 files changed, 291 insertions(+), 8 deletions(-) create mode 100644 src/libpsl-native/src/issamefilesystemitem.cpp create mode 100644 src/libpsl-native/src/issamefilesystemitem.h diff --git a/src/System.Management.Automation/CoreCLR/CorePsPlatform.cs b/src/System.Management.Automation/CoreCLR/CorePsPlatform.cs index d3f8cead66f..7a5f93cc94d 100644 --- a/src/System.Management.Automation/CoreCLR/CorePsPlatform.cs +++ b/src/System.Management.Automation/CoreCLR/CorePsPlatform.cs @@ -515,6 +515,11 @@ internal static bool NonWindowsIsDirectory(string path) return Unix.NativeMethods.IsDirectory(path); } + internal static bool NonWindowsIsSameFileSystemItem(string pathOne, string pathTwo) + { + return Unix.NativeMethods.IsSameFileSystemItem(pathOne, pathTwo); + } + internal static bool NonWindowsIsExecutable(string path) { return Unix.NativeMethods.IsExecutable(path); @@ -530,7 +535,7 @@ internal static int NonWindowsGetProcessParentPid(int pid) return IsOSX ? Unix.NativeMethods.GetPPid(pid) : Unix.GetProcFSParentPid(pid); } - + // Unix specific implementations of required functionality // @@ -722,6 +727,11 @@ internal static extern int CreateHardLink([MarshalAs(UnmanagedType.LPStr)]string [DllImport(psLib, CharSet = CharSet.Ansi, SetLastError = true)] [return: MarshalAs(UnmanagedType.I1)] internal static extern bool IsDirectory([MarshalAs(UnmanagedType.LPStr)]string filePath); + + [DllImport(psLib, CharSet = CharSet.Ansi, SetLastError = true)] + [return: MarshalAs(UnmanagedType.I1)] + internal static extern bool IsSameFileSystemItem([MarshalAs(UnmanagedType.LPStr)]string filePathOne, + [MarshalAs(UnmanagedType.LPStr)]string filePathTwo); } } } diff --git a/src/System.Management.Automation/namespaces/FileSystemProvider.cs b/src/System.Management.Automation/namespaces/FileSystemProvider.cs index 213a7e66ac7..6008edeb9b5 100644 --- a/src/System.Management.Automation/namespaces/FileSystemProvider.cs +++ b/src/System.Management.Automation/namespaces/FileSystemProvider.cs @@ -62,6 +62,11 @@ public sealed partial class FileSystemProvider : NavigationCmdletProvider, // copy script will accomodate the new value. private const int FILETRANSFERSIZE = 4 * 1024 * 1024; + // The name of the key in an exception's Data dictionary when attempting + // to copy an item onto itself. + private const string SelfCopyDataKey = "SelfCopy"; + + /// /// An instance of the PSTraceSource class used for trace output /// using "FileSystemProvider" as the category. @@ -3460,14 +3465,14 @@ protected override void CopyItem( _excludeMatcher = SessionStateUtilities.CreateWildcardsFromStrings(Exclude, WildcardOptions.IgnoreCase); // if the source and destination path are same (for a local copy) then flag it as error. - if ((toSession == null) && (fromSession == null) && path.Equals(destinationPath, StringComparison.OrdinalIgnoreCase)) + if ((toSession == null) && (fromSession == null) && InternalSymbolicLinkLinkCodeMethods.IsSameFileSystemItem(path, destinationPath)) { String error = StringUtil.Format(FileSystemProviderStrings.CopyError, path); Exception e = new IOException(error); + e.Data[SelfCopyDataKey] = destinationPath; WriteError(new ErrorRecord(e, "CopyError", ErrorCategory.WriteError, path)); return; } - // Copy-Item from session if (fromSession != null) { @@ -3777,14 +3782,14 @@ private void CopyFileInfoItem(FileInfo file, string destinationPath, bool force, } //if the source and destination path are same then flag it as error. - if (destinationPath.Equals(file.FullName, StringComparison.OrdinalIgnoreCase)) + if (InternalSymbolicLinkLinkCodeMethods.IsSameFileSystemItem(destinationPath, file.FullName)) { String error = StringUtil.Format(FileSystemProviderStrings.CopyError, destinationPath); Exception e = new IOException(error); + e.Data[SelfCopyDataKey] = file.FullName; WriteError(new ErrorRecord(e, "CopyError", ErrorCategory.WriteError, destinationPath)); return; } - // Verify that the target doesn't represent a device name if (PathIsReservedDeviceName(destinationPath, "CopyError")) { @@ -8211,6 +8216,42 @@ internal static bool WinIsHardLink(FileSystemInfo fileInfo) return isHardLink; } + internal static bool IsSameFileSystemItem(string pathOne, string pathTwo) + { +#if UNIX + return Platform.NonWindowsIsSameFileSystemItem(pathOne, pathTwo); +#else + return WinIsSameFileSystemItem(pathOne, pathTwo); +#endif + } + + internal static bool WinIsSameFileSystemItem(string pathOne, string pathTwo) + { + var access = FileAccess.Read; + var share = FileShare.Read; + var creation = FileMode.Open; + var attributes = FileAttributes.BackupSemantics | FileAttributes.PosixSemantics; + + using (var sfOne = AlternateDataStreamUtilities.NativeMethods.CreateFile(pathOne, access, share, IntPtr.Zero, creation, (int)attributes, IntPtr.Zero)) + using (var sfTwo = AlternateDataStreamUtilities.NativeMethods.CreateFile(pathTwo, access, share, IntPtr.Zero, creation, (int)attributes, IntPtr.Zero)) + { + if (!sfOne.IsInvalid && !sfTwo.IsInvalid) + { + BY_HANDLE_FILE_INFORMATION infoOne; + BY_HANDLE_FILE_INFORMATION infoTwo; + if ( GetFileInformationByHandle(sfOne.DangerousGetHandle(), out infoOne) + && GetFileInformationByHandle(sfTwo.DangerousGetHandle(), out infoTwo)) + { + return infoOne.VolumeSerialNumber == infoTwo.VolumeSerialNumber + && infoOne.FileIndexHigh == infoTwo.FileIndexHigh + && infoOne.FileIndexLow == infoTwo.FileIndexLow; + } + } + } + + return false; + } + internal static bool IsHardLink(ref IntPtr handle) { #if UNIX @@ -8664,7 +8705,7 @@ internal static void SetZoneOfOrigin(string path, SecurityZone securityZone) // the code above seems cleaner and more robust than the IAttachmentExecute approach } - private static class NativeMethods + internal static class NativeMethods { internal const int ERROR_HANDLE_EOF = 38; internal enum StreamInfoLevels { FindStreamInfoStandard = 0 } diff --git a/src/libpsl-native/src/CMakeLists.txt b/src/libpsl-native/src/CMakeLists.txt index 6cb86d0169d..720bc65f0fe 100644 --- a/src/libpsl-native/src/CMakeLists.txt +++ b/src/libpsl-native/src/CMakeLists.txt @@ -13,6 +13,7 @@ add_library(psl-native SHARED geterrorcategory.cpp isfile.cpp isdirectory.cpp + issamefilesystemitem.cpp issymlink.cpp isexecutable.cpp setdate.cpp diff --git a/src/libpsl-native/src/issamefilesystemitem.cpp b/src/libpsl-native/src/issamefilesystemitem.cpp new file mode 100644 index 00000000000..ba0f8390397 --- /dev/null +++ b/src/libpsl-native/src/issamefilesystemitem.cpp @@ -0,0 +1,48 @@ +//! @file issamefilesystemitem.cpp +//! @author Jeff Bienstadt +//! @brief returns if two paths ultimately point to the same filesystem object + +#include "getstat.h" +#include "issamefilesystemitem.h" + +#include +#include +#include +#include + +//! @brief returns if two paths ultimately refer to the same file or directory. +//! +//! IsSameFileSystemItem +//! +//! @param[in] path_one +//! @parblock +//! A pointer to the buffer that contains the first path. +//! +//! char* is marshaled as an LPStr, which on Linux is UTF-8. +//! @endparblock +//! +//! @param[in] path_two +//! @parblock +//! A pointer to the buffer that contains the second path. +//! +//! char* is marshaled as an LPStr, which on Linux is UTF-8. +//! @endparblock +//! +//! @retval true if both paths point to the same filesystem object, +//! false otherwise +//! +bool IsSameFileSystemItem(const char* path_one, const char* path_two) +{ + assert(path_one); + assert(path_two); + + struct stat buf_1; + struct stat buf_2; + + if (GetStat(path_one, &buf_1) == 0 && GetStat(path_two, &buf_2) == 0) + { + return buf_1.st_dev == buf_2.st_dev && buf_1.st_ino == buf_2.st_ino; + } + + return false; +} diff --git a/src/libpsl-native/src/issamefilesystemitem.h b/src/libpsl-native/src/issamefilesystemitem.h new file mode 100644 index 00000000000..7c3e3ade860 --- /dev/null +++ b/src/libpsl-native/src/issamefilesystemitem.h @@ -0,0 +1,11 @@ +#pragma once + +#include "pal.h" + +#include + +PAL_BEGIN_EXTERNC + +bool IsSameFileSystemItem(const char* path_one, const char* path_two); + +PAL_END_EXTERNC diff --git a/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 index 748fb18313b..543c24f7f87 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 @@ -246,6 +246,178 @@ Describe "Basic FileSystem Provider Tests" -Tags "CI" { } } +Describe "Copy-Item can avoid copying an item onto itself" -Tags "CI" { + BeforeAll { + # In Windows, only Administrators can create symbolic links. + # In Unix, anyone can. + function CanMakeSymlink + { + if ($IsWindows) + { + $winId = [System.Security.Principal.WindowsIdentity]::GetCurrent() + $principal = New-Object System.Security.Principal.WindowsPrincipal($WinId) + $admin = [System.Security.Principal.WindowsBuiltInRole]::Administrator + + return $principal.IsInRole($admin) + } + else + { + return $true + } + } + + # For now, we'll assume the tests are running the platform's + # native filesystem, in its default mode + $isCaseSensitive = $IsLinux + $canSymlink = CanMakeSymlink + $canDirSymLink = $IsWindows -And (CanMakeSymlink) + $canJunction = $IsWindows + + # The name of the key in an exception's Data dictionary when an + # attempt is made to copy an item onto itself. + $selfCopyKey = "SelfCopy" + + # Names of files, directories, and links we'll be using to test + $subDir = "$TestDrive/sub" + $otherSubDir = "$TestDrive/other-sub" + $otherFile = "$otherSubDir/other-file" + $otherFileName = "other-file" + $otherFile = "$otherSubDir/$otherFileName" + $symToOther = "$subDir/sym-to-other" + $secondSymToOther = "$subDir/another-sym-to-other" + $symToSym = "$subDir/sym-to-sym-to-other" + $symToOtherFile = "$subDir/sym-to-other-file" + $hardToOtherFile = "$subDir/hard-to-other-file" + $symdToOther = "$subDir/symd-to-other" + $junctionToOther = "$subDir/junction-to-other" + + # Set up our directories and links + New-Item -ItemType Directory $subDir >$null + New-Item -ItemType Directory $otherSubDir >$null + New-Item -ItemType File $otherFile -Value "some text" >$null + if ($canSymlink) + { + New-Item -ItemType SymbolicLink $symToOther -Value $otherSubDir >$null + New-Item -ItemType SymbolicLink $secondSymToOther -Value $otherSubDir >$null + New-Item -ItemType SymbolicLink $symToSym -Value $symToOther >$null + New-Item -ItemType SymbolicLink $symToOtherFile -Value $otherFile >$null + } + New-Item -ItemType HardLink $hardToOtherFile -Value $otherFile >$null + + if ($canJunction) + { + New-Item -ItemType Junction $junctionToOther -Value $otherSubDir >$null + } + if ($canDirSymLink) + { + New-Item -ItemType SymbolicLink $symdToOther -Value $otherSubDir >$null + } + + # Test the ability to avoid an item copying onto itself + function TestSelfCopy($testCase) + { + It "$($testCase.Name)" -Skip:($testCase.SkipIf) { + try + { + Copy-Item -Path $testCase.Source -Destination $testCase.Destination -ErrorAction Stop + + if ($testCase.SelfCopyExpected) + { + # we expected the copy to fail and it did not + throw "Copy-Item should not have succeeded!" + } + else + { + # we expected the copy to succeed. make sure it did + Test-Path $testCase.Destination | Should Be $true + Remove-Item -Path $testCase.Destination -Force -ErrorAction SilentlyContinue + } + } + catch + { + # we expected the copy to fail and it did. make sure it failed the right way + $_.FullyQualifiedErrorId | Should Be "CopyError,Microsoft.PowerShell.Commands.CopyItemCommand" + $_.Exception | Should BeOfType System.IO.IOException + $_.Exception.Data[$selfCopyKey] | Should Not Be $null + } + } + } + + # Set up our test cases + $testCases = @( + @{ + Name = "Copy to same path" + Source = $otherFile + Destination = $otherFile + SelfCopyExpected = $true + } + @{ + Name = "Copy to similar path, different case" + Source = $otherFile + Destination = "$otherSubDir/" + $otherFileName.ToUpper() + SelfCopyExpected = -Not $IsCaseSensitive + } + @{ + Name = "Copy hard link" + Source = $hardToOtherFile + Destination = $otherFile + SelfCopyExpected = $true + } + @{ + Name = "Copy hard link, reversed" + Source = $otherFile + Destination = $hardToOtherFile + SelfCopyExpected = $true + } + + # Only do the symbolic link tests if we were priveliged to create the links + @{ + Name = "Copy symbolic link to target" + Source = $symToOtherFile + Destination = $otherFile + SelfCopyExpected = $true + SkipIf = -Not $canSymlink + } + @{ + Name = "Copy symbolic link to symbolic link with same target" + Source = $secondSymToOther + Destination = $symToOther + SelfCopyExpected = $true + SkipIf = -Not $canSymlink + } + @{ + Name = "Copy through chain of symbolic links" + Source = $symToSym + Destination = $otherSubDir + SelfCopyExpected = $true + SkipIf = -Not $canSymlink + } + + # Junctions and directory symbolic links are Windows and NTFS only + @{ + Name = "Copy junction to target" + Source = $junctionToOther + Destination = $otherSubDir + SelfCopyExpected = $true + SkipIf = -Not $canJunction + } + @{ + Name = "Copy directory symbolic link to target" + Source = $symdToOther + Destination = $otherSubDir + SelfCopyExpected = $true + SkipIf = -Not $canDirSymLink + } + ) + } + + # run each of our tests + foreach ($case in $testCases) + { + TestSelfCopy($case) + } +} + Describe "Extended FileSystem Item/Content Cmdlet Provider Tests" -Tags "Feature" { BeforeAll { $testDir = "testDir" @@ -629,14 +801,14 @@ Describe "Extended FileSystem Path/Location Cmdlet Provider Tests" -Tags "Featur $result = Split-Path -Path $level1_0Full -Leaf $result | Should Be $level1_0 } - + It 'Validate LeafBase' { $result = Split-Path -Path "$level2_1Full$fileExt" -LeafBase $result | Should Be $level2_1 } It 'Validate LeafBase is not over-zealous' { - + $result = Split-Path -Path "$level2_1Full$fileExt$fileExt" -LeafBase $result | Should Be "$level2_1$fileExt" } From 3263893d54f69ea0edf227f41efdd7324ef2c424 Mon Sep 17 00:00:00 2001 From: Jeff Bienstadt Date: Tue, 18 Apr 2017 14:29:05 -0700 Subject: [PATCH 2/7] Update tests per code review --- .../FileSystem.Tests.ps1 | 210 +++++++++--------- 1 file changed, 107 insertions(+), 103 deletions(-) diff --git a/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 index 543c24f7f87..4d95ae93795 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 @@ -246,7 +246,8 @@ Describe "Basic FileSystem Provider Tests" -Tags "CI" { } } -Describe "Copy-Item can avoid copying an item onto itself" -Tags "CI" { + +Describe "Copy-Item can avoid copying an item onto itself" -Tags "CI", "RequireAdminOnWindows" { BeforeAll { # In Windows, only Administrators can create symbolic links. # In Unix, anyone can. @@ -254,11 +255,7 @@ Describe "Copy-Item can avoid copying an item onto itself" -Tags "CI" { { if ($IsWindows) { - $winId = [System.Security.Principal.WindowsIdentity]::GetCurrent() - $principal = New-Object System.Security.Principal.WindowsPrincipal($WinId) - $admin = [System.Security.Principal.WindowsBuiltInRole]::Administrator - - return $principal.IsInRole($admin) + return Test-IsElevated } else { @@ -280,7 +277,8 @@ Describe "Copy-Item can avoid copying an item onto itself" -Tags "CI" { # Names of files, directories, and links we'll be using to test $subDir = "$TestDrive/sub" $otherSubDir = "$TestDrive/other-sub" - $otherFile = "$otherSubDir/other-file" + $fileName = "file.txt" + $filePath = "$TestDrive/$fileName" $otherFileName = "other-file" $otherFile = "$otherSubDir/$otherFileName" $symToOther = "$subDir/sym-to-other" @@ -291,7 +289,8 @@ Describe "Copy-Item can avoid copying an item onto itself" -Tags "CI" { $symdToOther = "$subDir/symd-to-other" $junctionToOther = "$subDir/junction-to-other" - # Set up our directories and links + # Set up our files, directories, links + New-Item -ItemType File $filePath -Value "stuff" >$null New-Item -ItemType Directory $subDir >$null New-Item -ItemType Directory $otherSubDir >$null New-Item -ItemType File $otherFile -Value "some text" >$null @@ -312,109 +311,115 @@ Describe "Copy-Item can avoid copying an item onto itself" -Tags "CI" { { New-Item -ItemType SymbolicLink $symdToOther -Value $otherSubDir >$null } + } - # Test the ability to avoid an item copying onto itself - function TestSelfCopy($testCase) - { - It "$($testCase.Name)" -Skip:($testCase.SkipIf) { - try - { - Copy-Item -Path $testCase.Source -Destination $testCase.Destination -ErrorAction Stop - - if ($testCase.SelfCopyExpected) - { - # we expected the copy to fail and it did not - throw "Copy-Item should not have succeeded!" - } - else - { - # we expected the copy to succeed. make sure it did - Test-Path $testCase.Destination | Should Be $true - Remove-Item -Path $testCase.Destination -Force -ErrorAction SilentlyContinue - } - } - catch - { - # we expected the copy to fail and it did. make sure it failed the right way - $_.FullyQualifiedErrorId | Should Be "CopyError,Microsoft.PowerShell.Commands.CopyItemCommand" - $_.Exception | Should BeOfType System.IO.IOException - $_.Exception.Data[$selfCopyKey] | Should Not Be $null - } - } + Context "Copy-Item using different case (on case-sensitive file systems)" { + BeforeEach { + $sourcePath = $filePath + $destinationPath = "$TestDrive/" + $fileName.Toupper() + } + AfterEach { + Remove-Item -Path $destinationPath -Force -ErrorAction SilentlyContinue } - # Set up our test cases - $testCases = @( - @{ - Name = "Copy to same path" - Source = $otherFile - Destination = $otherFile - SelfCopyExpected = $true - } - @{ - Name = "Copy to similar path, different case" - Source = $otherFile - Destination = "$otherSubDir/" + $otherFileName.ToUpper() - SelfCopyExpected = -Not $IsCaseSensitive - } - @{ - Name = "Copy hard link" - Source = $hardToOtherFile - Destination = $otherFile - SelfCopyExpected = $true + It "Copy-Item can copy to file name differing only by case" { + if ($isCaseSensitive) + { + Copy-Item -Path $sourcePath -Destination $destinationPath -ErrorAction SilentlyContinue | Should Be $null + Test-Path -Path $destinationPath | Should Be $true } - @{ - Name = "Copy hard link, reversed" - Source = $otherFile - Destination = $hardToOtherFile - SelfCopyExpected = $true + else + { + Copy-Item -Path $sourcePath -Destination $destinationPath -ErrorAction SilentlyContinue | ShouldBeErrorId "CopyError,Microsoft.PowerShell.Commands.CopyItemCommand" + $_.Exception | Should BeOfType System.IO.IOException + $_.Exception.Data[$selfCopyKey] | Should Not Be $null } + } + } - # Only do the symbolic link tests if we were priveliged to create the links - @{ - Name = "Copy symbolic link to target" - Source = $symToOtherFile - Destination = $otherFile - SelfCopyExpected = $true - SkipIf = -Not $canSymlink - } - @{ - Name = "Copy symbolic link to symbolic link with same target" - Source = $secondSymToOther - Destination = $symToOther - SelfCopyExpected = $true - SkipIf = -Not $canSymlink - } - @{ - Name = "Copy through chain of symbolic links" - Source = $symToSym - Destination = $otherSubDir - SelfCopyExpected = $true - SkipIf = -Not $canSymlink - } + Context "Copy-Item avoids copying an item onto itself" { + BeforeAll { + # Set up our test cases + $testCases = @( + @{ + Name = "Copy to same path" + Source = $otherFile + Destination = $otherFile + } + @{ + Name = "Copy hard link" + Source = $hardToOtherFile + Destination = $otherFile + } + @{ + Name = "Copy hard link, reversed" + Source = $otherFile + Destination = $hardToOtherFile + } + + # Only do the symbolic link tests if we were priveliged to create the links + @{ + Name = "Copy symbolic link to target" + Source = $symToOtherFile + Destination = $otherFile + SkipIf = -Not $canSymlink + } + @{ + Name = "Copy symbolic link to symbolic link with same target" + Source = $secondSymToOther + Destination = $symToOther + SkipIf = -Not $canSymlink + } + @{ + Name = "Copy through chain of symbolic links" + Source = $symToSym + Destination = $otherSubDir + SkipIf = -Not $canSymlink + } + + # Junctions and directory symbolic links are Windows and NTFS only + @{ + Name = "Copy junction to target" + Source = $junctionToOther + Destination = $otherSubDir + SkipIf = -Not $canJunction + } + @{ + Name = "Copy directory symbolic link to target" + Source = $symdToOther + Destination = $otherSubDir + SkipIf = -Not $canDirSymLink + } + ) + } + + #It "" -TestCases $testCases -Skip:($testCase.SkipIf) { + It "" -TestCases $testCases { + Param ( + [string]$Name, + [string]$Source, + [string]$Destination, + [bool]$SelfCopyExpected + ) - # Junctions and directory symbolic links are Windows and NTFS only - @{ - Name = "Copy junction to target" - Source = $junctionToOther - Destination = $otherSubDir - SelfCopyExpected = $true - SkipIf = -Not $canJunction + # The source path must exist. + # If it does not, it's because it could not be created in the BeforeAll block + if (-Not (Test-Path -Path $Source)) + { + return } - @{ - Name = "Copy directory symbolic link to target" - Source = $symdToOther - Destination = $otherSubDir - SelfCopyExpected = $true - SkipIf = -Not $canDirSymLink + + # The destination path must exist. + # If it does not, it's because it could not be created in the BeforeAll block + if ($SelfCopyExpected -And (TestPath -Path $Destination)) + { + return } - ) - } - # run each of our tests - foreach ($case in $testCases) - { - TestSelfCopy($case) + $exc = { Copy-Item -Path $Source -Destination $Destination -ErrorAction Stop } | ShouldBeErrorId "CopyError,Microsoft.PowerShell.Commands.CopyItemCommand" + $Error[0].Exception | Should BeOfType System.IO.IOException + $Error[0].Exception.Data[$selfCopyKey] | Should Not Be $null + } } } @@ -801,12 +806,11 @@ Describe "Extended FileSystem Path/Location Cmdlet Provider Tests" -Tags "Featur $result = Split-Path -Path $level1_0Full -Leaf $result | Should Be $level1_0 } - + It 'Validate LeafBase' { $result = Split-Path -Path "$level2_1Full$fileExt" -LeafBase $result | Should Be $level2_1 } - It 'Validate LeafBase is not over-zealous' { $result = Split-Path -Path "$level2_1Full$fileExt$fileExt" -LeafBase From a1e905b9c12138ba775106c9bf9190191a42f706 Mon Sep 17 00:00:00 2001 From: Jeff Bienstadt Date: Tue, 18 Apr 2017 21:39:16 -0700 Subject: [PATCH 3/7] Fix Pester test for Windows --- .../Microsoft.PowerShell.Management/FileSystem.Tests.ps1 | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 index 4d95ae93795..ca24dc51a7d 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 @@ -330,9 +330,9 @@ Describe "Copy-Item can avoid copying an item onto itself" -Tags "CI", "RequireA } else { - Copy-Item -Path $sourcePath -Destination $destinationPath -ErrorAction SilentlyContinue | ShouldBeErrorId "CopyError,Microsoft.PowerShell.Commands.CopyItemCommand" - $_.Exception | Should BeOfType System.IO.IOException - $_.Exception.Data[$selfCopyKey] | Should Not Be $null + { Copy-Item -Path $sourcePath -Destination $destinationPath -ErrorAction Stop } | ShouldBeErrorId "CopyError,Microsoft.PowerShell.Commands.CopyItemCommand" + $Error[0].Exception | Should BeOfType System.IO.IOException + $Error[0].Exception.Data[$selfCopyKey] | Should Not Be $null } } } @@ -806,7 +806,7 @@ Describe "Extended FileSystem Path/Location Cmdlet Provider Tests" -Tags "Featur $result = Split-Path -Path $level1_0Full -Leaf $result | Should Be $level1_0 } - + It 'Validate LeafBase' { $result = Split-Path -Path "$level2_1Full$fileExt" -LeafBase $result | Should Be $level2_1 From 9fff05b20f11c59d3875c43d1f2bd16823ae8d7a Mon Sep 17 00:00:00 2001 From: Jeff Bienstadt Date: Wed, 19 Apr 2017 17:44:17 -0700 Subject: [PATCH 4/7] More test changes per code review --- .../FileSystem.Tests.ps1 | 89 ++++++------------- 1 file changed, 29 insertions(+), 60 deletions(-) diff --git a/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 index ca24dc51a7d..445d7b20b75 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 @@ -249,32 +249,16 @@ Describe "Basic FileSystem Provider Tests" -Tags "CI" { Describe "Copy-Item can avoid copying an item onto itself" -Tags "CI", "RequireAdminOnWindows" { BeforeAll { - # In Windows, only Administrators can create symbolic links. - # In Unix, anyone can. - function CanMakeSymlink - { - if ($IsWindows) - { - return Test-IsElevated - } - else - { - return $true - } - } - # For now, we'll assume the tests are running the platform's # native filesystem, in its default mode $isCaseSensitive = $IsLinux - $canSymlink = CanMakeSymlink - $canDirSymLink = $IsWindows -And (CanMakeSymlink) + $canDirSymLink = $IsWindows $canJunction = $IsWindows # The name of the key in an exception's Data dictionary when an # attempt is made to copy an item onto itself. $selfCopyKey = "SelfCopy" - # Names of files, directories, and links we'll be using to test $subDir = "$TestDrive/sub" $otherSubDir = "$TestDrive/other-sub" $fileName = "file.txt" @@ -289,18 +273,14 @@ Describe "Copy-Item can avoid copying an item onto itself" -Tags "CI", "RequireA $symdToOther = "$subDir/symd-to-other" $junctionToOther = "$subDir/junction-to-other" - # Set up our files, directories, links New-Item -ItemType File $filePath -Value "stuff" >$null New-Item -ItemType Directory $subDir >$null New-Item -ItemType Directory $otherSubDir >$null New-Item -ItemType File $otherFile -Value "some text" >$null - if ($canSymlink) - { - New-Item -ItemType SymbolicLink $symToOther -Value $otherSubDir >$null - New-Item -ItemType SymbolicLink $secondSymToOther -Value $otherSubDir >$null - New-Item -ItemType SymbolicLink $symToSym -Value $symToOther >$null - New-Item -ItemType SymbolicLink $symToOtherFile -Value $otherFile >$null - } + New-Item -ItemType SymbolicLink $symToOther -Value $otherSubDir >$null + New-Item -ItemType SymbolicLink $secondSymToOther -Value $otherSubDir >$null + New-Item -ItemType SymbolicLink $symToSym -Value $symToOther >$null + New-Item -ItemType SymbolicLink $symToOtherFile -Value $otherFile >$null New-Item -ItemType HardLink $hardToOtherFile -Value $otherFile >$null if ($canJunction) @@ -319,7 +299,7 @@ Describe "Copy-Item can avoid copying an item onto itself" -Tags "CI", "RequireA $destinationPath = "$TestDrive/" + $fileName.Toupper() } AfterEach { - Remove-Item -Path $destinationPath -Force -ErrorAction SilentlyContinue + Remove-Item -Path $destinationPath -ErrorAction SilentlyContinue } It "Copy-Item can copy to file name differing only by case" { @@ -339,7 +319,6 @@ Describe "Copy-Item can avoid copying an item onto itself" -Tags "CI", "RequireA Context "Copy-Item avoids copying an item onto itself" { BeforeAll { - # Set up our test cases $testCases = @( @{ Name = "Copy to same path" @@ -356,67 +335,57 @@ Describe "Copy-Item can avoid copying an item onto itself" -Tags "CI", "RequireA Source = $otherFile Destination = $hardToOtherFile } - - # Only do the symbolic link tests if we were priveliged to create the links @{ Name = "Copy symbolic link to target" Source = $symToOtherFile Destination = $otherFile - SkipIf = -Not $canSymlink } @{ Name = "Copy symbolic link to symbolic link with same target" Source = $secondSymToOther Destination = $symToOther - SkipIf = -Not $canSymlink } @{ Name = "Copy through chain of symbolic links" Source = $symToSym Destination = $otherSubDir - SkipIf = -Not $canSymlink - } - - # Junctions and directory symbolic links are Windows and NTFS only - @{ - Name = "Copy junction to target" - Source = $junctionToOther - Destination = $otherSubDir - SkipIf = -Not $canJunction - } - @{ - Name = "Copy directory symbolic link to target" - Source = $symdToOther - Destination = $otherSubDir - SkipIf = -Not $canDirSymLink } ) + + # Junctions and directory symbolic links are Windows and NTFS only + if ($IsWindows) + { + $testCases += @( + @{ + Name = "Copy junction to target" + Source = $junctionToOther + Destination = $otherSubDir + } + @{ + Name = "Copy directory symbolic link to target" + Source = $symdToOther + Destination = $otherSubDir + } + ) + } } - #It "" -TestCases $testCases -Skip:($testCase.SkipIf) { It "" -TestCases $testCases { Param ( [string]$Name, [string]$Source, - [string]$Destination, - [bool]$SelfCopyExpected + [string]$Destination ) - # The source path must exist. - # If it does not, it's because it could not be created in the BeforeAll block - if (-Not (Test-Path -Path $Source)) - { - return - } - - # The destination path must exist. - # If it does not, it's because it could not be created in the BeforeAll block - if ($SelfCopyExpected -And (TestPath -Path $Destination)) + # The source and destination paths must exist. + # If either do not, it's because they could not be created in the BeforeAll block + # This is not a test failure, but rather a precondition failure. + if ((-Not (Test-Path -Path $Source)) -or (-Not (Test-Path -Path $Destination))) { return } - $exc = { Copy-Item -Path $Source -Destination $Destination -ErrorAction Stop } | ShouldBeErrorId "CopyError,Microsoft.PowerShell.Commands.CopyItemCommand" + { Copy-Item -Path $Source -Destination $Destination -ErrorAction Stop } | ShouldBeErrorId "CopyError,Microsoft.PowerShell.Commands.CopyItemCommand" $Error[0].Exception | Should BeOfType System.IO.IOException $Error[0].Exception.Data[$selfCopyKey] | Should Not Be $null } From 8814054ef567a3d2772ac1b7e4f3f56f6020e9d3 Mon Sep 17 00:00:00 2001 From: Jeff Bienstadt Date: Thu, 20 Apr 2017 13:10:59 -0700 Subject: [PATCH 5/7] More changes per code review, and fix Travis CI failure on macOS --- .../FileSystem.Tests.ps1 | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 index 445d7b20b75..9cecee7ce27 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 @@ -252,13 +252,12 @@ Describe "Copy-Item can avoid copying an item onto itself" -Tags "CI", "RequireA # For now, we'll assume the tests are running the platform's # native filesystem, in its default mode $isCaseSensitive = $IsLinux - $canDirSymLink = $IsWindows - $canJunction = $IsWindows # The name of the key in an exception's Data dictionary when an # attempt is made to copy an item onto itself. $selfCopyKey = "SelfCopy" + $TestDrive = "TestDrive:" $subDir = "$TestDrive/sub" $otherSubDir = "$TestDrive/other-sub" $fileName = "file.txt" @@ -283,12 +282,9 @@ Describe "Copy-Item can avoid copying an item onto itself" -Tags "CI", "RequireA New-Item -ItemType SymbolicLink $symToOtherFile -Value $otherFile >$null New-Item -ItemType HardLink $hardToOtherFile -Value $otherFile >$null - if ($canJunction) + if ($IsWindows) { New-Item -ItemType Junction $junctionToOther -Value $otherSubDir >$null - } - if ($canDirSymLink) - { New-Item -ItemType SymbolicLink $symdToOther -Value $otherSubDir >$null } } @@ -377,14 +373,6 @@ Describe "Copy-Item can avoid copying an item onto itself" -Tags "CI", "RequireA [string]$Destination ) - # The source and destination paths must exist. - # If either do not, it's because they could not be created in the BeforeAll block - # This is not a test failure, but rather a precondition failure. - if ((-Not (Test-Path -Path $Source)) -or (-Not (Test-Path -Path $Destination))) - { - return - } - { Copy-Item -Path $Source -Destination $Destination -ErrorAction Stop } | ShouldBeErrorId "CopyError,Microsoft.PowerShell.Commands.CopyItemCommand" $Error[0].Exception | Should BeOfType System.IO.IOException $Error[0].Exception.Data[$selfCopyKey] | Should Not Be $null From 19702a0ae8b912b41eb0dc8701b5485c428d21c5 Mon Sep 17 00:00:00 2001 From: Jeff Bienstadt Date: Fri, 21 Apr 2017 02:13:50 -0700 Subject: [PATCH 6/7] Move Windows-only test into separate It block. Revise comments in C++ code. --- .../src/issamefilesystemitem.cpp | 4 +- .../FileSystem.Tests.ps1 | 39 ++++++++++++------- 2 files changed, 26 insertions(+), 17 deletions(-) diff --git a/src/libpsl-native/src/issamefilesystemitem.cpp b/src/libpsl-native/src/issamefilesystemitem.cpp index ba0f8390397..9e63e6e0749 100644 --- a/src/libpsl-native/src/issamefilesystemitem.cpp +++ b/src/libpsl-native/src/issamefilesystemitem.cpp @@ -1,6 +1,6 @@ //! @file issamefilesystemitem.cpp //! @author Jeff Bienstadt -//! @brief returns if two paths ultimately point to the same filesystem object +//! @brief Determines whether two paths ultimately point to the same filesystem object #include "getstat.h" #include "issamefilesystemitem.h" @@ -10,7 +10,7 @@ #include #include -//! @brief returns if two paths ultimately refer to the same file or directory. +//! @brief Returns a boolean value indicating whether two paths ultimately refer to the same file or directory. //! //! IsSameFileSystemItem //! diff --git a/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 index 9cecee7ce27..1ddc5ed5d4f 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 @@ -349,21 +349,18 @@ Describe "Copy-Item can avoid copying an item onto itself" -Tags "CI", "RequireA ) # Junctions and directory symbolic links are Windows and NTFS only - if ($IsWindows) - { - $testCases += @( - @{ - Name = "Copy junction to target" - Source = $junctionToOther - Destination = $otherSubDir - } - @{ - Name = "Copy directory symbolic link to target" - Source = $symdToOther - Destination = $otherSubDir - } - ) - } + $windowsTestCases = @( + @{ + Name = "Copy junction to target" + Source = $junctionToOther + Destination = $otherSubDir + } + @{ + Name = "Copy directory symbolic link to target" + Source = $symdToOther + Destination = $otherSubDir + } + ) } It "" -TestCases $testCases { @@ -377,6 +374,18 @@ Describe "Copy-Item can avoid copying an item onto itself" -Tags "CI", "RequireA $Error[0].Exception | Should BeOfType System.IO.IOException $Error[0].Exception.Data[$selfCopyKey] | Should Not Be $null } + + It "" -TestCases $windowsTestCases -Skip:(-not $IsWindows) { + Param ( + [string]$Name, + [string]$Source, + [string]$Destination + ) + + { Copy-Item -Path $Source -Destination $Destination -ErrorAction Stop } | ShouldBeErrorId "CopyError,Microsoft.PowerShell.Commands.CopyItemCommand" + $Error[0].Exception | Should BeOfType System.IO.IOException + $Error[0].Exception.Data[$selfCopyKey] | Should Not Be $null + } } } From 07b15946ed549a67cbfd978140199c7229f262c7 Mon Sep 17 00:00:00 2001 From: Jeff Bienstadt Date: Fri, 21 Apr 2017 02:21:41 -0700 Subject: [PATCH 7/7] Move test body from It block into function. --- .../FileSystem.Tests.ps1 | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 index 1ddc5ed5d4f..4a881d36ff5 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Management/FileSystem.Tests.ps1 @@ -361,6 +361,18 @@ Describe "Copy-Item can avoid copying an item onto itself" -Tags "CI", "RequireA Destination = $otherSubDir } ) + + function TestSelfCopy + { + Param ( + [string]$Source, + [string]$Destination + ) + + { Copy-Item -Path $Source -Destination $Destination -ErrorAction Stop } | ShouldBeErrorId "CopyError,Microsoft.PowerShell.Commands.CopyItemCommand" + $Error[0].Exception | Should BeOfType System.IO.IOException + $Error[0].Exception.Data[$selfCopyKey] | Should Not Be $null + } } It "" -TestCases $testCases { @@ -370,9 +382,7 @@ Describe "Copy-Item can avoid copying an item onto itself" -Tags "CI", "RequireA [string]$Destination ) - { Copy-Item -Path $Source -Destination $Destination -ErrorAction Stop } | ShouldBeErrorId "CopyError,Microsoft.PowerShell.Commands.CopyItemCommand" - $Error[0].Exception | Should BeOfType System.IO.IOException - $Error[0].Exception.Data[$selfCopyKey] | Should Not Be $null + TestSelfCopy $Source $Destination } It "" -TestCases $windowsTestCases -Skip:(-not $IsWindows) { @@ -382,9 +392,7 @@ Describe "Copy-Item can avoid copying an item onto itself" -Tags "CI", "RequireA [string]$Destination ) - { Copy-Item -Path $Source -Destination $Destination -ErrorAction Stop } | ShouldBeErrorId "CopyError,Microsoft.PowerShell.Commands.CopyItemCommand" - $Error[0].Exception | Should BeOfType System.IO.IOException - $Error[0].Exception.Data[$selfCopyKey] | Should Not Be $null + TestSelfCopy $Source $Destination } } }