-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Add span-based Uri {Try}{Un}EscapeDataString overloads #98074
Conversation
Tagging subscribers to this area: @dotnet/ncl Issue DetailsImplements #40603 namespace System
{
partial class Uri
{
public static string EscapeDataString(ReadOnlySpan<char> charsToEscape);
public static bool TryEscapeDataString(ReadOnlySpan<char> charsToEscape, Span<char> destination, out int charsWritten);
public static string UnescapeDataString(ReadOnlySpan<char> charsToUnescape);
public static bool TryUnescapeDataString(ReadOnlySpan<char> charsToUnescape, Span<char> destination, out int charsWritten);
}
} The test coverage for these was not great, so I added a bunch more combinations. I also moved similar test cases from
|
Note regarding the This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change. |
src/libraries/System.Private.Uri/tests/FunctionalTests/UriEscapingTest.cs
Show resolved
Hide resolved
} | ||
|
||
return HttpRuleParser.DefaultHttpEncoding.GetBytes(builder.ToString()); | ||
// We know the encoded length because the input is all ASCII. |
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.
How do we know the input is ASCII?
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.
The input is a bunch of concatenated Uri.EscapeDataString
results, which are always ASCII, and the encoding is Latin1.
I clarified it in the comment now.
Closes #40603
The test coverage for these was not great, so I added a bunch more combinations. I also moved similar test cases from
System.Runtime.Tests
into the Uri project.I updated
FormUrlEncodedContent
to also use these, so it's now allocating a lot less. For a random usage I pulled from eShopOnWeb tests:Looking at something like YARP's
EncodePath
helper with ASP.NET Core's test paths, the difference is