From f18d2c2fbac08745c05caac1de48d8a45e9d28db Mon Sep 17 00:00:00 2001 From: MihaZupan Date: Thu, 14 May 2020 04:50:33 +0200 Subject: [PATCH 1/4] Fix non-ascii absolute file path Uri handling --- .../System.Private.Uri/src/System/Uri.cs | 55 ++++--------------- .../tests/FunctionalTests/UriTests.cs | 53 ++++++++++++++++++ 2 files changed, 65 insertions(+), 43 deletions(-) diff --git a/src/libraries/System.Private.Uri/src/System/Uri.cs b/src/libraries/System.Private.Uri/src/System/Uri.cs index 0755daca458aa..3a7712b6aaa90 100644 --- a/src/libraries/System.Private.Uri/src/System/Uri.cs +++ b/src/libraries/System.Private.Uri/src/System/Uri.cs @@ -3262,16 +3262,17 @@ private unsafe void ParseRemaining() // For iri parsing if we found unicode the idx has offset into _originalUnicodeString.. // so restart parsing from there and make _info.Offset.Path as _string.Length - idx = _info.Offset.Path; - origIdx = _info.Offset.Path; + idx = origIdx = _info.Offset.Path; //Some uris do not have a query // When '?' is passed as delimiter, then it's special case // 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) { @@ -3314,19 +3315,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) { @@ -3445,6 +3434,8 @@ private unsafe void ParseRemaining() // if (buildIriStringFromPath) { + DebugAssertInCtor(); + int offset = origIdx; if (origIdx < _originalUnicodeString.Length && _originalUnicodeString[origIdx] == '?') @@ -3460,19 +3451,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) { @@ -3521,25 +3500,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) { diff --git a/src/libraries/System.Private.Uri/tests/FunctionalTests/UriTests.cs b/src/libraries/System.Private.Uri/tests/FunctionalTests/UriTests.cs index 6317c87b2942b..5de9eb1a8a631 100644 --- a/src/libraries/System.Private.Uri/tests/FunctionalTests/UriTests.cs +++ b/src/libraries/System.Private.Uri/tests/FunctionalTests/UriTests.cs @@ -738,5 +738,58 @@ public static void Uri_DoesNotLockOnString() Assert.True(Monitor.TryEnter(uriString, TimeSpan.FromSeconds(10))); Assert.False(timedOut); } + + public static IEnumerable FilePathHandlesNonAscii_TestData() + { + if (PlatformDetection.IsNotWindows) + { + // Unix absolute file path + yield return new object[] { "/\u00FCri/", "/\u00FCri/", "/%C3%BCri/", "file:///%C3%BCri/", "/\u00FCri/" }; + yield return new object[] { "/a/b\uD83D\uDE1F/Foo.cs", "/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/", "/a/?b/c\u00FC/", "/a/?b/c%C3%BC/", "file:///a/?b/c%C3%BC/", "/a/?b/c\u00FC/" }; + yield return new object[] { "/a/#b/c\u00FC/", "/a/#b/c\u00FC/", "/a/#b/c%C3%BC/", "file:///a/#b/c%C3%BC/", "/a/#b/c\u00FC/" }; + yield return new object[] { "/a/?b/#c/d\u00FC/", "/a/#b/c\u00FC/", "/a/?b/#c/d%C3%BC/", "file:///a/?b/#c/d%C3%BC/", "/a/?b/#c/d\u00FC/" }; + + yield return new object[] { "file:///a/?b/c\u00FC/", "file:///a/?b/c\u00FC/", "/a/?b/c%C3%BC/", "file:///a/?b/c%C3%BC/", "/a/?b/c\u00FC/" }; + yield return new object[] { "file:///a/#b/c\u00FC/", "file:///a/#b/c\u00FC/", "/a/#b/c%C3%BC/", "file:///a/#b/c%C3%BC/", "/a/#b/c\u00FC/" }; + yield return new object[] { "file:///a/?b/#c/d\u00FC/", "file:///a/?b/#c/d\u00FC/", "/a/?b/#c/d%C3%BC/", "file:///a/?b/#c/d%C3%BC/", "/a/?b/#c/d\u00FC/" }; + } + } + + [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); + } } } From 91d91d1d06b2d5fc3b2ed550b1360040c74ad7e5 Mon Sep 17 00:00:00 2001 From: MihaZupan Date: Thu, 14 May 2020 07:48:55 +0200 Subject: [PATCH 2/4] Fix Unix tests --- .../tests/FunctionalTests/UriTests.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/libraries/System.Private.Uri/tests/FunctionalTests/UriTests.cs b/src/libraries/System.Private.Uri/tests/FunctionalTests/UriTests.cs index 5de9eb1a8a631..703f60656d906 100644 --- a/src/libraries/System.Private.Uri/tests/FunctionalTests/UriTests.cs +++ b/src/libraries/System.Private.Uri/tests/FunctionalTests/UriTests.cs @@ -744,8 +744,8 @@ public static IEnumerable FilePathHandlesNonAscii_TestData() if (PlatformDetection.IsNotWindows) { // Unix absolute file path - yield return new object[] { "/\u00FCri/", "/\u00FCri/", "/%C3%BCri/", "file:///%C3%BCri/", "/\u00FCri/" }; - yield return new object[] { "/a/b\uD83D\uDE1F/Foo.cs", "/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" }; + 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 @@ -770,13 +770,13 @@ public static IEnumerable FilePathHandlesNonAscii_TestData() } else { - yield return new object[] { "/a/?b/c\u00FC/", "/a/?b/c\u00FC/", "/a/?b/c%C3%BC/", "file:///a/?b/c%C3%BC/", "/a/?b/c\u00FC/" }; - yield return new object[] { "/a/#b/c\u00FC/", "/a/#b/c\u00FC/", "/a/#b/c%C3%BC/", "file:///a/#b/c%C3%BC/", "/a/#b/c\u00FC/" }; - yield return new object[] { "/a/?b/#c/d\u00FC/", "/a/#b/c\u00FC/", "/a/?b/#c/d%C3%BC/", "file:///a/?b/#c/d%C3%BC/", "/a/?b/#c/d\u00FC/" }; + 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/?b/c%C3%BC/", "file:///a/?b/c%C3%BC/", "/a/?b/c\u00FC/" }; - yield return new object[] { "file:///a/#b/c\u00FC/", "file:///a/#b/c\u00FC/", "/a/#b/c%C3%BC/", "file:///a/#b/c%C3%BC/", "/a/#b/c\u00FC/" }; - yield return new object[] { "file:///a/?b/#c/d\u00FC/", "file:///a/?b/#c/d\u00FC/", "/a/?b/#c/d%C3%BC/", "file:///a/?b/#c/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/" }; } } From 32a4b8bda75e25b37498c9cf762acfe8bdb08d1a Mon Sep 17 00:00:00 2001 From: MihaZupan Date: Thu, 14 May 2020 14:19:25 +0200 Subject: [PATCH 3/4] Revert unrelated style change --- src/libraries/System.Private.Uri/src/System/Uri.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Private.Uri/src/System/Uri.cs b/src/libraries/System.Private.Uri/src/System/Uri.cs index 3a7712b6aaa90..1e81ec9c1fddf 100644 --- a/src/libraries/System.Private.Uri/src/System/Uri.cs +++ b/src/libraries/System.Private.Uri/src/System/Uri.cs @@ -3262,7 +3262,8 @@ private unsafe void ParseRemaining() // For iri parsing if we found unicode the idx has offset into _originalUnicodeString.. // so restart parsing from there and make _info.Offset.Path as _string.Length - idx = origIdx = _info.Offset.Path; + idx = _info.Offset.Path; + origIdx = _info.Offset.Path; //Some uris do not have a query // When '?' is passed as delimiter, then it's special case From 49afca21e8471ddc9a01c19d0e6e6be3cf4c7f1c Mon Sep 17 00:00:00 2001 From: MihaZupan Date: Mon, 18 May 2020 17:59:39 +0200 Subject: [PATCH 4/4] Add asserts to FilePathHandlesNonAscii to test AbsoluteUri roundtrips properties --- .../System.Private.Uri/tests/FunctionalTests/UriTests.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/libraries/System.Private.Uri/tests/FunctionalTests/UriTests.cs b/src/libraries/System.Private.Uri/tests/FunctionalTests/UriTests.cs index 703f60656d906..b8670afd64d83 100644 --- a/src/libraries/System.Private.Uri/tests/FunctionalTests/UriTests.cs +++ b/src/libraries/System.Private.Uri/tests/FunctionalTests/UriTests.cs @@ -790,6 +790,13 @@ public static void FilePathHandlesNonAscii(string uriString, string toString, st 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); } } }