Skip to content

Handle unicode in absolute URI path for combine. #111710

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

Merged

Conversation

rokonec
Copy link
Member

@rokonec rokonec commented Jan 22, 2025

Fixes: #111355

Changes Made:
Copy full UriInfo when flags indicate it has been fully initialized, so we can consider it immutable and freely reuse it without thread safety concerns.

Testing:
Added unit tests which failed before and passing now.

Notes:
Huge credit to: @MihaZupan

@rokonec
Copy link
Member Author

rokonec commented Jan 22, 2025

@MihaZupan @karelz Shall we consider it for backporting to 9? My recommendation is yes.

Copy link
Member

@MihaZupan MihaZupan left a comment

Choose a reason for hiding this comment

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

The product change looks good to me, happens to also make some existing uses a bit more efficient by avoiding reparsing.
In the future we could also choose to make the logic even simpler by removing the else entirely and call EnsureParseRemaining instead.

Shall we consider it for backporting to 9?

I think we should, I expect many more users to hit this as they move to .NET 9, especially since it can also impact "normal" schemas like http.

@MihaZupan MihaZupan added this to the 10.0.0 milestone Jan 23, 2025
rokonec and others added 2 commits January 24, 2025 16:42
Co-authored-by: Miha Zupan <mihazupan.zupan1@gmail.com>
Change assert
Copy link
Member

@MihaZupan MihaZupan left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

@rokonec rokonec merged commit d20c35e into dotnet:main Jan 28, 2025
83 of 85 checks passed
@rokonec rokonec deleted the dev/rokonec/uri-comined-with-uinicode-filename branch January 28, 2025 12:53
grendello added a commit to grendello/runtime that referenced this pull request Jan 28, 2025
* main: (31 commits)
  Fix linux-x86 build (dotnet#111861)
  Add FrozenDictionary specialization for integers / enums (dotnet#111886)
  [SRM] Refactor reading from streams. (dotnet#111323)
  Sign the DAC and DBI during the build process instead of in separate steps (dotnet#111416)
  Removing Entry2MethodDesc as it is unnecessary (dotnet#111756)
  Cross Product for Vector2 and Vector4 (dotnet#111265)
  Handle unicode in absolute URI path for combine. (dotnet#111710)
  Drop RequiresProcessIsolation on mcc tests (dotnet#111887)
  [main] Update dependencies from dotnet/roslyn (dotnet#111691)
  new trimmer feature System.TimeZoneInfo.Invariant (dotnet#111215)
  [browser] reduce msbuild memory footprint (dotnet#111751)
  Add debugging checks for stack overflow tests failure (dotnet#111867)
  Localized file check-in by OneLocBuild Task: Build definition ID 679: Build ID 2629821 (dotnet#111884)
  Bump main to preview2 (dotnet#111882)
  Avoid generic virtual dispatch for frozen collections alternate lookup (dotnet#108732)
  Bump main versioning to preview1 (dotnet#111880)
  Switch OneLoc to main (dotnet#111872)
  Improve docs on building ILVerify (dotnet#111851)
  Update Debian version to 13 (dotnet#111768)
  win32: add fallback to environment vars for system folder (dotnet#109673)
  ...
@github-actions github-actions bot locked and limited conversation to collaborators Feb 28, 2025
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.

exception on binding WPF BitmapImage and german umlauts are used in file names (dotnet 9.0.0 / windows 11)
2 participants