Skip to content
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.

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

Reset all Uri fields when reusing an instance #41324

Merged

Conversation

MihaZupan
Copy link
Member

Simplifying OriginalString to _originalUnicodeString ?? _string in #33044 introduced a bug affecting the new Uri(baseUri, relativeUri) combine operations.

The bug can be hit if:

  1. The relativeUri contains non-ascii characters
  2. The baseUri uses a custom (not built-in scheme) - in this case pack
  3. A custom UriParser is registered for that scheme
  4. That parser is a "Simple parser" - a GenericUriParser, and it is set up without the GenericUriParserOptions.IriParsing flag

The cause is that when combining the Uris, the Uri instance gets reused, but not all internal fields are reset. As _originalUnicodeString was not being reset, it changed the output of OriginalString when reused. This led to the relative Uri being used in the parsing logic instead of the constructed combined Uri.

This does not affect built-in schemes / custom parsers with the IriParsing flag, as _originalUnicodeString will be properly set during parsing (the old value won't sneak through).

Fixes #41177

@MihaZupan MihaZupan added this to the 5.0.0 milestone Aug 25, 2020
@MihaZupan MihaZupan requested a review from a team August 25, 2020 13:06
@ghost
Copy link

ghost commented Aug 25, 2020

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

@MihaZupan
Copy link
Member Author

cc: @ryalanms

@karelz karelz modified the milestones: 5.0.0, 6.0.0 Aug 25, 2020
@ryalanms
Copy link
Member

Thank you, @MihaZupan and @karelz, for the fast response and fix!

@ryalanms
Copy link
Member

cc @dotnet/wpf-developers

Copy link
Member

@ManickaP ManickaP left a comment

Choose a reason for hiding this comment

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

LGTM

@MihaZupan MihaZupan merged commit 436f155 into dotnet:master Aug 26, 2020
@MihaZupan
Copy link
Member Author

/backport to release/5.0

@github-actions
Copy link
Contributor

Started backporting to release/5.0: https://github.com/dotnet/runtime/actions/runs/225384771

@ghost ghost locked as resolved and limited conversation to collaborators Dec 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

URI creation for certain URIs regressed from Preview 4 to Preview 5
4 participants