-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Replace dangerous characters in response headers #117290
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
Replace dangerous characters in response headers #117290
Conversation
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.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnectionBase.cs
Outdated
Show resolved
Hide resolved
src/libraries/System.Net.Http/src/System/Net/Http/Headers/HeaderDescriptor.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.
@MihaZupan PTAL. Also see #117290 (comment).
src/libraries/System.Net.Http/src/System/Net/Http/Headers/HeaderDescriptor.cs
Show resolved
Hide resolved
/azp run runtime-libraries-coreclr outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
Fixes #116688
Edit: The original PR has been reworked.
The change has been optimized for the happy path, when no replacement has to be done. We are identifying it by scanning through undecoded
headerValue
bytes, which is expected to be cheap E2E.Original description:
I ran a basic microbenchmark to see how much can this impact performance. The difference seems negligible on the happy path, especially with the
headerValue.IndexOfAny(s_dangerousCharacterBytes) >= 0
optimization, though we can remove it if we prefer simplicity over a few nanosecs.