diff --git a/src/libraries/System.Private.Uri/src/System/Uri.cs b/src/libraries/System.Private.Uri/src/System/Uri.cs index 39a19d4f47b316..e188918eabb7ff 100644 --- a/src/libraries/System.Private.Uri/src/System/Uri.cs +++ b/src/libraries/System.Private.Uri/src/System/Uri.cs @@ -145,6 +145,8 @@ internal enum Flags : ulong [Conditional("DEBUG")] private void DebugSetLeftCtor() { + DebugAssertInCtor(); + _flags |= Flags.Debug_LeftConstructor; AssertInvariants(); @@ -424,7 +426,6 @@ public Uri([StringSyntax(StringSyntaxAttribute.Uri)] string uriString) ArgumentNullException.ThrowIfNull(uriString); CreateThis(uriString, false, UriKind.Absolute); - DebugSetLeftCtor(); } // @@ -438,7 +439,6 @@ public Uri([StringSyntax(StringSyntaxAttribute.Uri)] string uriString, bool dont ArgumentNullException.ThrowIfNull(uriString); CreateThis(uriString, dontEscape, UriKind.Absolute); - DebugSetLeftCtor(); } // @@ -456,7 +456,6 @@ public Uri(Uri baseUri, string? relativeUri, bool dontEscape) throw new ArgumentOutOfRangeException(nameof(baseUri)); CreateUri(baseUri, relativeUri, dontEscape); - DebugSetLeftCtor(); } // @@ -467,7 +466,6 @@ public Uri([StringSyntax(StringSyntaxAttribute.Uri, nameof(uriKind))] string uri ArgumentNullException.ThrowIfNull(uriString); CreateThis(uriString, false, uriKind); - DebugSetLeftCtor(); } /// @@ -480,7 +478,6 @@ public Uri([StringSyntax(StringSyntaxAttribute.Uri)] string uriString, in UriCre ArgumentNullException.ThrowIfNull(uriString); CreateThis(uriString, false, UriKind.Absolute, in creationOptions); - DebugSetLeftCtor(); } // @@ -498,7 +495,6 @@ public Uri(Uri baseUri, string? relativeUri) throw new ArgumentOutOfRangeException(nameof(baseUri)); CreateUri(baseUri, relativeUri, false); - DebugSetLeftCtor(); } // @@ -515,7 +511,6 @@ protected Uri(SerializationInfo serializationInfo, StreamingContext streamingCon if (uriString!.Length != 0) { CreateThis(uriString, false, UriKind.Absolute); - DebugSetLeftCtor(); return; } @@ -524,7 +519,6 @@ protected Uri(SerializationInfo serializationInfo, StreamingContext streamingCon throw new ArgumentException(SR.Format(SR.InvalidNullArgument, "RelativeUri"), nameof(serializationInfo)); CreateThis(uriString, false, UriKind.Relative); - DebugSetLeftCtor(); } // @@ -617,7 +611,6 @@ public Uri(Uri baseUri, Uri relativeUri) if (!ReferenceEquals(this, resolvedRelativeUri)) CreateThisFromUri(resolvedRelativeUri); - DebugSetLeftCtor(); return; } } @@ -634,7 +627,6 @@ public Uri(Uri baseUri, Uri relativeUri) _syntax = null!; _originalUnicodeString = null!; CreateThis(newUriString, dontEscape, UriKind.Absolute); - DebugSetLeftCtor(); } // diff --git a/src/libraries/System.Private.Uri/src/System/UriExt.cs b/src/libraries/System.Private.Uri/src/System/UriExt.cs index 0b623579a9efb5..d2eecb162f5b3e 100644 --- a/src/libraries/System.Private.Uri/src/System/UriExt.cs +++ b/src/libraries/System.Private.Uri/src/System/UriExt.cs @@ -13,11 +13,21 @@ namespace System { public partial class Uri { - // - // All public ctors go through here - // + /// Helper called by all constructors to construct the Uri. [MemberNotNull(nameof(_string))] private void CreateThis(string? uri, bool dontEscape, UriKind uriKind, in UriCreationOptions creationOptions = default) + { + UriFormatException? e = TryCreateThis(uri, dontEscape, uriKind, in creationOptions); + + if (e is not null) + { + throw e; + } + } + + /// Core helper called by all constructors and TryCreate factories to construct the Uri. + [MemberNotNull(nameof(_string))] + private UriFormatException? TryCreateThis(string? uri, bool dontEscape, UriKind uriKind, in UriCreationOptions creationOptions = default) { DebugAssertInCtor(); @@ -37,15 +47,6 @@ private void CreateThis(string? uri, bool dontEscape, UriKind uriKind, in UriCre _flags |= Flags.DisablePathAndQueryCanonicalization; ParsingError err = ParseScheme(_string, ref _flags, ref _syntax!); - - UriFormatException? e = InitializeUri(err, uriKind); - if (e != null) - throw e; - } - - private UriFormatException? InitializeUri(ParsingError err, UriKind uriKind) - { - DebugAssertInCtor(); Debug.Assert((err is ParsingError.None) == (_syntax is not null)); bool hasUnicode = false; @@ -63,19 +64,10 @@ private void CreateThis(string? uri, bool dontEscape, UriKind uriKind, in UriCre // and we'll allow relative Uri's, then create one. if (uriKind != UriKind.Absolute && err <= ParsingError.LastErrorOkayForRelativeUris) { - _flags &= Flags.UserEscaped | Flags.HasUnicode; // the only flags that makes sense for a relative uri - if (hasUnicode) - { - // Iri'ze and then normalize relative uris - var vsb = new ValueStringBuilder(stackalloc char[StackallocThreshold]); - IriHelper.EscapeUnescapeIri(ref vsb, _originalUnicodeString, isQuery: false); - _string = vsb.ToString(); - } - return null; + goto SwitchToRelativeUri; } // This is a fatal error based solely on scheme name parsing - _string = null!; // make it be invalid Uri return GetException(err); } @@ -85,9 +77,7 @@ private void CreateThis(string? uri, bool dontEscape, UriKind uriKind, in UriCre { if (uriKind == UriKind.Relative) { - _syntax = null!; // make it be relative Uri - _flags &= Flags.UserEscaped; // the only flag that makes sense for a relative uri - return null; + goto SwitchToRelativeUri; } // V1 compat @@ -98,9 +88,7 @@ private void CreateThis(string? uri, bool dontEscape, UriKind uriKind, in UriCre ((_string.Length >= 2 && (_string[0] != '\\' || _string[1] != '\\')) || (!OperatingSystem.IsWindows() && InFact(Flags.UnixPath)))) { - _syntax = null!; //make it be relative Uri - _flags &= Flags.UserEscaped; // the only flag that makes sense for a relative uri - return null; + goto SwitchToRelativeUri; // Otherwise an absolute file Uri wins when it's of the form "\\something" } } @@ -112,9 +100,7 @@ private void CreateThis(string? uri, bool dontEscape, UriKind uriKind, in UriCre if (uriKind != UriKind.Absolute && err <= ParsingError.LastErrorOkayForRelativeUris) { // RFC 3986 Section 5.4.2 - http:(relativeUri) may be considered a valid relative Uri. - _syntax = null!; // convert to relative uri - _flags &= Flags.UserEscaped; // the only flag that makes sense for a relative uri - return null; + goto SwitchToRelativeUri; } return GetException(err); @@ -164,6 +150,23 @@ private void CreateThis(string? uri, bool dontEscape, UriKind uriKind, in UriCre } // We have a valid absolute Uri. + DebugSetLeftCtor(); + return null; + + SwitchToRelativeUri: + Debug.Assert(uriKind != UriKind.Absolute); + + _syntax = null!; + _flags &= Flags.UserEscaped | Flags.HasUnicode; // the only flags that make sense for a relative uri + + if (hasUnicode) + { + var vsb = new ValueStringBuilder(stackalloc char[StackallocThreshold]); + IriHelper.EscapeUnescapeIri(ref vsb, _originalUnicodeString, isQuery: false); + _string = vsb.ToString(); + } + + DebugSetLeftCtor(); return null; } @@ -220,7 +223,6 @@ private static bool CheckForUnicodeOrEscapedUnreserved(string data) public static bool TryCreate([NotNullWhen(true), StringSyntax(StringSyntaxAttribute.Uri, "uriKind")] string? uriString, UriKind uriKind, [NotNullWhen(true)] out Uri? result) { result = CreateHelper(uriString, false, uriKind); - result?.DebugSetLeftCtor(); return result is not null; } @@ -234,7 +236,6 @@ public static bool TryCreate([NotNullWhen(true), StringSyntax(StringSyntaxAttrib public static bool TryCreate([NotNullWhen(true), StringSyntax(StringSyntaxAttribute.Uri)] string? uriString, in UriCreationOptions creationOptions, [NotNullWhen(true)] out Uri? result) { result = CreateHelper(uriString, false, UriKind.Absolute, in creationOptions); - result?.DebugSetLeftCtor(); return result is not null; } @@ -283,7 +284,6 @@ public static bool TryCreate(Uri? baseUri, Uri? relativeUri, [NotNullWhen(true)] result ??= CreateHelper(newUriString!, dontEscape, UriKind.Absolute); Debug.Assert(result is null || result.IsAbsoluteUri); - result?.DebugSetLeftCtor(); return result is not null; } @@ -625,24 +625,12 @@ public static string EscapeDataString(ReadOnlySpan charsToEscape) => public static bool TryEscapeDataString(ReadOnlySpan charsToEscape, Span destination, out int charsWritten) => UriHelper.TryEscapeDataString(charsToEscape, destination, out charsWritten); - // Should never be used except by the below method - private Uri(Flags flags, UriParser? uriParser, string uri) - { - _flags = flags; - _syntax = uriParser!; - _string = uri; - - if (uriParser is null) - { - // Relative Uris are fully initialized after the call to this constructor - // Absolute Uris will be initialized with a call to InitializeUri on the newly created instance - DebugSetLeftCtor(); - } - } +#pragma warning disable CS8618 // _string will be initialized by TryCreateThis later. + /// Must never be used except by . + private Uri() { } +#pragma warning restore CS8618 - // - // a Uri.TryCreate() method goes through here. - // + /// Called by TryCreate. internal static Uri? CreateHelper(string? uriString, bool dontEscape, UriKind uriKind, in UriCreationOptions creationOptions = default) { if (uriString is null) @@ -650,52 +638,15 @@ private Uri(Flags flags, UriParser? uriParser, string uri) return null; } - if (uriKind is < UriKind.RelativeOrAbsolute or > UriKind.Relative) - { - throw new ArgumentException(SR.Format(SR.net_uri_InvalidUriKind, uriKind)); - } - - UriParser? syntax = null; - Flags flags = Flags.Zero; - ParsingError err = ParseScheme(uriString, ref flags, ref syntax); - - if (dontEscape) - flags |= Flags.UserEscaped; - - if (creationOptions.DangerousDisablePathAndQueryCanonicalization) - flags |= Flags.DisablePathAndQueryCanonicalization; - - // We won't use User factory for these errors - if (err != ParsingError.None) - { - // If it looks as a relative Uri, custom factory is ignored - if (uriKind != UriKind.Absolute && err <= ParsingError.LastErrorOkayForRelativeUris) - return new Uri((flags & Flags.UserEscaped), null, uriString); - - return null; - } - - // Cannot be relative Uri if came here - Debug.Assert(syntax != null); - Uri result = new Uri(flags, syntax, uriString); - - // Validate instance using ether built in or a user Parser + Uri result = new(); try { - UriFormatException? e = result.InitializeUri(err, uriKind); - - if (e == null) - { - result.DebugSetLeftCtor(); - return result; - } - - return null; + UriFormatException? e = result.TryCreateThis(uriString, dontEscape, uriKind, in creationOptions); + return e is null ? result : null; } catch (UriFormatException) { - Debug.Assert(!syntax.IsSimple, "A UriPraser threw on InitializeAndValidate."); - // A precaution since custom Parser should never throw in this case. + Debug.Assert(result.Syntax is { IsSimple: false }, "A custom UriParser threw on InitializeAndValidate."); return null; } } @@ -938,6 +889,7 @@ internal bool IsBaseOfHelper(Uri uriLink) private void CreateThisFromUri(Uri otherUri) { DebugAssertInCtor(); + Debug.Assert((otherUri._flags & Flags.Debug_LeftConstructor) != 0); _flags = otherUri._flags; diff --git a/src/libraries/System.Private.Uri/tests/FunctionalTests/UriTests.cs b/src/libraries/System.Private.Uri/tests/FunctionalTests/UriTests.cs index 17add0951cf7ff..577ff26c093b06 100644 --- a/src/libraries/System.Private.Uri/tests/FunctionalTests/UriTests.cs +++ b/src/libraries/System.Private.Uri/tests/FunctionalTests/UriTests.cs @@ -914,14 +914,38 @@ public static IEnumerable ToStringTest_MemberData() // a) We can test each method without it being impacted by implicit caching of a previous method's results // b) xunit's implicit formatting of arguments doesn't similarly disturb the results - yield return new object[] { () => new Uri("http://test"), "http://test/" }; - yield return new object[] { () => new Uri(" http://test "), "http://test/" }; - yield return new object[] { () => new Uri("/test", UriKind.Relative), "/test" }; - yield return new object[] { () => new Uri("test", UriKind.Relative), "test" }; - yield return new object[] { () => new Uri("http://foo/bar/baz#frag"), "http://foo/bar/baz#frag" }; - yield return new object[] { () => new Uri(new Uri(@"http://www.contoso.com/"), "catalog/shownew.htm?date=today"), "http://www.contoso.com/catalog/shownew.htm?date=today" }; - yield return new object[] { () => new Uri("http://test/a/b/c/d/../../e/f"), "http://test/a/b/e/f" }; - yield return new object[] { () => { var uri = new Uri("http://test/a/b/c/d/../../e/f"); uri.ToString(); return uri; }, "http://test/a/b/e/f" }; + (Func, string)[] testData = + [ + (() => new Uri("http://test"), "http://test/"), + (() => new Uri(" http://test "), "http://test/"), + (() => new Uri("/test", UriKind.Relative), "/test"), + (() => new Uri("test", UriKind.Relative), "test"), + (() => new Uri("http://foo/bar/baz#frag"), "http://foo/bar/baz#frag"), + (() => new Uri(new Uri(@"http://www.contoso.com/"), "catalog/shownew.htm?date=today"), "http://www.contoso.com/catalog/shownew.htm?date=today"), + (() => new Uri("http://test/a/b/c/d/../../e/f"), "http://test/a/b/e/f"), + (() => { var uri = new Uri("http://test/a/b/c/d/../../e/f"); uri.ToString(); return uri; }, "http://test/a/b/e/f"), + (() => new Uri("p%41th", UriKind.Relative), "pAth"), + (() => new Uri("pa\uFFFFth", UriKind.Relative), "pa%EF%BF%BFth"), + (() => new Uri("C:\\path", UriKind.Relative), "C:\\path"), + (() => new Uri("C:\\p%41th", UriKind.Relative), "C:\\pAth"), + (() => new Uri("http:\\host/path", UriKind.Relative), "http:\\host/path"), + (() => new Uri("http:\\host/p%41th", UriKind.Relative), "http:\\host/pAth"), + (() => new Uri("//host/path", UriKind.RelativeOrAbsolute), "//host/path"), + (() => new Uri("//host/p%41th", UriKind.RelativeOrAbsolute), "//host/pAth"), + ]; + + foreach ((Func factory, string expected) in testData) + { + yield return [factory, expected]; + + yield return [() => + { + Uri uri = factory(); + Assert.True(Uri.TryCreate(uri.OriginalString, uri.IsAbsoluteUri ? UriKind.Absolute : UriKind.Relative, out Uri? uri2)); + Assert.Same(uri.OriginalString, uri2.OriginalString); + return uri2; + }, expected]; + } } [Theory]