Different output in Unix between Path.GetPathRoot overloads for redundant segments

[carlossanlop]

While working on this issue (PR), I found a bug in the string overload for Path.GetPathRoot (Unix):

using System;
using System.IO;
namespace ConsoleAppCore
{
    class Program
    {
        static void Main()
        {
            string original = "/home/../folder/../..";
            string expected = "/";
            ReadOnlySpan<char> actualSpan = Path.GetPathRoot(original.AsSpan());
            string actualString = Path.GetPathRoot(original);
            Console.WriteLine($"Expected:      '{expected}'");
            Console.WriteLine($"Actual string: '{actualString}'");          // This returns '/'
            Console.WriteLine($"Actual span:   '{actualSpan.ToString()}'"); // This returns ''
        }
    }
}

I also found that running the same code in Windows will yield the behavior below, which is why I originally opened this issue, but @jkotas kindly pointed out it's expected behavior - the string method overload normalizes, while the span overload does not:

            Console.WriteLine($"Actual span:   '{actualSpan.ToString()}'"); // This returns '/'
            Console.WriteLine($"Actual string: '{actualString}'");          // This returns '\'

[ghost]

Tagging subscribers to this area: @jozkee
Notify danmosemsft if you want to be subscribed.

[carlossanlop]

Assigning to myself to investigate and fix since it's blocking my redundant segments PR.

[jkotas]

FWIW, this is by design. From https://docs.microsoft.com/en-us/dotnet/api/system.io.path.getpathroot?view=netcore-3.1#System_IO_Path_GetPathRoot_System_ReadOnlySpan_System_Char__ :

Unlike the string overload, this method doesn't normalize directory separators.

[carlossanlop]

Thanks, @jkotas. I assume the behavior is different because we wanted to prevent introducing a breaking change in the old string overload method, and the new method had the behavior corrected to prevent normalization. I found a couple of places in our own code where the string overload of GetPathRoot is being used to normalize a path.

[carlossanlop]

I'm reopening this issue because I reviewed my notes more closely and realized the actual bug is in Unix. I edited the original description with the details.

[carlossanlop]

Found the root cause - The IsEffectivelyEmpty check is wrong:

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

Lines 130 to 133 in 0a2f0e6

	public static ReadOnlySpan<char> GetPathRoot(ReadOnlySpan<char> path)
	{
	return PathInternal.IsEffectivelyEmpty(path) && IsPathRooted(path) ? PathInternal.DirectorySeparatorCharAsString.AsSpan() : ReadOnlySpan<char>.Empty;
	}

It should be inverted to match the same way we check for empty in the string overload:

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

Lines 123 to 128 in 0a2f0e6

	public static string? GetPathRoot(string? path)
	{
	if (PathInternal.IsEffectivelyEmpty(path)) return null;
	return IsPathRooted(path) ? PathInternal.DirectorySeparatorCharAsString : string.Empty;
	}

I also noticed we do not have unit tests verifying this method. I'll include this fix in my PR for removing redundant segments, which also adds unit tests for the case that helped me find this problem.

[jkotas]

I would be better to submit the fix for the bug and the associated test coverage in separate PR.