Skip to content
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

Original Header Cases API #2695

Open
3 of 4 tasks
seanmonstar opened this issue Nov 16, 2021 · 12 comments
Open
3 of 4 tasks

Original Header Cases API #2695

seanmonstar opened this issue Nov 16, 2021 · 12 comments
Labels
A-http1 Area: HTTP/1 specific. B-rfc Blocked: More comments would be useful in determine next steps. C-feature Category: feature. This is adding a new feature.

Comments

@seanmonstar
Copy link
Member

This is a tracking issue for the feature to allow for sending and receiving the exact casing of header names, instead of the forced lowercasing of http. In most situations, simply forcing to lowercase is the better option, as it allows for a more performant HeaderMap, and is the required format for newer versions of HTTP (HTTP/2 and 3). However, there are rare occasions when being able to see the exact casing is needed.

Some initial support is available in hyper, and is exposed in the C API. But a public Rust API is not yet available.

@seanmonstar seanmonstar added C-feature Category: feature. This is adding a new feature. A-http1 Area: HTTP/1 specific. B-rfc Blocked: More comments would be useful in determine next steps. labels Nov 16, 2021
@kulame

This comment was marked as outdated.

@seanmonstar

This comment was marked as outdated.

@kulame

This comment was marked as outdated.

@tasn
Copy link

tasn commented Nov 22, 2022

We've also been bitten by this unfortunately. We know and realize that HTTP headers should be lower case.

Alas, some of our customers have compatibility requirements because they've been sending them as upper case (or camel case, depending) and some of their customers rely on them being of specific casing.

I'd be really grateful for even an experimental feature that we could enable to support this, because our current (terrible) alternative is to maintain our own Hyper fork. :(

@svix-jplatte

This comment was marked as off-topic.

@seanmonstar

This comment was marked as off-topic.

@svix-jplatte
Copy link

svix-jplatte commented Mar 28, 2024

How about this as a public API?

pub trait HttpMessageExt {
    fn headers_case_sensitive(&self) -> CaseSensitiveHeaderMap<'_>;
    fn headers_case_sensitive_mut(&mut self) -> CaseSensitiveHeaderMapMut<'_>;
}

impl<T> HttpMessageExt for http::Request<T> { /* ... */ }
impl<T> HttpMessageExt for http::Response<T> { /* ... */ }

// CaseSensitiveHeaderMap and CaseSensitiveHeaderMapMut methods would be more or less obvious, I guess?

For CaseSensitiveHeaderMapMut to work, we'd need extra API in http to allow mutably borrowing headers and extensions simultaneously, but I think that would "just" be a matter of

// request.rs
impl<T> Request<T> {
    pub fn parts_mut(&mut self) -> (PartsMut<'_>, &mut T) { /* ... */ }
}

pub struct PartsMut<'a> {
    /// The request's method
    pub method: &'a mut Method,

    /// The request's URI
    pub uri: &'a mut Uri,

    /// The request's version
    pub version: &'a mut Version,

    /// The request's headers
    pub headers: &'a mut HeaderMap<HeaderValue>,

    /// The request's extensions
    pub extensions: &'a mut Extensions,

    _priv: (),
}

// response.rs - equivalent with slightly different fields

@seanmonstar
Copy link
Member Author

Hm. What would be the specific way a user would interact with this? What would the methods of HeaderCaseMap look like?

@svix-jplatte
Copy link

svix-jplatte commented Apr 2, 2024

Some examples

let headers = response.headers_case_sensitive();
// Check for a header using the exact provided casing
if let Some(val) = message.headers_case_sensitive().get("Foo-Bar") {
    // read val: HeaderValue
}

let mut headers = request.headers_case_sensitive();
// Insert a header using the exact provided casing
headers.insert("Foo-Bar", <header value>);

Methods-wise, I could see this type having all of the methods (not constructors of course) of HeaderMap plus even a few _ignore_case ones (if you want to check an existing header ignoring casing to insert a new one with specific casing from one map object), but starting small, these seems like the important ones:

impl<'a> CaseSensitiveHeaderMap<'a> {
    pub fn contains_key(&self, key: impl AsCaseSensitiveHeaderName) -> bool;
    pub fn get(&self, key: impl AsaseSensitiveHeaderName) -> Option<&'a HeaderValue>;
    // GetAll<'a>: IntoIterator<Item = &'a HeaderValue>
    pub fn get_all(&self, key: impl AsCaseSensitiveHeaderName) -> GetAll<'a>;
    // Iter<'a>: Iterator<Item = (&'a CaseSensitiveHeaderName, &'a HeaderValue)
    pub fn iter(&self) -> Iter<'a>;
}

impl<'a> CaseSensitiveHeaderMapMut<'a> {
    // same methods as CaseSensitiveHeaderMap
    pub fn append(&mut self, key: impl IntoCaseSensitiveHeaderName, value: HeaderValue) -> bool;
    pub fn insert(&mut self, key: impl IntoCaseSensitiveHeaderName, value: HeaderValue) -> Option<HeaderValue>;
    // also try_append, try_insert
    pub fn get_mut(&mut self, key: impl IntoCaseSensitiveHeaderName) -> Option<&mut HeaderValue);
    pub fn remove(&mut self, key: impl IntoCaseSensitiveHeaderName) -> Option<HeaderValue>;
    // IterMut<'a>: Iterator<Item = (&'a CaseSensitiveHeaderName, &'a mut HeaderValue)>
    pub fn iter_mut(&mut self) -> IterMut<'_>;
}

pub trait AsCaseSensitiveHeaderName: hdr_as::Sealed {}
pub trait IntoCaseSensitiveHeaderName: hdr_into::Sealed {}

// similar impls, sealed traits as for upstream traits

#[repr(transparent)]
struct CaseSensitiveHeaderName([u8]);

@seanmonstar
Copy link
Member Author

Here's my suggestion: I'd aim for just figuring out the minimal methods needed to expose the existing hyper::ext::HeaderCaseMap. That would allow for enabling advanced users to use something quicker, and an eventual type that manages both HeaderMap and the extension together can be bikeshedded after.

@svix-jplatte
Copy link

svix-jplatte commented Aug 29, 2024

Alright, I can definitely work on that. At what level do you want to encapsulate things, though? Do you want the actual type stored in request / response extensions to be private? If yes, why? If not, sounds like the minimal API would just be what's already there, just made public, right?

@seanmonstar
Copy link
Member Author

Sorta. The methods that currently exist were just the easiest to work internally. No care was taken regarding flexibility, forwards compatibility, what would be easier to extend in the future. What makes it impossible to refactor the internals if we try to make it faster. Those sorts of questions.

Like, should the values just be Bytes? That doesn't validate that the name is a legal header name. We either expose a type that represents a legal any cased header name, or we require validating when adding. Otherwise, it's a bit too easier to cause message splitting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-http1 Area: HTTP/1 specific. B-rfc Blocked: More comments would be useful in determine next steps. C-feature Category: feature. This is adding a new feature.
Projects
None yet
Development

No branches or pull requests

4 participants