Refactor repeated invalid character checks in Path.GetFullPath Unix and Windows ...

* Refactored repeated checks for invalid path chars

* Removed checkedForInvalids param and renamed method to GetFullPathInternal

* Apply suggestions from code review

* Cleaned up trailing whitespace

Co-authored-by: Jan Kotas <jkotas@microsoft.com>

diff --git a/src/libraries/System.Private.CoreLib/src/System/IO/Path.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/IO/Path.Unix.cs
index 5a7f0c8..61786ad 100644 (file)
--- a/src/libraries/System.Private.CoreLib/src/System/IO/Path.Unix.cs
+++ b/src/libraries/System.Private.CoreLib/src/System/IO/Path.Unix.cs
@@ -25,22 +25,7 @@ namespace System.IO
             if (path.Contains('\0'))
                 throw new ArgumentException(SR.Argument_InvalidPathChars, nameof(path));
 
-            // Expand with current directory if necessary
-            if (!IsPathRooted(path))
-            {
-                path = Combine(Interop.Sys.GetCwd(), path);
-            }
-
-            // We would ideally use realpath to do this, but it resolves symlinks, requires that the file actually exist,
-            // and turns it into a full path, which we only want if fullCheck is true.
-            string collapsedString = PathInternal.RemoveRelativeSegments(path, PathInternal.GetRootLength(path));
-
-            Debug.Assert(collapsedString.Length < path.Length || collapsedString.ToString() == path,
-                "Either we've removed characters, or the string should be unmodified from the input path.");
-
-            string result = collapsedString.Length == 0 ? PathInternal.DirectorySeparatorCharAsString : collapsedString;
-
-            return result;
+            return GetFullPathInternal(path);
         }
 
         public static string GetFullPath(string path, string basePath)
@@ -58,9 +43,33 @@ namespace System.IO
                 throw new ArgumentException(SR.Argument_InvalidPathChars);
 
             if (IsPathFullyQualified(path))
-                return GetFullPath(path);
+                return GetFullPathInternal(path);
+
+            return GetFullPathInternal(CombineInternal(basePath, path));
+        }
 
-            return GetFullPath(CombineInternal(basePath, path));
+        // Gets the full path without argument validation
+        private static string GetFullPathInternal(string path)
+        {
+            Debug.Assert(!string.IsNullOrEmpty(path));
+            Debug.Assert(!path.Contains('\0'));
+
+            // Expand with current directory if necessary
+            if (!IsPathRooted(path))
+            {
+                path = Combine(Interop.Sys.GetCwd(), path);
+            }
+
+            // We would ideally use realpath to do this, but it resolves symlinks, requires that the file actually exist,
+            // and turns it into a full path, which we only want if fullCheck is true.
+            string collapsedString = PathInternal.RemoveRelativeSegments(path, PathInternal.GetRootLength(path));
+
+            Debug.Assert(collapsedString.Length < path.Length || collapsedString.ToString() == path,
+                "Either we've removed characters, or the string should be unmodified from the input path.");
+
+            string result = collapsedString.Length == 0 ? PathInternal.DirectorySeparatorCharAsString : collapsedString;
+
+            return result;
         }
 
         private static string RemoveLongPathPrefix(string path)

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 4d08cd9..e0ad5c0 100644 (file)
--- 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,7 @@ namespace System.IO
             if (path.Contains('\0'))
                 throw new ArgumentException(SR.Argument_InvalidPathChars, nameof(path));
 
-            return GetFullyQualifiedPath(path);
+            return GetFullPathInternal(path);
         }
 
         public static string GetFullPath(string path, string basePath)
@@ -68,7 +68,7 @@ namespace System.IO
                 throw new ArgumentException(SR.Argument_InvalidPathChars);
 
             if (IsPathFullyQualified(path))
-                return GetFullyQualifiedPath(path);
+                return GetFullPathInternal(path);
 
             if (PathInternal.IsEffectivelyEmpty(path.AsSpan()))
                 return basePath;
@@ -120,11 +120,15 @@ namespace System.IO
 
             return PathInternal.IsDevice(combinedPath.AsSpan())
                 ? PathInternal.RemoveRelativeSegments(combinedPath, PathInternal.GetRootLength(combinedPath.AsSpan()))
-                : GetFullyQualifiedPath(combinedPath);
+                : GetFullPathInternal(combinedPath);
         }
 
-        internal static string GetFullyQualifiedPath(string path)
+        // Gets the full path without argument validation
+        private static string GetFullPathInternal(string path)
         {
+            Debug.Assert(!string.IsNullOrEmpty(path));
+            Debug.Assert(!path.Contains('\0'));
+
             if (PathInternal.IsExtended(path.AsSpan()))
             {
                 // \\?\ paths are considered normalized by definition. Windows doesn't normalize \\?\