Skip to content

Fix parsing inconsistencies between Uri ctors and TryCreate factories #123932

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
MihaZupan merged 5 commits into from
Feb 13, 2026
+78 −110
Merged

Fix parsing inconsistencies between Uri ctors and TryCreate factories #123932

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f683c76
Unify Uri constructor and TryCreate paths
MihaZupan Feb 3, 2026
c98346a
Expand more ToString tests to cover TryCreate
MihaZupan Feb 3, 2026
1924f55
Make fallbacks to a relative Uri consistent
MihaZupan Feb 3, 2026
78486d2
Add more relative tests
MihaZupan Feb 3, 2026
1bc3330
Set Debug_LeftConstructor in just one place
MihaZupan Feb 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 2 additions & 10 deletions src/libraries/System.Private.Uri/src/System/Uri.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,8 @@ internal enum Flags : ulong
[Conditional("DEBUG")]
private void DebugSetLeftCtor()
{
DebugAssertInCtor();

_flags |= Flags.Debug_LeftConstructor;

AssertInvariants();
Expand Down Expand Up @@ -424,7 +426,6 @@ public Uri([StringSyntax(StringSyntaxAttribute.Uri)] string uriString)
ArgumentNullException.ThrowIfNull(uriString);

CreateThis(uriString, false, UriKind.Absolute);
DebugSetLeftCtor();
}

//
Expand All @@ -438,7 +439,6 @@ public Uri([StringSyntax(StringSyntaxAttribute.Uri)] string uriString, bool dont
ArgumentNullException.ThrowIfNull(uriString);

CreateThis(uriString, dontEscape, UriKind.Absolute);
DebugSetLeftCtor();
}

//
Expand All @@ -456,7 +456,6 @@ public Uri(Uri baseUri, string? relativeUri, bool dontEscape)
throw new ArgumentOutOfRangeException(nameof(baseUri));

CreateUri(baseUri, relativeUri, dontEscape);
DebugSetLeftCtor();
}

//
Expand All @@ -467,7 +466,6 @@ public Uri([StringSyntax(StringSyntaxAttribute.Uri, nameof(uriKind))] string uri
ArgumentNullException.ThrowIfNull(uriString);

CreateThis(uriString, false, uriKind);
DebugSetLeftCtor();
}

/// <summary>
Expand All @@ -480,7 +478,6 @@ public Uri([StringSyntax(StringSyntaxAttribute.Uri)] string uriString, in UriCre
ArgumentNullException.ThrowIfNull(uriString);

CreateThis(uriString, false, UriKind.Absolute, in creationOptions);
DebugSetLeftCtor();
}

//
Expand All @@ -498,7 +495,6 @@ public Uri(Uri baseUri, string? relativeUri)
throw new ArgumentOutOfRangeException(nameof(baseUri));

CreateUri(baseUri, relativeUri, false);
DebugSetLeftCtor();
}

//
Expand All @@ -515,7 +511,6 @@ protected Uri(SerializationInfo serializationInfo, StreamingContext streamingCon
if (uriString!.Length != 0)
{
CreateThis(uriString, false, UriKind.Absolute);
DebugSetLeftCtor();
return;
}

Expand All @@ -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();
}

//
Expand Down Expand Up @@ -617,7 +611,6 @@ public Uri(Uri baseUri, Uri relativeUri)
if (!ReferenceEquals(this, resolvedRelativeUri))
CreateThisFromUri(resolvedRelativeUri);

DebugSetLeftCtor();
return;
}
}
Expand All @@ -634,7 +627,6 @@ public Uri(Uri baseUri, Uri relativeUri)
_syntax = null!;
_originalUnicodeString = null!;
CreateThis(newUriString, dontEscape, UriKind.Absolute);
DebugSetLeftCtor();
}

//
Expand Down
136 changes: 44 additions & 92 deletions src/libraries/System.Private.Uri/src/System/UriExt.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,21 @@ namespace System
{
public partial class Uri
{
//
// All public ctors go through here
//
/// <summary>Helper called by all constructors to construct the Uri.</summary>
[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;
}
}

/// <summary>Core helper called by all constructors and TryCreate factories to construct the Uri.</summary>
[MemberNotNull(nameof(_string))]
private UriFormatException? TryCreateThis(string? uri, bool dontEscape, UriKind uriKind, in UriCreationOptions creationOptions = default)
{
DebugAssertInCtor();

Expand All @@ -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;
Expand All @@ -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);
}

Expand All @@ -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
Expand All @@ -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"
}
}
Expand All @@ -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);
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}

Expand All @@ -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;
}

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -625,77 +625,28 @@ public static string EscapeDataString(ReadOnlySpan<char> charsToEscape) =>
public static bool TryEscapeDataString(ReadOnlySpan<char> charsToEscape, Span<char> 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.
/// <summary>Must never be used except by <see cref="CreateHelper(string?, bool, UriKind, in UriCreationOptions)"/>.</summary>
private Uri() { }
#pragma warning restore CS8618

//
// a Uri.TryCreate() method goes through here.
//
/// <summary>Called by TryCreate.</summary>
internal static Uri? CreateHelper(string? uriString, bool dontEscape, UriKind uriKind, in UriCreationOptions creationOptions = default)
{
if (uriString is null)
{
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;
}
}
Expand Down Expand Up @@ -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;

Expand Down
40 changes: 32 additions & 8 deletions src/libraries/System.Private.Uri/tests/FunctionalTests/UriTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -914,14 +914,38 @@ public static IEnumerable<object[]> 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<Uri>, 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<Uri> 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]
Expand Down
Loading