Fix using Directory to move files (#19716)

JeremyKuhne · web-flow · commit ea514e2f4e61 · 2017-05-12T19:21:10.000-07:00
* Fix using Directory to move files See #19710. NetFX behavior is that you can use Directory to move files. This was broken trying to match other legacy behavior in #17913. Tested on NetFX and CoreFX. * Fix check for Unix Unix impl doesn't tolerate a trailing separator. Public api explicitly checks for this. * More fixes and tests Unix won't move the file with a trailing slash. Add more tests for different combinations of trailing slashes. Validated against NetFX. * Attempt to get MoveDirectory further aligned Trying to handle trailing separator on a source file cases in the same way as Windows. * Extend tests, move Unix condition * Another tweak and more tests * Fix comment
diff --git a/src/System.IO.FileSystem/src/System/IO/Directory.cs b/src/System.IO.FileSystem/src/System/IO/Directory.cs
@@ -563,8 +563,10 @@ public static void Move(String sourceDirName, String destDirName)
             String destinationRoot = Path.GetPathRoot(destPath);
             if (!String.Equals(sourceRoot, destinationRoot, pathComparison))
                 throw new IOException(SR.IO_SourceDestMustHaveSameRoot);
-            
-            if (!FileSystem.Current.DirectoryExists(fullsourceDirName))
+
+            // Windows will throw if the source file/directory doesn't exist, we preemptively check
+            // to make sure our cross platform behavior matches NetFX behavior.
+            if (!FileSystem.Current.DirectoryExists(fullsourceDirName) && !FileSystem.Current.FileExists(fullsourceDirName))
                 throw new DirectoryNotFoundException(SR.Format(SR.IO_PathNotFound_Path, fullsourceDirName));
             
             if (FileSystem.Current.DirectoryExists(fulldestDirName))
diff --git a/src/System.IO.FileSystem/src/System/IO/DirectoryInfo.cs b/src/System.IO.FileSystem/src/System/IO/DirectoryInfo.cs
@@ -379,19 +379,20 @@ public DirectoryInfo Root
         }
 
         [System.Security.SecuritySafeCritical]
