Skip to content

Commit 58efa4b

Browse files
steveberdyjkotas
andauthored
Refactor repeated invalid character checks in Path.GetFullPath Unix and Windows (#56568)
* 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>
1 parent 91021fe commit 58efa4b

File tree

2 files changed

+35
-22
lines changed

2 files changed

+35
-22
lines changed

src/libraries/System.Private.CoreLib/src/System/IO/Path.Unix.cs

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -25,22 +25,7 @@ public static string GetFullPath(string path)
2525
if (path.Contains('\0'))
2626
throw new ArgumentException(SR.Argument_InvalidPathChars, nameof(path));
2727

28-
// Expand with current directory if necessary
29-
if (!IsPathRooted(path))
30-
{
31-
path = Combine(Interop.Sys.GetCwd(), path);
32-
}
33-
34-
// We would ideally use realpath to do this, but it resolves symlinks, requires that the file actually exist,
35-
// and turns it into a full path, which we only want if fullCheck is true.
36-
string collapsedString = PathInternal.RemoveRelativeSegments(path, PathInternal.GetRootLength(path));
37-
38-
Debug.Assert(collapsedString.Length < path.Length || collapsedString.ToString() == path,
39-
"Either we've removed characters, or the string should be unmodified from the input path.");
40-
41-
string result = collapsedString.Length == 0 ? PathInternal.DirectorySeparatorCharAsString : collapsedString;
42-
43-
return result;
28+
return GetFullPathInternal(path);
4429
}
4530

4631
public static string GetFullPath(string path, string basePath)
@@ -58,9 +43,33 @@ public static string GetFullPath(string path, string basePath)
5843
throw new ArgumentException(SR.Argument_InvalidPathChars);
5944

6045
if (IsPathFullyQualified(path))
61-
return GetFullPath(path);
46+
return GetFullPathInternal(path);
47+
48+
return GetFullPathInternal(CombineInternal(basePath, path));
49+
}
6250

63-
return GetFullPath(CombineInternal(basePath, path));
51+
// Gets the full path without argument validation
52+
private static string GetFullPathInternal(string path)
53+
{
54+
Debug.Assert(!string.IsNullOrEmpty(path));
55+
Debug.Assert(!path.Contains('\0'));
56+
57+
// Expand with current directory if necessary
58+
if (!IsPathRooted(path))
59+
{
60+
path = Combine(Interop.Sys.GetCwd(), path);
61+
}
62+
63+
// We would ideally use realpath to do this, but it resolves symlinks, requires that the file actually exist,
64+
// and turns it into a full path, which we only want if fullCheck is true.
65+
string collapsedString = PathInternal.RemoveRelativeSegments(path, PathInternal.GetRootLength(path));
66+
67+
Debug.Assert(collapsedString.Length < path.Length || collapsedString.ToString() == path,
68+
"Either we've removed characters, or the string should be unmodified from the input path.");
69+
70+
string result = collapsedString.Length == 0 ? PathInternal.DirectorySeparatorCharAsString : collapsedString;
71+
72+
return result;
6473
}
6574

6675
private static string RemoveLongPathPrefix(string path)

src/libraries/System.Private.CoreLib/src/System/IO/Path.Windows.cs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public static string GetFullPath(string path)
5050
if (path.Contains('\0'))
5151
throw new ArgumentException(SR.Argument_InvalidPathChars, nameof(path));
5252

53-
return GetFullyQualifiedPath(path);
53+
return GetFullPathInternal(path);
5454
}
5555

5656
public static string GetFullPath(string path, string basePath)
@@ -68,7 +68,7 @@ public static string GetFullPath(string path, string basePath)
6868
throw new ArgumentException(SR.Argument_InvalidPathChars);
6969

7070
if (IsPathFullyQualified(path))
71-
return GetFullyQualifiedPath(path);
71+
return GetFullPathInternal(path);
7272

7373
if (PathInternal.IsEffectivelyEmpty(path.AsSpan()))
7474
return basePath;
@@ -120,11 +120,15 @@ public static string GetFullPath(string path, string basePath)
120120

121121
return PathInternal.IsDevice(combinedPath.AsSpan())
122122
? PathInternal.RemoveRelativeSegments(combinedPath, PathInternal.GetRootLength(combinedPath.AsSpan()))
123-
: GetFullyQualifiedPath(combinedPath);
123+
: GetFullPathInternal(combinedPath);
124124
}
125125

126-
internal static string GetFullyQualifiedPath(string path)
126+
// Gets the full path without argument validation
127+
private static string GetFullPathInternal(string path)
127128
{
129+
Debug.Assert(!string.IsNullOrEmpty(path));
130+
Debug.Assert(!path.Contains('\0'));
131+
128132
if (PathInternal.IsExtended(path.AsSpan()))
129133
{
130134
// \\?\ paths are considered normalized by definition. Windows doesn't normalize \\?\

0 commit comments

Comments
 (0)