From 80e9b0fb20c717087adf33bc805cfa92a6f0eeae Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 25 Mar 2021 12:06:51 +0100 Subject: [PATCH 1/2] File.Unix: Replace: increase Windows compatibility - When source and destination are not a file throw UnauthorizedAccessException. - When source and destination are the same file throw IOException. --- .../src/Resources/Strings.resx | 6 ++++ .../src/System/IO/FileSystem.Unix.cs | 30 ++++++++++++++++++ .../tests/File/Replace.cs | 31 +++++++++++++++++++ 3 files changed, 67 insertions(+) diff --git a/src/libraries/System.IO.FileSystem/src/Resources/Strings.resx b/src/libraries/System.IO.FileSystem/src/Resources/Strings.resx index 683090b0becc90..1d4d85ef079825 100644 --- a/src/libraries/System.IO.FileSystem/src/Resources/Strings.resx +++ b/src/libraries/System.IO.FileSystem/src/Resources/Strings.resx @@ -197,6 +197,9 @@ The specified directory '{0}' cannot be created. + + The source and destination are the same file. + Unable to read beyond the end of the stream. @@ -218,6 +221,9 @@ IO operation will not work. Most likely the file will become too long or the handle was not opened to support synchronous IO operations. + + The specified path '{0}' is not a file. + Could not find a part of the path. diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/FileSystem.Unix.cs b/src/libraries/System.IO.FileSystem/src/System/IO/FileSystem.Unix.cs index 5993c1d6ac5081..6e3b6445f0dae7 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/FileSystem.Unix.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/FileSystem.Unix.cs @@ -97,6 +97,36 @@ private static void LinkOrCopyFile (string sourceFullPath, string destFullPath) public static void ReplaceFile(string sourceFullPath, string destFullPath, string? destBackupFullPath, bool ignoreMetadataErrors) { + // Unix rename works in more cases, we limit to what is allowed by Windows File.Replace. + // These checks are not atomic, the file could change after a check was performed and before it is renamed. + Interop.Sys.FileStatus sourceStat; + if (Interop.Sys.LStat(sourceFullPath, out sourceStat) != 0) + { + Interop.ErrorInfo errno = Interop.Sys.GetLastErrorInfo(); + throw Interop.GetExceptionForIoErrno(errno, sourceFullPath); + } + // Check source is not a directory. + if ((sourceStat.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR) + { + throw new UnauthorizedAccessException(SR.Format(SR.IO_NotAFile, sourceFullPath)); + } + + Interop.Sys.FileStatus destStat; + if (Interop.Sys.LStat(destFullPath, out destStat) == 0) + { + // Check destination is not a directory. + if ((destStat.Mode & Interop.Sys.FileTypes.S_IFMT) == Interop.Sys.FileTypes.S_IFDIR) + { + throw new UnauthorizedAccessException(SR.Format(SR.IO_NotAFile, destFullPath)); + } + // Check source and destination are not the same. + if (sourceStat.Dev == destStat.Dev && + sourceStat.Ino == destStat.Ino) + { + throw new IOException(SR.IO_CannotReplaceSameFile); + } + } + if (destBackupFullPath != null) { // We're backing up the destination file to the backup file, so we need to first delete the backup diff --git a/src/libraries/System.IO.FileSystem/tests/File/Replace.cs b/src/libraries/System.IO.FileSystem/tests/File/Replace.cs index 2cd2a3011a3b76..8bebeef1bb042f 100644 --- a/src/libraries/System.IO.FileSystem/tests/File/Replace.cs +++ b/src/libraries/System.IO.FileSystem/tests/File/Replace.cs @@ -109,6 +109,37 @@ public void InvalidFileNames() Assert.Throws(() => Replace(testFile, testFile2, "*\0*")); } + [Fact] + public void SourceCannotBeADirectory() + { + string testFile = GetTestFilePath(); + File.Create(testFile).Dispose(); + string testDir = GetTestFilePath(); + Directory.CreateDirectory(testDir); + + Assert.Throws(() => File.Replace(testDir, testFile, null)); + } + + [Fact] + public void DestinationCannotBeADirectory() + { + string testFile = GetTestFilePath(); + File.Create(testFile).Dispose(); + string testDir = GetTestFilePath(); + Directory.CreateDirectory(testDir); + + Assert.Throws(() => File.Replace(testFile, testDir, null)); + } + + [Fact] + public void SourceAndDestinationCannotBeTheSame() + { + string testFile = GetTestFilePath(); + File.Create(testFile).Dispose(); + + Assert.Throws(() => File.Replace(testFile, testFile, null)); + } + #endregion } From 9bb83f3d72570aafccfe4cefb7dd4d5bc6733ac8 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Mon, 29 Mar 2021 13:13:40 +0200 Subject: [PATCH 2/2] Include paths in exception message --- src/libraries/System.IO.FileSystem/src/Resources/Strings.resx | 2 +- .../System.IO.FileSystem/src/System/IO/FileSystem.Unix.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.IO.FileSystem/src/Resources/Strings.resx b/src/libraries/System.IO.FileSystem/src/Resources/Strings.resx index 1d4d85ef079825..3c391ca6bcde4a 100644 --- a/src/libraries/System.IO.FileSystem/src/Resources/Strings.resx +++ b/src/libraries/System.IO.FileSystem/src/Resources/Strings.resx @@ -198,7 +198,7 @@ The specified directory '{0}' cannot be created. - The source and destination are the same file. + The source '{0}' and destination '{1}' are the same file. Unable to read beyond the end of the stream. diff --git a/src/libraries/System.IO.FileSystem/src/System/IO/FileSystem.Unix.cs b/src/libraries/System.IO.FileSystem/src/System/IO/FileSystem.Unix.cs index 6e3b6445f0dae7..6f36e92da99205 100644 --- a/src/libraries/System.IO.FileSystem/src/System/IO/FileSystem.Unix.cs +++ b/src/libraries/System.IO.FileSystem/src/System/IO/FileSystem.Unix.cs @@ -123,7 +123,7 @@ public static void ReplaceFile(string sourceFullPath, string destFullPath, strin if (sourceStat.Dev == destStat.Dev && sourceStat.Ino == destStat.Ino) { - throw new IOException(SR.IO_CannotReplaceSameFile); + throw new IOException(SR.Format(SR.IO_CannotReplaceSameFile, sourceFullPath, destFullPath)); } }