Skip to content

Remove reference to InvalidOperationException in Uri.OriginalString#2657

Merged
Thraka merged 1 commit intodotnet:masterfrom
watfordgnf:patch-1
Jul 26, 2019
Merged

Remove reference to InvalidOperationException in Uri.OriginalString#2657
Thraka merged 1 commit intodotnet:masterfrom
watfordgnf:patch-1

Conversation

@watfordgnf
Copy link
Copy Markdown
Contributor

Summary

Uri.OriginalString does not throw InvalidOperationException if a relative URI is used. This is true for .NET Core 3.0+ (at least) and the .NET Framework 4.8 (via ReferenceSource).

Sources: https://github.com/dotnet/corefx/blob/e2b4dc93c9f9482b67f962106937d4fdd46f5673/src/System.Private.Uri/src/System/Uri.cs#L1145-L1153
https://referencesource.microsoft.com/#System/net/System/URI.cs,1234

This caused some confusion developing Uri support in System.Text.Json dotnet/corefx#39015

Uri.OriginalString does not throw InvalidOperationException if a relative URI is used. This is true for .NET Core 3.0+ (at least) and the .NET Framework 4.8 (via ReferenceSource).

Sources: https://github.com/dotnet/corefx/blob/e2b4dc93c9f9482b67f962106937d4fdd46f5673/src/System.Private.Uri/src/System/Uri.cs#L1145-L1153
https://referencesource.microsoft.com/#System/net/System/URI.cs,1234
@BillWagner
Copy link
Copy Markdown
Member

Thanks for making this update @watfordgnf

@karelz Can you verify this for accuracy? It looks good to me, but I want someone on the product team to verify.

Copy link
Copy Markdown
Contributor

@Thraka Thraka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created an app to test on core 3.0 and framework 4.5 and there is no exception thrown. I also checked the source on the internal index and in github for core 2.2 and 3.0. No exception is thrown. I think this PR is OK to accept.

@Thraka Thraka merged commit d91a45d into dotnet:master Jul 26, 2019
@Thraka Thraka added this to the July 2019 milestone Jul 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants