From 1374724159f82baf45a0abcc6b8a2d4e436afb1c Mon Sep 17 00:00:00 2001 From: Roman Konecny Date: Wed, 22 Jan 2025 17:11:25 +0100 Subject: [PATCH 1/4] Handle unicode in absolute URI path for combine. --- .../System.Private.Uri/src/System/UriExt.cs | 47 ++++++++++++------- .../tests/FunctionalTests/UriTests.cs | 35 ++++++++++++++ 2 files changed, 64 insertions(+), 18 deletions(-) diff --git a/src/libraries/System.Private.Uri/src/System/UriExt.cs b/src/libraries/System.Private.Uri/src/System/UriExt.cs index ee35d01e96232e..881a0e0ea8b939 100644 --- a/src/libraries/System.Private.Uri/src/System/UriExt.cs +++ b/src/libraries/System.Private.Uri/src/System/UriExt.cs @@ -1053,30 +1053,41 @@ private void CreateThisFromUri(Uri otherUri) { DebugAssertInCtor(); - // Clone the other URI but develop own UriInfo member - _info = null!; - _flags = otherUri._flags; - if (InFact(Flags.MinimalUriInfoSet)) + + if (InFact(Flags.AllUriInfoSet)) + { + // We can share it now without mutation concern, for since AllUriInfoSet it is immutable. + _info = otherUri._info; + } + else { - _flags &= ~(Flags.MinimalUriInfoSet | Flags.AllUriInfoSet | Flags.IndexMask); - // Port / Path offset - int portIndex = otherUri._info.Offset.Path; - if (InFact(Flags.NotDefaultPort)) + // Clone the other URI but develop own UriInfo member + // We cannot just reference otherUri._info as this UriInfo will be mutated later + // which could be happening concurrently and in a not thread safe manner. + _info = null!; + + if (InFact(Flags.MinimalUriInfoSet)) { - // Find the start of the port. Account for non-canonical ports like :00123 - while (otherUri._string[portIndex] != ':' && portIndex > otherUri._info.Offset.Host) + _flags &= ~(Flags.MinimalUriInfoSet | Flags.AllUriInfoSet | Flags.IndexMask); + // Port / Path offset + int portIndex = otherUri._info.Offset.Path; + if (InFact(Flags.NotDefaultPort)) { - portIndex--; - } - if (otherUri._string[portIndex] != ':') - { - // Something wrong with the NotDefaultPort flag. Reset to path index - Debug.Fail("Uri failed to locate custom port at index: " + portIndex); - portIndex = otherUri._info.Offset.Path; + // Find the start of the port. Account for non-canonical ports like :00123 + while (otherUri._string[portIndex] != ':' && portIndex > otherUri._info.Offset.Host) + { + portIndex--; + } + if (otherUri._string[portIndex] != ':') + { + // Something wrong with the NotDefaultPort flag. Reset to path index + Debug.Fail("Uri failed to locate custom port at index: " + portIndex); + portIndex = otherUri._info.Offset.Path; + } } + _flags |= (Flags)portIndex; // Port or path } - _flags |= (Flags)portIndex; // Port or path } _syntax = otherUri._syntax; diff --git a/src/libraries/System.Private.Uri/tests/FunctionalTests/UriTests.cs b/src/libraries/System.Private.Uri/tests/FunctionalTests/UriTests.cs index 7b9bf9d1d4ec6f..df44109d7932f1 100644 --- a/src/libraries/System.Private.Uri/tests/FunctionalTests/UriTests.cs +++ b/src/libraries/System.Private.Uri/tests/FunctionalTests/UriTests.cs @@ -729,6 +729,41 @@ public static void Uri_CombineUsesNewUriString() Assert.Equal(Combined, new Uri(baseUri, RelativeUriString).AbsoluteUri); } + [Theory] + [InlineData("http://bar/Testue/testImage.jpg")] + // Tests that internal Uri info were properly applied during a Combine operation when URI contains non-ascii character. + [InlineData("http://bar/Testü/testImage.jpg")] + public static void Uri_CombineWithAbsoluteHttpUriResultInAbsoluteHttpSchema(string fileUri) + { + string baseUriString = "combine-scheme://foo"; + + var baseUri = new Uri(baseUriString, UriKind.Absolute); + var uri = new Uri(fileUri); + var resultUri = new Uri(baseUri, uri); + string resultUriString = resultUri.ToString(); + + Assert.DoesNotContain(baseUriString, resultUriString); + Assert.Contains("http://", resultUriString); + Assert.Equal(fileUri, resultUriString); + } + + [Theory] + [InlineData(@"\\nas\Testue\testImage.jpg")] + // Tests that internal Uri info were properly applied during a Combine operation when URI contains non-ascii character. + [InlineData(@"\\nas\Testü\testImage.jpg")] + public static void Uri_CombineWithAbsoluteFilePathResultInAbsoluteFileSchema(string fileUri) + { + string baseUriString = "combine-scheme://foo"; + + var baseUri = new Uri(baseUriString, UriKind.Absolute); + var uri = new Uri(fileUri); + var resultUri = new Uri(baseUri, uri); + string resultUriString = resultUri.ToString(); + + Assert.DoesNotContain(baseUriString, resultUriString); + Assert.Contains("file://", resultUriString); + } + [Fact] public static void Uri_CachesIdnHost() { From 00242ed34e8498f5f48c44e17821e949057d5b8d Mon Sep 17 00:00:00 2001 From: Roman Konecny Date: Fri, 24 Jan 2025 16:42:32 +0100 Subject: [PATCH 2/4] Adding debug assert Co-authored-by: Miha Zupan --- src/libraries/System.Private.Uri/src/System/UriExt.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/System.Private.Uri/src/System/UriExt.cs b/src/libraries/System.Private.Uri/src/System/UriExt.cs index 881a0e0ea8b939..34137971008c42 100644 --- a/src/libraries/System.Private.Uri/src/System/UriExt.cs +++ b/src/libraries/System.Private.Uri/src/System/UriExt.cs @@ -1062,6 +1062,7 @@ private void CreateThisFromUri(Uri otherUri) } else { + Debug.Assert(!InFact(Flags.HasUnicode)); // Clone the other URI but develop own UriInfo member // We cannot just reference otherUri._info as this UriInfo will be mutated later // which could be happening concurrently and in a not thread safe manner. From a59ca7bb0d9d350b5cca26df7757c64eb8d6c763 Mon Sep 17 00:00:00 2001 From: Roman Konecny Date: Fri, 24 Jan 2025 20:49:51 +0100 Subject: [PATCH 3/4] Fakactor unit tests. Change assert --- .../System.Private.Uri/src/System/UriExt.cs | 2 +- .../tests/FunctionalTests/UriTests.cs | 35 ++++++------------- 2 files changed, 11 insertions(+), 26 deletions(-) diff --git a/src/libraries/System.Private.Uri/src/System/UriExt.cs b/src/libraries/System.Private.Uri/src/System/UriExt.cs index 34137971008c42..5afdef39335386 100644 --- a/src/libraries/System.Private.Uri/src/System/UriExt.cs +++ b/src/libraries/System.Private.Uri/src/System/UriExt.cs @@ -1062,7 +1062,7 @@ private void CreateThisFromUri(Uri otherUri) } else { - Debug.Assert(!InFact(Flags.HasUnicode)); + Debug.Assert(!InFact(Flags.HasUnicode) || otherUri.IsNotAbsoluteUri); // Clone the other URI but develop own UriInfo member // We cannot just reference otherUri._info as this UriInfo will be mutated later // which could be happening concurrently and in a not thread safe manner. diff --git a/src/libraries/System.Private.Uri/tests/FunctionalTests/UriTests.cs b/src/libraries/System.Private.Uri/tests/FunctionalTests/UriTests.cs index df44109d7932f1..2a98606f32441c 100644 --- a/src/libraries/System.Private.Uri/tests/FunctionalTests/UriTests.cs +++ b/src/libraries/System.Private.Uri/tests/FunctionalTests/UriTests.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.Threading; using System.Threading.Tasks; @@ -730,38 +731,22 @@ public static void Uri_CombineUsesNewUriString() } [Theory] - [InlineData("http://bar/Testue/testImage.jpg")] + [InlineData("http://bar/Testue/testImage.jpg", "http://bar/Testue/testImage.jpg", "http://bar/Testue/testImage.jpg", "bar")] + [InlineData(@"\\nas\Testue\testImage.jpg", @"file://nas/Testue/testImage.jpg", @"file://nas/Testue/testImage.jpg", "nas")] // Tests that internal Uri info were properly applied during a Combine operation when URI contains non-ascii character. - [InlineData("http://bar/Testü/testImage.jpg")] - public static void Uri_CombineWithAbsoluteHttpUriResultInAbsoluteHttpSchema(string fileUri) + [InlineData("http://bar/Testü/testImage.jpg", "http://bar/Testü/testImage.jpg", "http://bar/Test%C3%BC/testImage.jpg", "bar")] + [InlineData(@"\\nas\Testü\testImage.jpg", @"file://nas/Testü/testImage.jpg", @"file://nas/Test%C3%BC/testImage.jpg", "nas")] + public static void Uri_CombineWithAbsoluteUriResultInAbsoluteSchemaIgnoringOriginalBase(string relativeUri, string expectedUri, string expectedAbsoluteUri, string expectedHost) { string baseUriString = "combine-scheme://foo"; var baseUri = new Uri(baseUriString, UriKind.Absolute); - var uri = new Uri(fileUri); + var uri = new Uri(relativeUri); var resultUri = new Uri(baseUri, uri); - string resultUriString = resultUri.ToString(); - Assert.DoesNotContain(baseUriString, resultUriString); - Assert.Contains("http://", resultUriString); - Assert.Equal(fileUri, resultUriString); - } - - [Theory] - [InlineData(@"\\nas\Testue\testImage.jpg")] - // Tests that internal Uri info were properly applied during a Combine operation when URI contains non-ascii character. - [InlineData(@"\\nas\Testü\testImage.jpg")] - public static void Uri_CombineWithAbsoluteFilePathResultInAbsoluteFileSchema(string fileUri) - { - string baseUriString = "combine-scheme://foo"; - - var baseUri = new Uri(baseUriString, UriKind.Absolute); - var uri = new Uri(fileUri); - var resultUri = new Uri(baseUri, uri); - string resultUriString = resultUri.ToString(); - - Assert.DoesNotContain(baseUriString, resultUriString); - Assert.Contains("file://", resultUriString); + Assert.Equal(expectedUri, resultUri.ToString()); + Assert.Equal(expectedAbsoluteUri, resultUri.AbsoluteUri); + Assert.Equal(expectedHost, resultUri.Host); } [Fact] From 4b77d6b79d2bbf0511f5f3ddb8cf210d5ef7708f Mon Sep 17 00:00:00 2001 From: Roman Konecny Date: Mon, 27 Jan 2025 17:55:55 +0100 Subject: [PATCH 4/4] Replace unicode chars in literals by C# \u escapes --- .../System.Private.Uri/tests/FunctionalTests/UriTests.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/System.Private.Uri/tests/FunctionalTests/UriTests.cs b/src/libraries/System.Private.Uri/tests/FunctionalTests/UriTests.cs index 2a98606f32441c..77127e35d0b3d4 100644 --- a/src/libraries/System.Private.Uri/tests/FunctionalTests/UriTests.cs +++ b/src/libraries/System.Private.Uri/tests/FunctionalTests/UriTests.cs @@ -732,10 +732,10 @@ public static void Uri_CombineUsesNewUriString() [Theory] [InlineData("http://bar/Testue/testImage.jpg", "http://bar/Testue/testImage.jpg", "http://bar/Testue/testImage.jpg", "bar")] - [InlineData(@"\\nas\Testue\testImage.jpg", @"file://nas/Testue/testImage.jpg", @"file://nas/Testue/testImage.jpg", "nas")] + [InlineData(@"\\nas\Testue\testImage.jpg", "file://nas/Testue/testImage.jpg", "file://nas/Testue/testImage.jpg", "nas")] // Tests that internal Uri info were properly applied during a Combine operation when URI contains non-ascii character. - [InlineData("http://bar/Testü/testImage.jpg", "http://bar/Testü/testImage.jpg", "http://bar/Test%C3%BC/testImage.jpg", "bar")] - [InlineData(@"\\nas\Testü\testImage.jpg", @"file://nas/Testü/testImage.jpg", @"file://nas/Test%C3%BC/testImage.jpg", "nas")] + [InlineData("http://bar/Test\u00fc/testImage.jpg", "http://bar/Test\u00fc/testImage.jpg", "http://bar/Test%C3%BC/testImage.jpg", "bar")] + [InlineData("\\\\nas\\Test\u00fc\\testImage.jpg", "file://nas/Test\u00fc/testImage.jpg", "file://nas/Test%C3%BC/testImage.jpg", "nas")] public static void Uri_CombineWithAbsoluteUriResultInAbsoluteSchemaIgnoringOriginalBase(string relativeUri, string expectedUri, string expectedAbsoluteUri, string expectedHost) { string baseUriString = "combine-scheme://foo";