-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Fix URI edge case issue #116789
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
Fix URI edge case issue #116789
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes a URI edge case by adjusting the offset calculations during URI parsing and adding a test case to validate the behavior when leading whitespace and Unicode characters are present.
- Updated the offset assignments for scheme, user, and host in the URI parsing logic.
- Added a new functional test to verify URI construction under edge-case conditions in Windows and non-Windows environments.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
File | Description |
---|---|
src/libraries/System.Private.Uri/tests/FunctionalTests/UriTests.cs | Added a new test verifying the behavior of URI parsing with leading whitespace and Unicode. |
src/libraries/System.Private.Uri/src/System/Uri.cs | Updated offset fields in the URI parsing method to address the edge-case bug. |
Comments suppressed due to low confidence (2)
src/libraries/System.Private.Uri/tests/FunctionalTests/UriTests.cs:805
- [nitpick] Consider adding an inline comment explaining the intention behind the chosen URI string format and handling of leading whitespace/unicode characters for clarity in future maintenance.
string uriString = PlatformDetection.IsWindows ? "\tC:\\\u005C" : "\t/\u005C";
src/libraries/System.Private.Uri/src/System/Uri.cs:3301
- [nitpick] It might be beneficial to document why the offset fields for User and Host are set to the current string length immediately after setting the Scheme offset, to aid future maintainers in understanding the edge-case handling.
_info.Offset.Scheme = 0;
Tagging subscribers to this area: @dotnet/ncl |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test failures are related
src/libraries/System.Private.Uri/tests/FunctionalTests/UriTests.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you
Fixes #110480