-
Notifications
You must be signed in to change notification settings - Fork 5.2k
Expand set of known header names and values #117989
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
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 expands the set of known HTTP header names and values to optimize string allocation avoidance in response header processing. The changes add more common header values (like text/javascript
, Accept-Ranges: bytes
, Connection: keep-alive
) to the pre-allocated string pools, reducing memory allocations during HTTP processing.
Key changes:
- Added 3 new known header names: Cross-Origin-Resource-Policy, Timing-Allow-Origin, and X-Served-By
- Expanded known values for existing headers with commonly-used values
- Updated Content-Type optimizations to handle more media types and charset variations
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
KnownHeadersTest.cs |
Added test cases for new known headers and Content-Type values, removed X-Frame-Options test cases |
QPackStaticTable.cs |
Updated reference to use KnownHeaders.TimingAllowOrigin instead of string literal |
KnownHeaders.cs |
Added new header definitions, expanded known values arrays, updated constructor calls to use collection expressions |
KnownHeader.cs |
Simplified constructor overload to include knownValues parameter |
HeaderDescriptor.cs |
Enhanced Content-Type value optimization with additional media types and charset combinations |
Comments suppressed due to low confidence (1)
src/libraries/System.Net.Http/tests/UnitTests/Headers/KnownHeadersTest.cs:223
- The removal of X-Frame-Options test cases (DENY and SAMEORIGIN) reduces test coverage for this header's known values. These test cases should be retained to ensure the X-Frame-Options header values are properly validated.
[InlineData("X-Content-Type-Options", "nosniff")]
src/libraries/System.Net.Http/src/System/Net/Http/Headers/HeaderDescriptor.cs
Show resolved
Hide resolved
src/libraries/System.Net.Http/src/System/Net/Http/Headers/KnownHeaders.cs
Show resolved
Hide resolved
/ba-g Build analysis isn't picking up the duplicate WASM queue timeouts |
Closes #63750
We have a set of known header names and values that allows us to avoid materializing
string
s for response headers. This PR expands that list.Not trying to be perfect here, this is just an easy way to avoid some allocations.
E.g. we were missing some basic cases like
Content-Type: text/javascript
,Accept-Ranges: bytes
, orConnection: keep-alive
.The real change is just 3ab3770