Fix Path.GetPathRoot wrong check in Unix (#35734)

* Fix Path.GetPathRoot wrong check in Unix * Return .Empty instead of null * Update src/libraries/System.Private.CoreLib/src/System/IO/Path.Unix.cs Co-authored-by: Jan Kotas <[email protected]> * Address unit test comments Co-authored-by: Jan Kotas <[email protected]>
dotnet · May 13, 2020 · 61c6581 · 61c6581
1 parent 9386d50
commit 61c6581
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 5 deletions.
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
@@ -122,13 +122,12 @@ public static bool IsPathRooted(ReadOnlySpan<char> path)
         public static string? GetPathRoot(string? path)
         {
             if (PathInternal.IsEffectivelyEmpty(path)) return null;
-
             return IsPathRooted(path) ? PathInternal.DirectorySeparatorCharAsString : string.Empty;
         }
 
         public static ReadOnlySpan<char> GetPathRoot(ReadOnlySpan<char> path)
         {
-            return PathInternal.IsEffectivelyEmpty(path) && IsPathRooted(path) ? PathInternal.DirectorySeparatorCharAsString.AsSpan() : ReadOnlySpan<char>.Empty;
+            return IsPathRooted(path) ? PathInternal.DirectorySeparatorCharAsString.AsSpan() : ReadOnlySpan<char>.Empty;
         }
 
         /// <summary>Gets whether the system is case-sensitive.</summary>

diff --git a/src/libraries/System.Runtime.Extensions/tests/System/IO/PathTests.cs b/src/libraries/System.Runtime.Extensions/tests/System/IO/PathTests.cs
@@ -52,6 +52,7 @@ public void GetDirectoryName_CurrentDirectory()
             Assert.Equal(curDir, Path.GetDirectoryName(Path.Combine(curDir, "baz")));
 
             Assert.Null(Path.GetDirectoryName(Path.GetPathRoot(curDir)));
+            Assert.True(Path.GetDirectoryName(Path.GetPathRoot(curDir.AsSpan())).IsEmpty);
         }
 
         [Fact]
@@ -108,10 +109,16 @@ public void GetPathRoot_Null()
         public void GetPathRoot_Basic()
         {
             string cwd = Directory.GetCurrentDirectory();
-            Assert.Equal(cwd.Substring(0, cwd.IndexOf(Path.DirectorySeparatorChar) + 1), Path.GetPathRoot(cwd));
+            string substring = cwd.Substring(0, cwd.IndexOf(Path.DirectorySeparatorChar) + 1);
+
+            Assert.Equal(substring, Path.GetPathRoot(cwd));
+            PathAssert.Equal(substring.AsSpan(), Path.GetPathRoot(cwd.AsSpan()));
+
             Assert.True(Path.IsPathRooted(cwd));
 
             Assert.Equal(string.Empty, Path.GetPathRoot(@"file.exe"));
+            Assert.True(Path.GetPathRoot(@"file.exe".AsSpan()).IsEmpty);
+
             Assert.False(Path.IsPathRooted("file.exe"));
         }
 
@@ -417,6 +424,7 @@ public void GetInvalidPathChars()
                     Assert.EndsWith(bad, Path.GetFullPath(bad));
                 }
                 Assert.Equal(string.Empty, Path.GetPathRoot(bad));
+                Assert.True(Path.GetPathRoot(bad.AsSpan()).IsEmpty);
                 Assert.False(Path.IsPathRooted(bad));
             });
         }
@@ -431,7 +439,7 @@ public void GetInvalidPathChars_Span()
                 Assert.Equal(string.Empty, new string(Path.GetExtension(bad.AsSpan())));
                 Assert.Equal(bad, new string(Path.GetFileName(bad.AsSpan())));
                 Assert.Equal(bad, new string(Path.GetFileNameWithoutExtension(bad.AsSpan())));
-                Assert.Equal(string.Empty, new string(Path.GetPathRoot(bad.AsSpan())));
+                Assert.True(Path.GetPathRoot(bad.AsSpan()).IsEmpty);
                 Assert.False(Path.IsPathRooted(bad.AsSpan()));
             });
         }

diff --git a/src/libraries/System.Runtime.Extensions/tests/System/IO/PathTests_Unix.cs b/src/libraries/System.Runtime.Extensions/tests/System/IO/PathTests_Unix.cs
@@ -19,6 +19,7 @@ public static void GetPathRoot(string value, string expected)
             // UNCs and device paths have no special meaning in Unix
             _ = expected;
             Assert.Empty(Path.GetPathRoot(value));
+            Assert.True(Path.GetPathRoot(value.AsSpan()).IsEmpty);
         }
 
         [Theory,
@@ -115,13 +116,44 @@ public void GetFullPath_Unix_Whitespace()
             { @"../../../tmp/bar/..", @"/home/git", @"/tmp" },
             { @"../../././tmp/..", @"/home/git", @"/" },
             { @"././../../../", @"/home/git", @"/" },
+            { @"../tmp/../..", @"/home", @"/" }
         };
 
         [Theory,
            MemberData(nameof(GetFullPath_BasePath_BasicExpansions_TestData_Unix))]
         public static void GetFullPath_BasicExpansions_Unix(string path, string basePath, string expected)
         {
-            Assert.Equal(expected, Path.GetFullPath(path, basePath));
+            string fullPath = Path.GetFullPath(path, basePath);
+            Assert.Equal(expected, fullPath);
+        }
+
+        [Theory]
+        [InlineData(@"/../../.././tmp/..")]
+        [InlineData(@"/../../../")]
+        [InlineData(@"/../../../tmp/bar/..")]
+        [InlineData(@"/../.././././bar/../../../")]
+        [InlineData(@"/../../././tmp/..")]
+        [InlineData(@"/../../tmp/../../")]
+        [InlineData(@"/../../tmp/bar/..")]
+        [InlineData(@"/../tmp/../..")]
+        [InlineData(@"/././../../../../")]
+        [InlineData(@"/././../../../")]
+        [InlineData(@"/./././bar/../../../")]
+        [InlineData(@"/")]
+        [InlineData(@"/bar")]
+        [InlineData(@"/bar/././././../../..")]
+        [InlineData(@"/bar/tmp")]
+        [InlineData(@"/tmp/..")]
+        [InlineData(@"/tmp/../../../../../bar")]
+        [InlineData(@"/tmp/../../../bar")]
+        [InlineData(@"/tmp/../bar/../..")]
+        [InlineData(@"/tmp/bar")]
+        [InlineData(@"/tmp/bar/..")]
+        public static void GePathRoot_Unix(string path)
+        {
+            string expected = @"/";
+            Assert.Equal(expected, Path.GetPathRoot(path));
+            PathAssert.Equal(expected.AsSpan(), Path.GetPathRoot(path.AsSpan()));
         }
 
         [Fact]