(async) Remove or replace typed headers

[jebrosen]

Typed headers were removed between hyper 0.10 and hyper 0.12. typed-headers does not appear to have all the headers hyper 0.10 did, but it looks like hyperx and headers might.

Options:

• Re-export headers from one or the other library - this is what we did with hyper 0.10
• Remove typed header support - this is currently implemented in the async branch; only the header names are available in hyper and we re-export those.
• Implement From<X> for Header for every header in one or both of hyperx and typed-headers, perhaps behind a feature flag

[davidbarsky]

Typed headers exist at hyperium/headers, and they can be used with the latest releases of hyperium/http & hyperium/hyper—you might not need to write your own bridging.

[jebrosen]

Thanks for pointing that out, I missed and/or forgot about it when I checked for existing implementations.

[davidbarsky]

It’s not well advertised, and it requires importing an extension trait so that the http::header::HeaderMap can use the typed headers.

The derive headers in the repository only supports headers defined within hyperium/header. I’d like to make them more extensible/usable outside the crate, but I haven’t had the time, unfortunately.

[jebrosen]

I'm inclined towards trying out the headers from the headers crate first, re-exporting and/or From impls - possibly under a feature flag. In 0.4, the headers are both re-exported and have implementations for From<X> for rocket::http::Header<'static> and I think we should start with the same approach for now to minimize disruption.

[jebrosen]

This should be an easy issue to help with, and first it's a bit of a research question - how complete is headers, and what kind of integration could be done with Rocket?

I previously opened hyperium/headers#48 because the types I looked at (Location, ETag) didn't seem to have public constructors. Since that time:

• ETag has gained a public constructor via FromStr. However it is still of limited utility, because the comparison functions i.e. EntityTag::weak_eq are pub(crate).
• I realized that I picked two unlucky examples to check - ContentRange and CacheControl are two headers that do have public constructors and can be useful.
• I also realized that Header::decode can be used to construct headers. It does not seem to be clearly documented which headers do or don't validate their contents - ETag appears to, but Location does not - it just contains any HeaderValue.

Looking forward:

• Some feedback would be helpful on what are the "must-have" headers that most applications would likely use? Headers related to caching, ranges, and Content-* seem like high priorities, for example.
• Is another similar crate (I know of hyperx or typed-headers) stable, correct, complete, and well-enough maintained that we should use one of them instead of headers?
• We should implement Into<rocket::http::Header<'static>> (or the converse From impl)
  • Is this useful and stable enough to live in rocket_http, or should the conversion live in an external crate or as a snippet/example code? It would be unfortunate if headers 0.4 was likely to released soon and rocket_http would not be able to upgrade to use it.
  • This impl might be an individual impl for each header type (like 0.4 did with a macro), a blanket impl, or an adapter type that implements Into<rocket::http::Header<'static>. These have different eases of use with #[derive(Responder)], and some approaches (blanket impl) can only be used if done in rocket_http directly.

Any help would be appreciated in terms of Proof-of-Concept, feedback on what should be prioritized, or testimonials for any of the crates or approaches I've mentioned or not mentioned above.

[ejmg]

Can I take a swing at this issue? I will need to get familiar with hyperium along with the internals related to the change, but I have the time to commit for the foreseeable future.

[jespersm]

I was just looking at this:

Ad 2) As for applicable type header libraries, it appears that both headers and hyperx have recent releases (January 2021).

Both support:

• AcceptRanges
• AccessControlAllowCredentials
• AccessControlAllowHeaders
• AccessControlAllowMethods
• AccessControlExposeHeaders
• AccessControlMaxAge
• AccessControlRequestHeaders
• AccessControlRequestMethod
• Allow
• Authorization
• CacheControl
• Connection
• ContentDisposition
• ContentEncoding
• ContentLength
• ContentLocation
• ContentRange
• ContentType
• Cookie
• Date
• ETag
• EntityTag
• Expires
• Host
• IfModifiedSince
• IfUnmodifiedSince
• LastModified
• Location
• Origin
• ProxyAuthorization
• Referer
• Server
• SetCookie
• StrictTransportSecurity
• TE
• TransferEncoding
• Upgrade
• UserAgent

But hyperx 1.3.0 contain these, which headers 0.3.3 do not have:

• Accept
• AcceptCharset
• AcceptEncoding
• AcceptLanguage
• ContentLanguage
• From
• LastEventId
• Link
• LinkValue
• Prefer
• PreferenceApplied
• Warning

However, hyperx lacks the following, which headerssupport:

• AccessControlAllowOrigin
• IfMatch
• IfNoneMatch
• IfRange
• Pragma
• ReferrerPolicy
• RetryAfter
• SecWebsocketAccept
• SecWebsocketKey
• SecWebsocketVersion
• Vary

The support seems to be quite comparable, and the fact that hyperx is past 1.0 may speak to its advantage? (The typed-headers support the shortest list of headers and has seen no update since December 2019) Caveat: I've only checked the docs and a few tests, but not worked with them as such.

Ad 1) It seems easier to identify the header library first, and then refine the header list afterwards. Some headers may be irrelevant, and I'm not sure how e.g. CORS headers are handled in Rocket, but I'd guess that they have special treatment anyway due to preflight handling. Same would likely apply for the Websocket-specific headers.

Ad 3) So, a proof-of-concept should handle how to to get or match the headers (i.e. how they bridge with FromRequest<T> and how they may be added to a response?

As for matching with the request, I'm guessing there may be several ways of accessing headers:

#[get("/")]
fn index(languages: AcceptLanguage) -> String {
    // Only match this route if the "Accept-Language" header is included in the request
    ...
}

or perhaps:

#[get("/")]
fn index(languages_opt: Option<AcceptLanguage>) -> String {
    // This route is always matched on GET /, but the "Accept-Language" may be omitted from the request
    ...
}

I didn't see a reasonable handling for repeated headers in the three typed header libraries. From a request, repeated headers should "fold" their values together while preserving order, but only if they're of the repeating kind (like Accept-Language). Some types should never repeat, that would be a user error (like Host). Mabye it's best to defer that little bit.

For responses, I assume the headers need to work with the responder derive macros, like ContentType is handled today.

I'm assuming that it is acceptable to expose the imported typed headers (via aliases) as it's done for http::hyper. Right?

---

Is that the rough scope for going forward?

[jebrosen]

It seems easier to identify the header library first, and then refine the header list afterwards. Some headers may be irrelevant, and I'm not sure how e.g. CORS headers are handled in Rocket, but I'd guess that they have special treatment anyway due to preflight handling. Same would likely apply for the Websocket-specific headers.

rocket doesn't support CORS headers out of the box, nor does it support websocket (yet). The only feature, to my knowledge, that was changed (lost) going from 0.4 to master is the re-exports in https://api.rocket.rs/v0.4/rocket/http/hyper/header/index.html that implemented Into<Header>, which let them be used in set_header and #[derive(Responder)]. The current state of the master branch is that any usage of those have to now be changed to set_raw_header or the stringly-typed rocket::http::Header.

In other words, this issue affects rocket itself only a little bit since there are not too many headers being set by Rocket, but it affects applications using rocket in proportion to how frequently they used those headers.

So, a proof-of-concept should handle how to to get or match the headers (i.e. how they bridge with FromRequest<T> and how they may be added to a response?

This is #1283, and is a new feature rather than fixing a regression - but I am also interested in it. This would give us both Into<Header> (for responses, like we had in 0.4) and FromRequest (for requests, a new feature) for whichever headers we end up using. Your ideas about repeated headers and how to handle Option suggest it needs some more design thought, so I don't want to hold up restoration of typed headers on that point.

The support seems to be quite comparable, and the fact that hyperx is past 1.0 may speak to its advantage?

If it were entirely up to me to decide this and I had to choose today, I think I would pick hyperx. It has a 1.0 release, and its public API seems to be more complete overall: hyperx has public constructors, public error types, public methods on https://docs.rs/hyperx/1.3.0/hyperx/header/struct.EntityTag.html, etc. where many of the equivalents in headers are still pub(crate) or not implemented.

I'm assuming that it is acceptable to expose the imported typed headers (via aliases) as it's done for http::hyper. Right? Is that the rough scope for going forward?

This is about what I had in mind, yeah: restoring the functionality that was lost is the main goal of this issue, even if we gain/lose a few individual headers in the process, change the way the functionality is accessed, or use a different library, as long as we are comfortable with it (and so far, I do like or at least don't mind hyperx).

[jespersm]

Thanks, I'll see about Into<Header>first and demonstrate the typed getter from hyperx, and defer the support for various cases of FromHeader<...> and parameter binding for #1283.

It turns out that the API in hyperxhandles repeated headers just fine, so that's not a concern for this issue.

I'm not a very seasoned Rust developer, but I'll give it a go!

[jebrosen]

I'm not a very seasoned Rust developer, but I'll give it a go!

Happy to hear it! It's actually a fairly simple change in terms of lines of code; the main thing to look at is this part of hyper.rs in 0.4 which has been removed in master: https://github.com/SergioBenitez/Rocket/blob/v0.4/core/http/src/hyper.rs#L38, and the call to import_hyper_headers!.