Skip to content

Commit

Permalink
Fix non-ascii absolute file path handling in Uri (#36429)
Browse files Browse the repository at this point in the history 
* Fix non-ascii absolute file path Uri handling

* Fix Unix tests

* Revert unrelated style change

* Add asserts to FilePathHandlesNonAscii to test AbsoluteUri roundtrips properties
  • Loading branch information
@MihaZupan
MihaZupan committed May 21, 2020
1 parent 4eaccd6 commit e0683a9
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 41 deletions.
52 changes: 11 additions & 41 deletions src/libraries/System.Private.Uri/src/System/Uri.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3219,8 +3219,10 @@ private unsafe void ParseRemaining()
// so both '?' and '#' will work as delimiters
if (buildIriStringFromPath)
{
// Dos paths have no host. Other schemes cleared/set _string with host information in PrivateParseMinimal.
if (IsDosPath)
DebugAssertInCtor();

// Dos/Unix paths have no host. Other schemes cleared/set _string with host information in PrivateParseMinimal.
if (IsFile && !IsUncPath)
{
if (IsImplicitFile)
{
Expand Down Expand Up @@ -3263,19 +3265,7 @@ private unsafe void ParseRemaining()
origIdx = index == -1 ? _originalUnicodeString.Length : (index + origIdx);
}

// Correctly escape unescape
string escapedPath = EscapeUnescapeIri(_originalUnicodeString, offset, origIdx, UriComponents.Path);

// Normalize path
try
{
_string += escapedPath;
}
catch (ArgumentException)
{
UriFormatException e = GetException(ParsingError.BadFormat)!;
throw e;
}
_string += EscapeUnescapeIri(_originalUnicodeString, offset, origIdx, UriComponents.Path);

if (_string.Length > ushort.MaxValue)
{
Expand Down Expand Up @@ -3394,6 +3384,8 @@ private unsafe void ParseRemaining()
//
if (buildIriStringFromPath)
{
DebugAssertInCtor();

int offset = origIdx;

if (origIdx < _originalUnicodeString.Length && _originalUnicodeString[origIdx] == '?')
Expand All @@ -3409,19 +3401,7 @@ private unsafe void ParseRemaining()
origIdx = _originalUnicodeString.Length;
}

// Correctly escape unescape
string escapedPath = EscapeUnescapeIri(_originalUnicodeString, offset, origIdx, UriComponents.Query);

// Normalize path
try
{
_string += escapedPath;
}
catch (ArgumentException)
{
UriFormatException e = GetException(ParsingError.BadFormat)!;
throw e;
}
_string += EscapeUnescapeIri(_originalUnicodeString, offset, origIdx, UriComponents.Query);

if (_string.Length > ushort.MaxValue)
{
Expand Down Expand Up @@ -3470,25 +3450,15 @@ private unsafe void ParseRemaining()
//
if (buildIriStringFromPath)
{
DebugAssertInCtor();

int offset = origIdx;

if (origIdx < _originalUnicodeString.Length && _originalUnicodeString[origIdx] == '#')
{
origIdx = _originalUnicodeString.Length;

// Correctly escape unescape
string escapedPath = EscapeUnescapeIri(_originalUnicodeString, offset, origIdx, UriComponents.Fragment);

// Normalize path
try
{
_string += escapedPath;
}
catch (ArgumentException)
{
UriFormatException e = GetException(ParsingError.BadFormat)!;
throw e;
}
_string += EscapeUnescapeIri(_originalUnicodeString, offset, origIdx, UriComponents.Fragment);

if (_string.Length > ushort.MaxValue)
{
Expand Down
60 changes: 60 additions & 0 deletions src/libraries/System.Private.Uri/tests/FunctionalTests/UriTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -759,5 +759,65 @@ public static void Uri_DoesNotLockOnString()
Assert.True(Monitor.TryEnter(uriString, TimeSpan.FromSeconds(10)));
Assert.False(timedOut);
}

public static IEnumerable<object[]> FilePathHandlesNonAscii_TestData()
{
if (PlatformDetection.IsNotWindows)
{
// Unix absolute file path
yield return new object[] { "/\u00FCri/", "file:///\u00FCri/", "/%C3%BCri/", "file:///%C3%BCri/", "/\u00FCri/" };
yield return new object[] { "/a/b\uD83D\uDE1F/Foo.cs", "file:///a/b\uD83D\uDE1F/Foo.cs", "/a/b%F0%9F%98%9F/Foo.cs", "file:///a/b%F0%9F%98%9F/Foo.cs", "/a/b\uD83D\uDE1F/Foo.cs" };
}

// Absolute fie path
yield return new object[] { "file:///\u00FCri/", "file:///\u00FCri/", "/%C3%BCri/", "file:///%C3%BCri/", "/\u00FCri/" };
yield return new object[] { "file:///a/b\uD83D\uDE1F/Foo.cs", "file:///a/b\uD83D\uDE1F/Foo.cs", "/a/b%F0%9F%98%9F/Foo.cs", "file:///a/b%F0%9F%98%9F/Foo.cs", "/a/b\uD83D\uDE1F/Foo.cs" };

// DOS
yield return new object[] { "file://C:/\u00FCri/", "file:///C:/\u00FCri/", "C:/%C3%BCri/", "file:///C:/%C3%BCri/", "C:\\\u00FCri\\" };
yield return new object[] { "file:///C:/\u00FCri/", "file:///C:/\u00FCri/", "C:/%C3%BCri/", "file:///C:/%C3%BCri/", "C:\\\u00FCri\\" };
yield return new object[] { "C:/\u00FCri/", "file:///C:/\u00FCri/", "C:/%C3%BCri/", "file:///C:/%C3%BCri/", "C:\\\u00FCri\\" };

// UNC
yield return new object[] { "\\\\\u00FCri/", "file://\u00FCri/", "/", "file://\u00FCri/", "\\\\\u00FCri\\" };
yield return new object[] { "file://\u00FCri/", "file://\u00FCri/", "/", "file://\u00FCri/", "\\\\\u00FCri\\" };

// ? and # handling
if (PlatformDetection.IsWindows)
{
yield return new object[] { "file:///a/?b/c\u00FC/", "file:///a/?b/c\u00FC/", "/a/", "file:///a/?b/c%C3%BC/", "/a/" };
yield return new object[] { "file:///a/#b/c\u00FC/", "file:///a/#b/c\u00FC/", "/a/", "file:///a/#b/c%C3%BC/", "/a/" };
yield return new object[] { "file:///a/?b/#c/d\u00FC/", "file:///a/?b/#c/d\u00FC/", "/a/", "file:///a/?b/#c/d%C3%BC/", "/a/" };
}
else
{
yield return new object[] { "/a/?b/c\u00FC/", "file:///a/%3Fb/c\u00FC/", "/a/%3Fb/c%C3%BC/", "file:///a/%3Fb/c%C3%BC/", "/a/?b/c\u00FC/" };
yield return new object[] { "/a/#b/c\u00FC/", "file:///a/%23b/c\u00FC/", "/a/%23b/c%C3%BC/", "file:///a/%23b/c%C3%BC/", "/a/#b/c\u00FC/" };
yield return new object[] { "/a/?b/#c/d\u00FC/", "file:///a/%3Fb/%23c/d\u00FC/", "/a/%3Fb/%23c/d%C3%BC/", "file:///a/%3Fb/%23c/d%C3%BC/", "/a/?b/#c/d\u00FC/" };

yield return new object[] { "file:///a/?b/c\u00FC/", "file:///a/?b/c\u00FC/", "/a/", "file:///a/?b/c%C3%BC/", "/a/" };
yield return new object[] { "file:///a/#b/c\u00FC/", "file:///a/#b/c\u00FC/", "/a/", "file:///a/#b/c%C3%BC/", "/a/" };
yield return new object[] { "file:///a/?b/#c/d\u00FC/", "file:///a/?b/#c/d\u00FC/", "/a/", "file:///a/?b/#c/d%C3%BC/", "/a/" };
}
}

[Theory]
[MemberData(nameof(FilePathHandlesNonAscii_TestData))]
public static void FilePathHandlesNonAscii(string uriString, string toString, string absolutePath, string absoluteUri, string localPath)
{
var uri = new Uri(uriString);

Assert.Equal(toString, uri.ToString());
Assert.Equal(absolutePath, uri.AbsolutePath);
Assert.Equal(absoluteUri, uri.AbsoluteUri);
Assert.Equal(localPath, uri.LocalPath);

var uri2 = new Uri(uri.AbsoluteUri);

Assert.Equal(toString, uri2.ToString());
Assert.Equal(absolutePath, uri2.AbsolutePath);
Assert.Equal(absoluteUri, uri2.AbsoluteUri);
Assert.Equal(localPath, uri2.LocalPath);
}
}
}

0 comments on commit e0683a9

Please sign in to comment.