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

Stringly-typed HeaderValue? #136

Open
srijs opened this Issue Nov 6, 2017 · 11 comments

Comments

Projects
None yet
10 participants
@srijs

srijs commented Nov 6, 2017

Hi!

Hyper made the design decision to having properly-typed header values, and cites good reasons for it: https://docs.rs/hyper/0.11.6/hyper/header/index.html

I was wondering if it was an explicit design decision in http to not do this, and if so, could someone go into the reasons?

I'm genuinely curious if this could be a nice addition to http, especially since it seems hyper is planning to move to http-provided types in a future version, or whether there are strong reasons against it.

@steveklabnik

This comment has been minimized.

Show comment
Hide comment
@srijs

This comment has been minimized.

Show comment
Hide comment
@srijs

srijs Nov 6, 2017

Cool, thanks @steveklabnik! @seanmonstar, what does this mean for hyper specifically? Will hyper continue to provide the typed header abstraction on top of http?

srijs commented Nov 6, 2017

Cool, thanks @steveklabnik! @seanmonstar, what does this mean for hyper specifically? Will hyper continue to provide the typed header abstraction on top of http?

@seanmonstar

This comment has been minimized.

Show comment
Hide comment
@seanmonstar

seanmonstar Nov 7, 2017

Member

hyper will eventually replace its usage of Request and Response for those in http (and thus, the associated types like method, status, headers, etc). It needs to do this to in order to allow hyper to be used with frameworks who don't specifically use hyper, but just http.

The typed headers will be moved a separate crate, I imagine. Related hyper issue.

Member

seanmonstar commented Nov 7, 2017

hyper will eventually replace its usage of Request and Response for those in http (and thus, the associated types like method, status, headers, etc). It needs to do this to in order to allow hyper to be used with frameworks who don't specifically use hyper, but just http.

The typed headers will be moved a separate crate, I imagine. Related hyper issue.

@carllerche

This comment has been minimized.

Show comment
Hide comment
@carllerche

carllerche Nov 7, 2017

Collaborator

I have some thoughts re: typed headers (for outside the http crate), but I haven't explored them fully.

Collaborator

carllerche commented Nov 7, 2017

I have some thoughts re: typed headers (for outside the http crate), but I haven't explored them fully.

@mikeyhew

This comment has been minimized.

Show comment
Hide comment
@mikeyhew

mikeyhew Feb 28, 2018

@seanmonstar from reddit, linked above

While I still feel that typed headers are a fantastic thing that Rust can provide, they don't fit in http. Some people want them, some people hate them and just use headers.set_raw and headers.get_raw. And, the typed headers do have an overhead to them (it's very minimal, but it's not free).

Can you explain the overhead, and is there a longer explanation why it was decided that typed headers don't fit into the http crate? I feel that since http is supposed to be the standard HTTP crate that the community uses, it is important that a decision like this be explained in detail.

mikeyhew commented Feb 28, 2018

@seanmonstar from reddit, linked above

While I still feel that typed headers are a fantastic thing that Rust can provide, they don't fit in http. Some people want them, some people hate them and just use headers.set_raw and headers.get_raw. And, the typed headers do have an overhead to them (it's very minimal, but it's not free).

Can you explain the overhead, and is there a longer explanation why it was decided that typed headers don't fit into the http crate? I feel that since http is supposed to be the standard HTTP crate that the community uses, it is important that a decision like this be explained in detail.

@sagebind

This comment has been minimized.

Show comment
Hide comment
@sagebind

sagebind Feb 28, 2018

Just to chime in pseudo-randomly, I personally prefer the "stringly-typed" header values how they are in this crate. Maybe I just have a bad "get off my lawn" attitude, but I know the HTTP headers and generally how they should be written. This crate should worry about the basic HTTP types needed for interoperability, and not whether I'm reading or writing the right content in my HTTP messages. Moreover, I think a big use case is to translate headers from "raw" sources (HTTP parsers, (Fast)CGI, etc.) where typed header values are of little use anyway.

It's also a lot easier to put a typed abstraction on a string-based approach than the other way around.

sagebind commented Feb 28, 2018

Just to chime in pseudo-randomly, I personally prefer the "stringly-typed" header values how they are in this crate. Maybe I just have a bad "get off my lawn" attitude, but I know the HTTP headers and generally how they should be written. This crate should worry about the basic HTTP types needed for interoperability, and not whether I'm reading or writing the right content in my HTTP messages. Moreover, I think a big use case is to translate headers from "raw" sources (HTTP parsers, (Fast)CGI, etc.) where typed header values are of little use anyway.

