-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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 WhiteSpace validation in HttpHeaders #102693
Conversation
Tagging subscribers to this area: @dotnet/ncl |
@MihuBot fuzz HttpHeaders |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
using System;
using System.Net.Http;
using System.Net.Http.Headers;
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;
BenchmarkRunner.Run<HeaderBenchmarks>(args: args);
//BenchmarkRunner.Run<HeaderBenchmarks>();
public class HeaderBenchmarks
{
private readonly HttpRequestHeaders _request = new HttpRequestMessage().Headers;
private readonly HttpResponseHeaders _response = new HttpResponseMessage().Headers;
[Benchmark]
public void Warning()
{
_request.Add("Warning", "001 Hello \"World\"");
_request.Clear();
}
[Benchmark]
public void ETag()
{
_response.Add("ETag", "\"foo\"");
_response.Clear();
}
} |
Benchmark results on Intel
|
EgorBot manual
All targets are Linux-only at the moment. NOTE: [DisassemblyDiagnoser] may cause unexpected crashes in BDN on Linux (at least on x64) Usage example: link |
@dotnet/ncl any volunteers for reviewing this one? |
I will, but would be better to have one more eye on this |
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.
LGTM, just a question
Fixes #102584
#86007 added a bit too much validation for header values. In a couple of places it was redundant with the checks we would do anyway (e.g. right before calling
HeaderDescriptor.TryGet
orHeaderUtilities.CheckValidToken
), and in a few others it was more strict than the parsing validation that only looks for the ASCII space and horizontal tab.In a couple of places we were also calling into validating constructors right after parsing, thus wasting cycles on validating twice.