-        public void MoveTo(String destDirName)
+        public void MoveTo(string destDirName)
         {
             if (destDirName == null)
                 throw new ArgumentNullException(nameof(destDirName));
             if (destDirName.Length == 0)
                 throw new ArgumentException(SR.Argument_EmptyFileName, nameof(destDirName));
             Contract.EndContractBlock();
 
-            String fullDestDirName = Path.GetFullPath(destDirName);
-            if (fullDestDirName[fullDestDirName.Length - 1] != Path.DirectorySeparatorChar)
-                fullDestDirName = fullDestDirName + PathHelpers.DirectorySeparatorCharAsString;
+            string destination = Path.GetFullPath(destDirName);
+            string destinationWithSeparator = destination;
+            if (destinationWithSeparator[destinationWithSeparator.Length - 1] != Path.DirectorySeparatorChar)
+                destinationWithSeparator = destinationWithSeparator + PathHelpers.DirectorySeparatorCharAsString;
 
-            String fullSourcePath;
+            string fullSourcePath;
             if (FullPath.Length > 0 && FullPath[FullPath.Length - 1] == Path.DirectorySeparatorChar)
                 fullSourcePath = FullPath;
             else
@@ -400,28 +401,30 @@ public void MoveTo(String destDirName)
             if (PathInternal.IsDirectoryTooLong(fullSourcePath))
                 throw new PathTooLongException(SR.IO_PathTooLong);
 
-            if (PathInternal.IsDirectoryTooLong(fullDestDirName))
+            if (PathInternal.IsDirectoryTooLong(destinationWithSeparator))
                 throw new PathTooLongException(SR.IO_PathTooLong);
 
             StringComparison pathComparison = PathInternal.StringComparison;
-            if (String.Equals(fullSourcePath, fullDestDirName, pathComparison))
+            if (string.Equals(fullSourcePath, destinationWithSeparator, pathComparison))
                 throw new IOException(SR.IO_SourceDestMustBeDifferent);
 
-            String sourceRoot = Path.GetPathRoot(fullSourcePath);
-            String destinationRoot = Path.GetPathRoot(fullDestDirName);
+            string sourceRoot = Path.GetPathRoot(fullSourcePath);
+            string destinationRoot = Path.GetPathRoot(destinationWithSeparator);
 
-            if (!String.Equals(sourceRoot, destinationRoot, pathComparison))
+            if (!string.Equals(sourceRoot, destinationRoot, pathComparison))
                 throw new IOException(SR.IO_SourceDestMustHaveSameRoot);
 
-            if (!Exists)
+            // Windows will throw if the source file/directory doesn't exist, we preemptively check
+            // to make sure our cross platform behavior matches NetFX behavior.
+            if (!Exists && !FileSystem.Current.FileExists(FullPath))
                 throw new DirectoryNotFoundException(SR.Format(SR.IO_PathNotFound_Path, FullPath));
 
-            if (FileSystem.Current.DirectoryExists(fullDestDirName))
-                throw new IOException(SR.Format(SR.IO_AlreadyExists_Name, fullDestDirName));
+            if (FileSystem.Current.DirectoryExists(destinationWithSeparator))
+                throw new IOException(SR.Format(SR.IO_AlreadyExists_Name, destinationWithSeparator));
 
-            FileSystem.Current.MoveDirectory(FullPath, fullDestDirName);
+            FileSystem.Current.MoveDirectory(FullPath, destination);
 
-            FullPath = fullDestDirName;
+            FullPath = destinationWithSeparator;
             OriginalPath = destDirName;
             DisplayPath = GetDisplayName(OriginalPath);
 
diff --git a/src/System.IO.FileSystem/src/System/IO/UnixFileSystem.cs b/src/System.IO.FileSystem/src/System/IO/UnixFileSystem.cs
@@ -276,6 +276,24 @@ public override void CreateDirectory(string fullPath)
 
         public override void MoveDirectory(string sourceFullPath, string destFullPath)
         {
+            // Windows doesn't care if you try and copy a file via "MoveDirectory"...
+            if (FileExists(sourceFullPath))
+            {
+                // ... but it doesn't like the source to have a trailing slash ...
+
+                // On Windows we end up with ERROR_INVALID_NAME, which is
+                // "The filename, directory name, or volume label syntax is incorrect."
+                //
+                // This surfaces as a IOException, if we let it go beyond here it would
+                // give DirectoryNotFound.
+
+                if (PathHelpers.EndsInDirectorySeparator(sourceFullPath))
+                    throw new IOException(SR.Format(SR.IO_PathNotFound_Path, sourceFullPath));
+
+                // ... but it doesn't care if the destination has a trailing separator.
+                destFullPath = PathHelpers.TrimEndingDirectorySeparator(destFullPath);
+            }
+
             if (Interop.Sys.Rename(sourceFullPath, destFullPath) < 0)
             {
                 Interop.ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo();
@@ -389,7 +407,9 @@ private static bool DirectoryExists(string fullPath, out Interop.ErrorInfo error
         public override bool FileExists(string fullPath)
         {
             Interop.ErrorInfo ignored;
-            return FileExists(fullPath, Interop.Sys.FileTypes.S_IFREG, out ignored);
+
+            // Windows doesn't care about the trailing separator
+            return FileExists(PathHelpers.TrimEndingDirectorySeparator(fullPath), Interop.Sys.FileTypes.S_IFREG, out ignored);
         }
 
         private static bool FileExists(string fullPath, int fileType, out Interop.ErrorInfo errorInfo)
diff --git a/src/System.IO.FileSystem/tests/Directory/Move.cs b/src/System.IO.FileSystem/tests/Directory/Move.cs
@@ -56,6 +56,62 @@ public void MoveOntoExistingDirectory()
             Assert.Throws<IOException>(() => Move(testDir.FullName, secondDir.FullName));
         }
 
+        [Fact]
+        public void MoveFile()
+        {
+            // Regression https://github.com/dotnet/corefx/issues/19710
+            string source = GetTestFilePath();
+            string destination = GetTestFilePath();
+            File.Create(source).Dispose();
+            Move(source, destination);
+            Assert.True(File.Exists(destination));
+            Assert.False(File.Exists(source));
+        }
+
+        [Fact]
+        public void MoveFile_TrailingDestinationSlash()
+        {
+            // Regression https://github.com/dotnet/corefx/issues/19710
+            string source = GetTestFilePath();
+            string destination = GetTestFilePath();
+            File.Create(source).Dispose();
+            Move(source, destination + Path.DirectorySeparatorChar);
+            Assert.True(File.Exists(destination));
+            Assert.False(File.Exists(source));
+        }
+
+        [Fact]
+        [PlatformSpecific(TestPlatforms.Windows)]
+        public void MoveFile_TrailingDestinationAltSlash_Windows()
+        {
+            // Regression https://github.com/dotnet/corefx/issues/19710
+            string source = GetTestFilePath();
+            string destination = GetTestFilePath();
+            File.Create(source).Dispose();
+            Move(source, destination + Path.AltDirectorySeparatorChar);
+            Assert.True(File.Exists(destination));
+            Assert.False(File.Exists(source));
+        }
+
+        [Fact]
+        public void MoveFile_TrailingSourceSlash()
+        {
+            string source = GetTestFilePath();
+            string destination = GetTestFilePath();
+            File.Create(source).Dispose();
+            Assert.Throws<IOException>(() => Move(source + Path.DirectorySeparatorChar, destination));
+        }
+
+        [Fact]
+        [PlatformSpecific(TestPlatforms.Windows)]
+        public void MoveFile_TrailingSourceAltSlash_Windows()
+        {
+            string source = GetTestFilePath();
+            string destination = GetTestFilePath();
+            File.Create(source).Dispose();
+            Assert.Throws<IOException>(() => Move(source + Path.AltDirectorySeparatorChar, destination));
+        }
+
         [Fact]
         public void MoveOntoFile()
         {
diff --git a/src/System.IO.FileSystem/tests/File/Exists.cs b/src/System.IO.FileSystem/tests/File/Exists.cs
@@ -80,6 +80,31 @@ public void PathEndsInTrailingSlash()
             Assert.False(Exists(path));
         }
 
+        [Fact]
+        [PlatformSpecific(TestPlatforms.Windows)]
+        public void PathEndsInAltTrailingSlash_Windows()
+        {
+            string path = GetTestFilePath() + Path.DirectorySeparatorChar;
+            Assert.False(Exists(path));
+        }
+
+        [Fact]
+        public void PathEndsInTrailingSlash_AndExists()
+        {
+            string path = GetTestFilePath();
+            File.Create(path).Dispose();
+            Assert.False(Exists(path + Path.DirectorySeparatorChar));
+        }
+
+        [Fact]
+        [PlatformSpecific(TestPlatforms.Windows)]
+        public void PathEndsInAltTrailingSlash_AndExists_Windows()
+        {
+            string path = GetTestFilePath();
+            File.Create(path).Dispose();
+            Assert.False(Exists(path + Path.DirectorySeparatorChar));
+        }
+
         [Fact]
         public void PathAlreadyExistsAsDirectory()
         {
