From 16270cd8aea06263057824472a451eea6f3d4d3f Mon Sep 17 00:00:00 2001 From: Steve Berdy <86739818+steveberdy@users.noreply.github.com> Date: Thu, 8 Jul 2021 16:47:25 -0400 Subject: [PATCH 1/5] Update Path.Windows.cs --- .../src/System/IO/Path.Windows.cs | 31 +++++++++++-------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Path.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Path.Windows.cs index 5f4b8e11a19206..95b625e88e2d2e 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Path.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Path.Windows.cs @@ -49,17 +49,8 @@ public static string GetFullPath(string path) // unpredictable results. if (path.Contains('\0')) throw new ArgumentException(SR.Argument_InvalidPathChars, nameof(path)); - - if (PathInternal.IsExtended(path.AsSpan())) - { - // \\?\ paths are considered normalized by definition. Windows doesn't normalize \\?\ - // paths and neither should we. Even if we wanted to GetFullPathName does not work - // properly with device paths. If one wants to pass a \\?\ path through normalization - // one can chop off the prefix, pass it to GetFullPath and add it again. - return path; - } - - return PathHelper.Normalize(path); + + return GetFullQualifiedPath(path); } public static string GetFullPath(string path, string basePath) @@ -77,7 +68,7 @@ public static string GetFullPath(string path, string basePath) throw new ArgumentException(SR.Argument_InvalidPathChars); if (IsPathFullyQualified(path)) - return GetFullPath(path); + return GetFullQualifiedPath(path); if (PathInternal.IsEffectivelyEmpty(path.AsSpan())) return basePath; @@ -129,7 +120,21 @@ public static string GetFullPath(string path, string basePath) return PathInternal.IsDevice(combinedPath.AsSpan()) ? PathInternal.RemoveRelativeSegments(combinedPath, PathInternal.GetRootLength(combinedPath.AsSpan())) - : GetFullPath(combinedPath); + : GetFullQualifiedPath(combinedPath); + } + + private static string GetFullQualifiedPath(string path) + { + if (PathInternal.IsExtended(path.AsSpan())) + { + // \\?\ paths are considered normalized by definition. Windows doesn't normalize \\?\ + // paths and neither should we. Even if we wanted to GetFullPathName does not work + // properly with device paths. If one wants to pass a \\?\ path through normalization + // one can chop off the prefix, pass it to GetFullPath and add it again. + return path; + } + + return PathHelper.Normalize(path); } public static string GetTempPath() From ce6bfd26b32b71c2460088a79c3b2cdc7360e14a Mon Sep 17 00:00:00 2001 From: Steve Berdy <86739818+steveberdy@users.noreply.github.com> Date: Fri, 9 Jul 2021 08:18:11 -0400 Subject: [PATCH 2/5] Change helper method to internal Switched method from a private protection level to an internal protection level. Also removed trailing whitespace. --- .../System.Private.CoreLib/src/System/IO/Path.Windows.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Path.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Path.Windows.cs index 95b625e88e2d2e..6737ffbc357b2c 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Path.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Path.Windows.cs @@ -49,7 +49,7 @@ public static string GetFullPath(string path) // unpredictable results. if (path.Contains('\0')) throw new ArgumentException(SR.Argument_InvalidPathChars, nameof(path)); - + return GetFullQualifiedPath(path); } @@ -122,8 +122,8 @@ public static string GetFullPath(string path, string basePath) ? PathInternal.RemoveRelativeSegments(combinedPath, PathInternal.GetRootLength(combinedPath.AsSpan())) : GetFullQualifiedPath(combinedPath); } - - private static string GetFullQualifiedPath(string path) + + internal static string GetFullQualifiedPath(string path) { if (PathInternal.IsExtended(path.AsSpan())) { From b88008d56cac7b8dc22e812e761cd68ccb853fc6 Mon Sep 17 00:00:00 2001 From: Steve Berdy Date: Wed, 14 Jul 2021 12:48:36 -0400 Subject: [PATCH 3/5] Removed sourceFullPath from Unix FileSystem.MoveDirectory thrown exception --- .../src/System/IO/Directory.cs | 2 +- .../src/System/IO/FileSystem.Unix.cs | 4 ++-- .../src/System/IO/FileSystem.Windows.cs | 23 ++++++++++--------- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Directory.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Directory.cs index f6bc4f42b064d2..9768c16248dfd8 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Directory.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Directory.cs @@ -288,7 +288,7 @@ public static void Move(string sourceDirName, string destDirName) if (!FileSystem.DirectoryExists(fullsourceDirName) && !FileSystem.FileExists(fullsourceDirName)) throw new DirectoryNotFoundException(SR.Format(SR.IO_PathNotFound_Path, fullsourceDirName)); - if (!sameDirectoryDifferentCase // This check is to allowing renaming of directories + if (!sameDirectoryDifferentCase // This check is to allow renaming of directories && FileSystem.DirectoryExists(fulldestDirName)) throw new IOException(SR.Format(SR.IO_AlreadyExists_Name, fulldestDirName)); diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs index 0151499893aa8b..43111fb81ae016 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Unix.cs @@ -380,7 +380,7 @@ public static void MoveDirectory(string sourceFullPath, string destFullPath) // 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 + // This surfaces as an IOException, if we let it go beyond here it would // give DirectoryNotFound. if (Path.EndsInDirectorySeparator(sourceFullPath)) @@ -405,7 +405,7 @@ public static void MoveDirectory(string sourceFullPath, string destFullPath) case Interop.Error.EACCES: // match Win32 exception throw new IOException(SR.Format(SR.UnauthorizedAccess_IODenied_Path, sourceFullPath), errorInfo.RawErrno); default: - throw Interop.GetExceptionForIoErrno(errorInfo, sourceFullPath, isDirectory: true); + throw Interop.GetExceptionForIoErrno(errorInfo, isDirectory: true); } } } diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs index 5f88f53a7c539f..43a5084e1765d3 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs @@ -123,17 +123,18 @@ public static void MoveDirectory(string sourceFullPath, string destFullPath) { int errorCode = Marshal.GetLastWin32Error(); - if (errorCode == Interop.Errors.ERROR_FILE_NOT_FOUND) - throw Win32Marshal.GetExceptionForWin32Error(Interop.Errors.ERROR_PATH_NOT_FOUND, sourceFullPath); - - if (errorCode == Interop.Errors.ERROR_ALREADY_EXISTS) - throw Win32Marshal.GetExceptionForWin32Error(Interop.Errors.ERROR_ALREADY_EXISTS, destFullPath); - - // This check was originally put in for Win9x (unfortunately without special casing it to be for Win9x only). We can't change the NT codepath now for backcomp reasons. - if (errorCode == Interop.Errors.ERROR_ACCESS_DENIED) // WinNT throws IOException. This check is for Win9x. We can't change it for backcomp. - throw new IOException(SR.Format(SR.UnauthorizedAccess_IODenied_Path, sourceFullPath), Win32Marshal.MakeHRFromErrorCode(errorCode)); - - throw Win32Marshal.GetExceptionForWin32Error(errorCode); + switch (errorCode) + { + case Interop.Errors.ERROR_FILE_NOT_FOUND: + throw Win32Marshal.GetExceptionForWin32Error(Interop.Errors.ERROR_PATH_NOT_FOUND, sourceFullPath); + case Interop.Errors.ERROR_ALREADY_EXISTS: + throw Win32Marshal.GetExceptionForWin32Error(Interop.Errors.ERROR_ALREADY_EXISTS, destFullPath); + // This check was originally put in for Win9x (unfortunately without special casing it to be for Win9x only). We can't change the NT codepath now for backcomp reasons. + case Interop.Errors.ERROR_ACCESS_DENIED: // WinNT throws IOException. This check is for Win9x. We can't change it for backcomp. + throw new IOException(SR.Format(SR.UnauthorizedAccess_IODenied_Path, sourceFullPath), Win32Marshal.MakeHRFromErrorCode(errorCode)); + default: + throw Win32Marshal.GetExceptionForWin32Error(errorCode); + } } } From 7890ab49908128578d392632fe5edadc61568dd9 Mon Sep 17 00:00:00 2001 From: Steve Berdy Date: Wed, 14 Jul 2021 12:55:46 -0400 Subject: [PATCH 4/5] Undo commit from main fork branch --- .../src/System/IO/Path.Windows.cs | 29 ++++++++----------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Path.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Path.Windows.cs index 6737ffbc357b2c..5f4b8e11a19206 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/Path.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/Path.Windows.cs @@ -50,7 +50,16 @@ public static string GetFullPath(string path) if (path.Contains('\0')) throw new ArgumentException(SR.Argument_InvalidPathChars, nameof(path)); - return GetFullQualifiedPath(path); + if (PathInternal.IsExtended(path.AsSpan())) + { + // \\?\ paths are considered normalized by definition. Windows doesn't normalize \\?\ + // paths and neither should we. Even if we wanted to GetFullPathName does not work + // properly with device paths. If one wants to pass a \\?\ path through normalization + // one can chop off the prefix, pass it to GetFullPath and add it again. + return path; + } + + return PathHelper.Normalize(path); } public static string GetFullPath(string path, string basePath) @@ -68,7 +77,7 @@ public static string GetFullPath(string path, string basePath) throw new ArgumentException(SR.Argument_InvalidPathChars); if (IsPathFullyQualified(path)) - return GetFullQualifiedPath(path); + return GetFullPath(path); if (PathInternal.IsEffectivelyEmpty(path.AsSpan())) return basePath; @@ -120,21 +129,7 @@ public static string GetFullPath(string path, string basePath) return PathInternal.IsDevice(combinedPath.AsSpan()) ? PathInternal.RemoveRelativeSegments(combinedPath, PathInternal.GetRootLength(combinedPath.AsSpan())) - : GetFullQualifiedPath(combinedPath); - } - - internal static string GetFullQualifiedPath(string path) - { - if (PathInternal.IsExtended(path.AsSpan())) - { - // \\?\ paths are considered normalized by definition. Windows doesn't normalize \\?\ - // paths and neither should we. Even if we wanted to GetFullPathName does not work - // properly with device paths. If one wants to pass a \\?\ path through normalization - // one can chop off the prefix, pass it to GetFullPath and add it again. - return path; - } - - return PathHelper.Normalize(path); + : GetFullPath(combinedPath); } public static string GetTempPath() From 208b9f4d69ca573837613d2ddacb7330a932a04d Mon Sep 17 00:00:00 2001 From: Steve Berdy Date: Thu, 15 Jul 2021 08:17:11 -0400 Subject: [PATCH 5/5] Revert FileSystem.Windows.cs changes --- .../src/System/IO/FileSystem.Windows.cs | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs index 43a5084e1765d3..5f88f53a7c539f 100644 --- a/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs +++ b/src/libraries/System.Private.CoreLib/src/System/IO/FileSystem.Windows.cs @@ -123,18 +123,17 @@ public static void MoveDirectory(string sourceFullPath, string destFullPath) { int errorCode = Marshal.GetLastWin32Error(); - switch (errorCode) - { - case Interop.Errors.ERROR_FILE_NOT_FOUND: - throw Win32Marshal.GetExceptionForWin32Error(Interop.Errors.ERROR_PATH_NOT_FOUND, sourceFullPath); - case Interop.Errors.ERROR_ALREADY_EXISTS: - throw Win32Marshal.GetExceptionForWin32Error(Interop.Errors.ERROR_ALREADY_EXISTS, destFullPath); - // This check was originally put in for Win9x (unfortunately without special casing it to be for Win9x only). We can't change the NT codepath now for backcomp reasons. - case Interop.Errors.ERROR_ACCESS_DENIED: // WinNT throws IOException. This check is for Win9x. We can't change it for backcomp. - throw new IOException(SR.Format(SR.UnauthorizedAccess_IODenied_Path, sourceFullPath), Win32Marshal.MakeHRFromErrorCode(errorCode)); - default: - throw Win32Marshal.GetExceptionForWin32Error(errorCode); - } + if (errorCode == Interop.Errors.ERROR_FILE_NOT_FOUND) + throw Win32Marshal.GetExceptionForWin32Error(Interop.Errors.ERROR_PATH_NOT_FOUND, sourceFullPath); + + if (errorCode == Interop.Errors.ERROR_ALREADY_EXISTS) + throw Win32Marshal.GetExceptionForWin32Error(Interop.Errors.ERROR_ALREADY_EXISTS, destFullPath); + + // This check was originally put in for Win9x (unfortunately without special casing it to be for Win9x only). We can't change the NT codepath now for backcomp reasons. + if (errorCode == Interop.Errors.ERROR_ACCESS_DENIED) // WinNT throws IOException. This check is for Win9x. We can't change it for backcomp. + throw new IOException(SR.Format(SR.UnauthorizedAccess_IODenied_Path, sourceFullPath), Win32Marshal.MakeHRFromErrorCode(errorCode)); + + throw Win32Marshal.GetExceptionForWin32Error(errorCode); } }