Open seanmonstar opened 1 year ago
Thanks for creating this issue.
I went through the list of existing constants and here are some thoughts on the existing ones:
Expires
: replaced by Cache-Control - still supported in browsers and applicationsPragma
: header from the HTTP/1.0 days - still supported in browsers.Public-Key-Pinning
| Public-Key-Pinning-Report-Only
: dropped by all browsers - explanationFrom
: idk what it's doing in HTTP :thinking:Warning
: not sure who uses it - deprecatedX-Frame-Options
: still supported but replaced by frame-ancestors in CSP v2 which is supported by all modern browsersX-XSS-Protection
: deprecated - should be unset or disabled as it may break websites or create additional XSS vulnerabilitiesX-DNS-Prefetch-Control
: non standard - supported anyway by browsers :man_shrugging:Speaking for me I would very well do without most of them if that meant adding newer security or otherwise useful headers.
Thanks for listing those, it may be worth considering quicker if we should chop out some of the obscure ones that are currently in the crate before 1.0 (which is coming soon!).
A good place to start is the IANA Field Name Registry. The registry itself is here, and the specification of the registry is here in RFC 9110.
In this gist I compared the names in the registry vs the names defined as constants in http
. There are quite a few more in the registry than we currently have as constants. I'm in favor of including any registry headers that aren't provisional
as constants in this crate, as they are very unlikely to ever go away.
There are three sets that I think are more debatable.
provisional
headers in the IANA registryamp-cache-transform
configuration-context
ediint-features
isolation
repeatability-client-id
repeatability-first-sent
repeatability-request-id
repeatability-result
sec-gpc
timing-allow-origin
I'm inclined to not proactively make constants for these given their lack of stability, but we should be open to their inclusion if folks request them and it appears they're on track to standardize. For example, Timing-Allow-Origin
is part of the Resource Timing W3C Editor's Draft.
http
but not the registrydnt
referrer-policy
upgrade-insecure-requests
x-dns-prefetch-control
x-xss-protection
These probably need to be handled on a case-by-case basis, as they might be referenced in other specs. For example, dnt
is part of the Fetch standard.
http
...but present in widely-cited sources such as MDN. For example, Accept-CH-Lifetime
. These probably also need to be treated on a case-by-case basis.
Hm, even the MDN DNT page says the header is deprecated. Perhaps it's worth just removing the non-standard ones entirely.
Could we get away with removing all the deprecated/obsolete headers even if in the registry?
I'm in favor of including any registry headers that aren't
provisional
as constants in this crate, as they are very unlikely to ever go away.
It's a fair position to take. If many others prefer it, we can do that.
I'm personally skeptical of adding obscure names even if they are in the registry. They seem like a potential distraction in the docs. But I can't really articulate a good reason otherwise. Perhaps at least I'd wait to add the odd ones until someone really asked?
Could we get away with removing all the deprecated/obsolete headers even if in the registry?
I'm definitely coming from a biased perspective as an implementer of a reverse proxy. We don't control the nature of requests from clients or the responses from origins, so we try to accept as much actual existing HTTP traffic as possible without compromising security.
It's also common for current, active specs to refer to deprecated and obsolete headers. This is often in MAY
or SHOULD
contexts, but nonetheless comprehensive, modern implementations frequently need to at least refer to and understand the cruft even if they don't produce the cruft themselves.
I'm personally skeptical of adding obscure names even if they are in the registry. They seem like a potential distraction in the docs.
This raises a higher-level question: what is our goal for including these constants? For me, the value comes from being able to avoid repeating HeaderName::from_static()
invocations for standardized headers across my different crates. And while that is less painful since that method became const
, there are still some odd limitations that crop up (particularly around static references) that the crate-defined constants avoid. I don't personally come to the http
rustdoc for information about the semantics of the headers, but I know I am not the typical user in that regard, and therefore am probably not a good judge about the level of distraction imposed by additional definitions in the docs.
Perhaps at least I'd wait to add the odd ones until someone really asked?
I think being proactive about adding registered names would help us avoid a trickle of one-off PRs and requests for releases (like I am doing now in #583 😬). And while I doubt we want to impose a proc macro dependency for http
, we could also consider automatically generating the constants based on the IANA registry's CSV, which I got IANA and IETF to license as CC0.
What if a core, standard, forward-looking set of constants were included by default, but then the rest of the obscure/deprecated/provisional/nonstandard-but-in-use ones were hidden behind a feature flag, like extra-headers
?
@d2718 the complexity of wrangling a Cargo feature would outweigh the benefit of less clutter in the namespace, in my opinion.
I suppose we could add some namespacing via child modules of http::header
: IANA permanent
header names could stay in http::header
, but we could stash other categories in http::header::obsolete
, http::header::provisional
, etc. I kinda like that as a quick signal to the user that they're straying off the beaten path.
Another thing that came to my mind today was that now that from_static
is a const fn
, we could potentially provide constants that don't require being in the internal StandardHeader
enum. A small concern I had had before was around growing that enum and match statements. But since they can also be from a static string, it does feel like we could include all easily.
Then I suppose a last question is how to determine whether something should be in StandardHeader
, or if it should be from a string. A value in StandardHeader
will mean that hashing and comparing it will be faster than a static string, but growing the enum may mean the match in hash
and cmp
(and as_str
) will be more code, and could be miss optimizations. Probably worth measuring or determining some heuristic? I think most people would prefer that the most common headers are as fast as possible.
Then I suppose a last question is how to determine whether something should be in
StandardHeader
, or if it should be from a string.
I think we should start with a larger set in StandardHeader
, since we should be able to "demote" them to from_static
invocations without semver breakage if it turns out performance is a problem. We can check the microbenchmarks right away to have some idea of impact, but concerns about missed optimizations are hard to definitively disprove.
Would you be open for me to make a PR for Client Hint headers, or do these count as provisional, even if browsers do all support them now a days (the most common type of user agent among them all).
I would understand if you do not want it yet though, even though servers and proxies do need these for content negotiation. It also impacts other aspects such as Caching.
We include many standard header names as constants in
http::header
. The exact criteria has not been defined, and it would be wise to do so. This stops it from being an unwritten rule in a few people's heads. The policy likely should consider the pros of adding any name versus the costs of adding a name.