It's also a lot easier to put a typed abstraction on a string-based approach than the other way around.

@ChristophWurst ChristophWurst referenced this issue Apr 17, 2018

Closed

Document the new changes #1463

6 of 6 tasks complete
@vorot93

This comment has been minimized.

Show comment
Hide comment
@vorot93

vorot93 Jun 2, 2018

Typed headers got lost in transition to Hyper 0.12 after all. What a shame! Now downstream has to take extra care every time a Request or Response is crafted not to screw up.

vorot93 commented Jun 2, 2018

Typed headers got lost in transition to Hyper 0.12 after all. What a shame! Now downstream has to take extra care every time a Request or Response is crafted not to screw up.

@sfackler

This comment has been minimized.

Show comment
Hide comment
@sfackler

sfackler Jun 2, 2018

Contributor

@vorot93 https://crates.io/crates/hyperx is one option for typed headers.

Contributor

sfackler commented Jun 2, 2018

@vorot93 https://crates.io/crates/hyperx is one option for typed headers.

@fdubois1

This comment has been minimized.

Show comment
Hide comment
@fdubois1

fdubois1 Jun 5, 2018

Is there an easy way to rebuild the typed headers from the HeaderValue ? Because from what I understood, now, the HeaderMap::get(key) will return a HeaderValue.

For example, I used to write something like this :
req.headers().get::()

But now, the get is defined like this :
pub fn get(&self, key: K) -> Option<&T>

The key, if I understand correctly is http::header::IF_MATCH. So I should do something like this :
headers().get(::http::header::IF_MATCH) and this will return a HeaderValue. How can I build the IfMatch type from that ?

Thanks

fdubois1 commented Jun 5, 2018

Is there an easy way to rebuild the typed headers from the HeaderValue ? Because from what I understood, now, the HeaderMap::get(key) will return a HeaderValue.

For example, I used to write something like this :
req.headers().get::()

But now, the get is defined like this :
pub fn get(&self, key: K) -> Option<&T>

The key, if I understand correctly is http::header::IF_MATCH. So I should do something like this :
headers().get(::http::header::IF_MATCH) and this will return a HeaderValue. How can I build the IfMatch type from that ?

Thanks

@saghm

This comment has been minimized.

Show comment
Hide comment
@saghm

saghm Oct 11, 2018

Hi @fdubois1! I've just run into the same issue myself, and I think I can provide at least a partial solution. Assuming you know which header you want, you can use HeaderMap::get to obtain the HeaderValue that you want, using either a static string or the constant provided by the library (e.g. for the Link header, you can either pass the actual string "link" or this constant). HeaderValue has a to_str method, which lets you get the raw string value of the header, which you can then use to construct the header by using its FromStr implementation. All together, it would look something like this (with the caveat that I haven't compiled or run this snippet):

let header_value = headers.get(::http::header::LINK);
let raw_string = header_value.to_str()?;
let link_header = ::hyperx::header::Link::from_str(raw_string)?;

Converting the entire HeaderMap into a list of hyperx::Header values would be a bit more work, but hopefully this is sufficient for what you need!

saghm commented Oct 11, 2018

Hi @fdubois1! I've just run into the same issue myself, and I think I can provide at least a partial solution. Assuming you know which header you want, you can use HeaderMap::get to obtain the HeaderValue that you want, using either a static string or the constant provided by the library (e.g. for the Link header, you can either pass the actual string "link" or this constant). HeaderValue has a to_str method, which lets you get the raw string value of the header, which you can then use to construct the header by using its FromStr implementation. All together, it would look something like this (with the caveat that I haven't compiled or run this snippet):

let header_value = headers.get(::http::header::LINK);
let raw_string = header_value.to_str()?;
let link_header = ::hyperx::header::Link::from_str(raw_string)?;

Converting the entire HeaderMap into a list of hyperx::Header values would be a bit more work, but hopefully this is sufficient for what you need!

@carllerche

This comment has been minimized.

Show comment
Hide comment
@carllerche

carllerche Oct 16, 2018

Collaborator

There is ongoing work to provide a crate that includes typed headers.

Collaborator

carllerche commented Oct 16, 2018

There is ongoing work to provide a crate that includes typed headers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment