Typed Headers?

[seanmonstar]

Work has been progressing on a typed headers system to replace what hyper had prior to v0.12. Some questions arise as to where this stuff should "live".

• @ashleygwilliams is owner of the headers crate name, and has agreed to transfering the name if we wish to use it for this.
• @carllerche is the owner of the http-headers crate name, and has suggested we could publish there, as the name headers might be too vague.
• We could include them directly in http.
  • It's certainly tempting to be able to use them directly, like let conlen = http::header::ContentLength(450);, and perhaps it should be the eventual goal.
  • However, I believe the typed headers are less stable than the core HTTP types. The goal of http was to have a stable crate that the ecosystem can use to be "generic" over HTTP implementations. Therefore, breaking changes must be exceptional. I have taken much effort to reduce the exposed surface area in the new crate API, specifically to reduce the need for breaking changes. Still, I would have to have missed something and be stuck with it.
  • It's been suggested that the current http API could be moved to http-core and re-exported, and have the stability be stronger on http-core than http.

See also #136.

[carllerche]

The header implementations are generally useful. I think it would be valuable to provide them as part of http. My preferred plan is to:

• Move stable types to http-core.
• Release typed headers as http-headers (headers is a bit generic).
• Convert http to a facade.

most libraries will only depend on the core types. This strategy is being adopted to allow breaking change iterations while maintaining stability of core types.

[davidbarsky]

+1 to @carllerche's plan. The other thing that I'd like—and this is mostly optional/depends on stability of functionality in the headers crate—is the ability to define custom headers without waiting for additional features in const fn. Building on the discussion in hyperium/headers#29, a compromise that I'd be happy with is a highly unstable, off-by-default feature flag that defines fn name() -> HeaderName for the Header trait until the required functionality lands in rustc. Alternatively, I'd be happy with a similarly unstable feature flag that exposes functionality similar to standard_headers!. Thoughts, @seanmonstar?

[sfackler]

You should be able to use lazy_static to make 'static HeaderName instances for custom headers.

[dekellum]

See also #174, which included an example (and slightly weird syntax requirement) with lazy_static.

[seanmonstar]

@sfackler the trait was adjusted to have const NAME: &'static HeaderName instead of fn name(), which means types made with lazy_static don't actually fit as a constant, I believe.

[sfackler]

Oh yeah, that would be a problem. Is the name actually used in a const context? fn name() -> &'static HeaderName seems like it would work just fine, right?

[davidbarsky]

Well, it might be useful for me to explain why I care about this feature. Namely—I'd like to have typed versions of the headers the AWS Lambda Runtime API returns without all sorts of stringly-typed interfaces. Like I said previously, I'd be happy to opt into some sort of unstable interface/feature flag so that our customers can interact with typed values.

[carllerche]

I agree, it would be nice to have a way to define custom static headers.

I think we can do this with a macro, but it will require bytes to provide a macro to define a static bytes struct as well.

[davidbarsky]

I think we can do this with a macro, but it will require bytes to provide a macro to define a static bytes struct as well.

I think I have the capacity to tackle this. Would the macro wrap the Bytes::from_static method?

[seanmonstar]

Why would Bytes need a macro? I would think we could eventually make Bytes::from_static a const fn, and eventually the same with HeaderName::from_static, since I've read of plans for allow const fns to inspect strings and panic.

[davidbarsky]

My understanding is that string inspection + panicking if invalid is blocked on rust-lang/rust#49146 and rust-lang/rust#51999, which don't have too much traction at the moment. That seems to be the correct solution to me, but I don't know when to expect it.

[davidbarsky]

So, I tried to create a macro that'd create a header, but I don't think it's possible to do so. Here's the code I wrote:

macro_rules! const_header {
    ($header_name:tt) => {
        &HeaderName::from_static($header_name)
    }
}

struct AWSRequestId(String);

impl Header for AWSRequestId {
    const NAME: &'static HeaderName = const_header!("LAMBDA_RUNTIME_AWS_REQUEST_ID");

    fn decode<'i, I>(values: &mut I) -> Result<Self, HeaderError> where
        Self: Sized,
        I: Iterator<Item=&'i HeaderValue> {
        unimplemented!()
    }

    fn encode<E: Extend<HeaderValue>>(&self, values: &mut E) {
        unimplemented!()
    }
}

...and here's the error message:

error[E0015]: calls in constants are limited to tuple structs and tuple variants
  --> lambda-runtime-server/src/main.rs:12:10
   |
12 |         &HeaderName::from_static($header_name)
   |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...
19 |     const NAME: &'static HeaderName = const_header!("LAMBDA_RUNTIME_AWS_REQUEST_ID");
   |                                       ---------------------------------------------- in this macro invocation
   |
note: a limited form of compile-time function evaluation is available on a nightly compiler via `const fn`
  --> lambda-runtime-server/src/main.rs:12:10
   |
12 |         &HeaderName::from_static($header_name)
   |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
...
19 |     const NAME: &'static HeaderName = const_header!("LAMBDA_RUNTIME_AWS_REQUEST_ID");
   |                                       ---------------------------------------------- in this macro invocation

Looking more at the standard_headers macro, it seems to be extremely error-prone and not fit for public consumption. I'll post a PR at the headers repo that changes the const NAME in Header to a fn name(). I know Sean isn't the biggest fan, but I'm not sure how to support customer headers otherwise.

[dodomorandi]

Sorry to revive this thread in a strange way, but I would like to point out a clippy lint when using HeaderName::from_static in order to create a const (instead of a static variable).

The following code:

pub const NAME: HeaderName = HeaderName::from_static("test");

Produces the following warning:

warning: a const item should never be interior mutable

Playground

I could just ignore the issue and use static instead of const, but I think that the lint exposes an interesting fact I would like to discuss.

First of all, the reason behind the lint: there is an AtomicPtr deep down. The dependency chain is:

HeaderName -> Repr<Custom> -> Custom -> ByteStr -> Bytes -> AtomicPtr

Now, the issue itself... It is a non-problem, except for one detail: the fact that potentially we have internal mutability in a header string that obviously is meant to never change is unexpected. Don't get me wrong, I understand the reason behind this: we want to be able to reuse memory when receiving headers (thanks to Bytes) and at the same time it is extremely nice to have just one HeaderName abstraction (instead of HeaderName and ConstHeaderName, for instance). However, following the principle of least surprise, probably you would expect a const HeaderName::from_static() function to not have interior mutability.

Now, what should be done? To be honest, I am not sure. I personally don't see a practical problem with the current approach, which is pretty ergonomic. Maybe we could consider adding a note in the documentation of HeaderName::from_static, suggesting to use a static variable instead of a const to avoid being flooded by clippy warnings.

Do you have better ideas? Do you think that this should be handled in a dedicated issue?

CC @paolobarbolini

[seanmonstar]

Sounds to me like the clippy lint is wrong.

[dodomorandi]

I went through the discussion for the lint, because this was exactly my initial thought. However, after reading the issues related to the implicit copies related to the interior mutability when working with consts, I changed my mind and I think that the clippy lint is right instead.

Let me make a counter-example: if you change from const to static in my simple example, you will notice that you cannot directly use the HEADER_NAME variable anymore, you need to clone it or use a reference (which works because &HeaderName implements IntoHeaderName). We need to agree that the const behaviour makes things much more ergonomic to use, but totally hides copies (which hopefully are optimized away, but I did not check) from the user.

[seanmonstar]

I'd recommend moving this to a different issue, so that this one can stay on "typed headers".