Use interned Header Names for known headers not in the pre-allocated block

[benaadams]

Use single instance strings for known header names; even if they aren't in the preserved header block.

This helps prevent multiple duplicates of the same string in long lived connections e.g. WebSockets

Resolves #31305

/cc @davidfowl

[benaadams]

Why this change?

Saw an; unrelated, issue in dotnet/runtime where someone was complaining about the casing of headers, so added change to keep baggage, tracestate, and traceparent 's previous casing

[JamesNK]

This method looks like it will be used to set unknown headers.

Aren't most of the headers in this collection known? They will never be used because generated code will handle setting them, right?

[benaadams]

There are 93 header names in HeaderNames; but only 20 in commonHeaders shared by HttpRequestHeaders and HttpReponseHeaders; could drop those 20; or have a different HashSet<string> for request and response but it doesn't seem worth it?

However yes, if the header is in the set for HttpRequestHeaders or HttpReponseHeaders and comes in on that direction it won't use the HashSet<string>, it only uses it for header names that aren't in that set rather than making a freshly constructed string long lived.

Any headers outside the set in HeaderNames will remain as the newly constructed string and won't be exchanged (i.e. the HashSet<string> is not updated while the app is running)

[JamesNK]

Ok!

This two phases of hash lookups (intern hashset then unknown dictionary) makes me wonder whether it makes sense to write a custom collection in the future.

As well as optimizing interning known keys, there is probably an opportunity to optimize memory usage. Most requests have few unknown headers, so O(n) lookups wouldn't matter. e.g. an adaptive collection that can switch between KeyValuePair<string, StringValues>[] and Dictionary<string, StringValues> depending upon how many headers have been added.

[benaadams]

Any headers outside the set in HeaderNames will remain as the newly constructed string and won't be exchanged

Is a question on whether to extend the values in HeaderNames and it becomes more valuable to do so with this change, rather than just being coinvent for the developer; e.g. missed standardise headers and defacto standard headers: Prefer, Preference-Applied, X-Request-ID, X-Correlation-ID, X-Forwarded-For, X-Forwarded-Host, X-Forwarded-Proto, Refresh, X-Powered-By, Timing-Allow-Origin

Though response set headers would likely be less important than request set headers; assuming they are being set by a single assembly so already an interned value (beyond developer convenience); though a pass-through name where Kestrel was acting as a proxy would still be valuable.

However that's probably a separate issue...

[benaadams]

Added issue for other headers #31374

[Tratcher]

Why are you adding Link, X-Content-Type-Options and X-XSS-Protection here? We don't intend this list to be exhaustive, we primarily add entries that are used directly by the framework. It's not clear why these were selected for special treatment in Kestrel.

[benaadams]

Why are you adding Link, X-Content-Type-Options and X-XSS-Protection here?

www.webpagetest.org is one of the commonly used performance testing sites (along with Lighthouse based things for core web vitals e.g. web.dev/measure); and it will call you out; rightly or wrongly, for not having X-Content-Type-Options and X-XSS-Protection set; amongst others:

Funnily X-Content-Type-Options is the only one dotnet.microsoft.com does set:

vs

Just looks bad if you are explicitly called out for not setting them; but the Framework doesn't support them as a pre-defined headername when it has 90 other ones...

The Link header rfc5988; is a good one for using for preloading of fonts prior to the css being parsed and html starting to render to increase webpage performance and reduce flash of unstyled content (FOUC) or flash of invisible text (FOIT) if you mark the text as unrenderable until the font is downloaded.

e.g. Adding this header on the landing HTML page

link: <https://assets.ageofascent.net/fonts/teko-light-latin.woff2>; rel=preload; as=font; crossorigin=anonymous,<https://assets.ageofascent.net/fonts/pt_sans-web-regular.woff>; rel=preload; as=font; crossorigin=anonymous

Chrome/Edge will then immediately start downloading the fonts; without yet having encountered the fonts in the parsing of the HTML (triggered by evaluating its css)

The browsers will also tell you off if you then don't use the preloaded font within the first few seconds of page load:

Anyway Link is one of the missed headers I use; so fringe benefit in adding for me with this change 😉

Its not adding it to the pre-reserved Kestrel headers; just to the HeaderNames constants; as otherwise they look fugly in use:

Though since you made me look; seems should add "Referrer-Policy" also 🤔

[ghost]

Hello @Tratcher!

Because this pull request has the auto-merge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.