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

[tracking] typed headers #99

Open
51 of 72 tasks
yoshuawuyts opened this issue Apr 16, 2020 · 20 comments
Open
51 of 72 tasks

[tracking] typed headers #99

yoshuawuyts opened this issue Apr 16, 2020 · 20 comments

Comments

@yoshuawuyts
Copy link
Member

yoshuawuyts commented Apr 16, 2020

This is a tracking issue for typed headers. I looked over the
HTTP headers page and categorized which headers would need to be ported.

There are probably some nuances in how we want to implement them, but this provides an overview of how far along we are in constructing typed constructors for all header types.

edit: I realize now that this might read a bit dated. I drafted this text back in December but forgot to post it. Some things may seem a bit outdated or different; the most important part is that we can track whether we can construct headers .

Authentication (auth)

  • WWW-Authenticate (auth::Authenticate)
  • Authorization (auth::Authorization)
  • Proxy-Authenticate (auth::ProxyAuthenticate)
  • Proxy-Authorization (auth::ProxyAuthorization)

Caching (cache)

  • Age (cache::Age)
  • Cache-Control (cache::Control)
  • Clear-Site-Data (cache::ClearSite)
  • Expires (cache::Expires)
  • Pragma (cache::Pragma) (HTTP/1.0 only)
  • Warning (cache::Warning) (soon to be deprecated)

Client Hints (client_hints)

  • Accept-CH (ch::Accept) (experimental, implementation postponed)
  • Accept-CH-Lifetime (ch::Lifetime) (experimental, implementation postponed)
  • Early-Data (ch::EarlyData) (experimental, implementation postponed)
  • Content-DPR (ch::ContentDpr) (experimental, implementation postponed)
  • DPR (ch::Dpr) (experimental, implementation postponed)
  • Device-Memory (ch::DeviceMemory) (experimental, implementation postponed)
  • Save-Data (ch::SaveData) (experimental, implementation postponed)
  • Viewport-Width (ch::ViewportWidth) (experimental, implementation postponed)
  • Width (ch::Width) (experimental, implementation postponed)

Conditionals (conditionals)

  • Last-Modified (conditionals::LastModified)
  • Etag (conditionals::Etag)
  • If-Match (conditionals::IfMatch)
  • If-Modified-Since (conditionals::IfModifiedSince)
  • If-Unmodified-Since (conditionals::IfUnmodifiedSince)
  • Vary (conditionals::Vary)

Content Negotiation (content)

  • Accept (content::Accept)
  • Accept-Charset (content::Charset) (no longer supported by any browser)
  • Accept-Encoding (content::Encoding)
  • Accept-Language (content::Language)

Controls (controls)

  • Expect (controls::Expect)

Cookies

I think this one is special, and we should re-export the
cookie crate, and just expose get /
set cookies as methods on the Request and Response types. Let's just treat
them as built-ins to evade the name collision.

CORS (cors)

  • Access-Control-Allow-Origin (cors::AllowOrigin)
  • Access-Control-Allow-Credentials (cors::AllowCredentials)
  • Access-Control-Allow-Headers (cors::AllowHeaders)
  • Access-Control-Allow-Methods (cors::AllowMethods)
  • Access-Control-Expose-Headers (cors::ExposeHeaders)
  • Access-Control-Max-Age (cors::MaxAge)
  • Access-Control-Request-Headers (cors::RequestHeaders)
  • Access-Control-Request-Method (cors::RequestMethod)
  • Access-Control-Origin (cors::Origin)
  • Access-Control-Timing-Allow-Origin (cors::TimingAllowOrigin)

Do Not Track (privacy)

  • DNT (privacy::DoNotTrack)
  • Tk (privacy::TrackingStatus)

Content Information (content)

  • Content-Disposition (downloads::Disposition)
  • Content-Length (content::Length)
  • Content-Type (content::Type)
  • Content-Encoding (content::Encoding)
  • Content-Language (content::Language)
  • Content-Location (content::Location)

Proxies (proxies)

  • Forwarded (proxies::Forwarded)
  • Via (proxies::Via)

Response (response)

  • Allow (response::Allow)

Range (content)

  • Accept-Ranges (range::Accept)
  • Range (range::Range)
  • If-Range (range::If)
  • Content-Range (range::Content)

Security (security)

  • Cross-Origin-Opener-Policy (security::Coop)
  • Cross-Origin-Resource-Policy (security::Cors)
  • Content-Security-Policy (security::Csp)
  • Content-Security-Policy-Report-Only (security::Cspro)
  • Expect-CT (security::Cspro)
  • Feature-Policy (security::FeaturePolicy)
  • Public-Key-Pins (security::HPKP) (deprecated)

Other

  • Alt-Svc
  • Date
  • Large-Allocation (unspecced; firefox only)
  • Link
  • Referer
  • Retry-After
  • SourceMap
  • Upgrade
  • User-Agent
  • Keep-Alive
@Licenser
Copy link
Contributor

A good part of them are implemented in #107

@yoshuawuyts
Copy link
Member Author

Example for a typed expires constructor:

use http_types::cache::Expires;
use std::time::{SystemTime, Duration};

let when = SystemTime::now() + Duration::from_secs(60 * 60);
let expires = Expires::new(when);
res.insert_header(expires.name(), expires.value());

@ririsoft
Copy link
Contributor

ririsoft commented Jun 7, 2020

Hello @yoshuawuyts ,

I have implemented a basic HTTP Range support (no multipart support) for a private project of mine using http-rs/tide. I have implemented a serve_content function, taking inspiration from golang net/http, for serving bodies implementing async Seek.

I would be happy to help implementing this back to http-types, async-h1 and tide. I believe things start with http-types but I am not sure about the API your are expecting "typed headers" to follow. Should I start with your example above and submit a PR for review ?

@yoshuawuyts
Copy link
Member Author

@ririsoft hey! -- that's a good question. I'm not sure either what such an API should look like, but I'm sure we can figure it out. Opening a PR with a usage example would indeed be a great starting point.

@ririsoft
Copy link
Contributor

ririsoft commented Jun 8, 2020

Hello !

Opening a PR with a usage example would indeed be a great starting point.

I will submit one shortly for further brainstorming, addressing the HTTP range headers parsing and validation use case.

@ririsoft
Copy link
Contributor

ririsoft commented Jun 10, 2020

After my implementation of range request headers (PR #180) I am not convinced that a typed header will bring much value over the existing http_types::headers::ToHeaderValues and std::convert::TryFrom<&HeaderValue>.

The only added value I see is the automation of header name handling when fetching header values. For instance we could have the following (simplified version):

Gist: https://gist.github.com/rust-play/07ab2a72218d80531af7225dc0579c17

use std::collections::HashMap;
use std::str::FromStr;
use std::string::ToString;

// Here FromStr should be TryFrom<&HeaderValue>
// and ToString should be ToHeaderValues.
trait TypedHeader: FromStr + ToString {
    const HEADER_NAME: &'static str;
}

struct Headers {
    vals: HashMap<String, String>,
}

impl Headers {
    fn get<T: TypedHeader>(&self) -> Option<Result<T, <T as FromStr>::Err>> {
        self.vals.get(T::HEADER_NAME).map(|v| T::from_str(&v))
    }

    fn set<T: TypedHeader>(&mut self, val: T) {
        self.vals
            .insert(T::HEADER_NAME.to_string(), val.to_string());
    }
}

#[derive(Debug, Eq, PartialEq, Clone)]
struct Range {
    start: u64,
    end: u64,
}

impl ToString for Range {
    fn to_string(&self) -> String {
        format!("bytes={}-{}", self.start, self.end)
    }
}

impl FromStr for Range {
    type Err = &'static str;

    fn from_str(s: &str) -> Result<Self, Self::Err> {
        let err = "invalid range header";
        let mut bounds = s.trim_start_matches("bytes=").split('-');
        let start = bounds
            .next()
            .ok_or(err)
            .and_then(|v| u64::from_str(v).map_err(|_| err))?;
        let end = bounds
            .next()
            .ok_or(err)
            .and_then(|v| u64::from_str(v).map_err(|_| err))?;
        Ok(Range { start, end })
    }
}

impl TypedHeader for Range {
    const HEADER_NAME: &'static str = "Range";
}

fn main() {
    let mut h = Headers {
        vals: HashMap::new(),
    };
    let r = Range {
        start: 10,
        end: 100,
    };
    h.set(r.clone());
    let other = h.get::<Range>();
    assert_eq!(Some(Ok(r)), other);
}

We would need to add some Headers::set_values(&mut self, name: impl Into<HeaderName>, val: imp ToHeaderValues) and get_values equivalent, as well as append methods.

Actually the true additional value of the above implementation is the possibility to get the first header value instead of having to get all headers value and map option to HeaderValues::last().

What do you think ?

@Fishrock123
Copy link
Member

Fishrock123 commented Jun 10, 2020

I think we may be able to think about this better.

After thinking about this since yesterday, I was thinking about my wip port of negotiator - i.e. Accept-* headers, and realized we may want to specify "typed headers" for both Request and Response the same way:

This is derived from the neat api of accepts, a JS lib which builds on negotiator:

  var accept = accepts(req)
 
  // the order of this list is significant; should be server preferred order
  switch (accept.type(['json', 'html'])) {
    case 'json':
      res.setHeader('Content-Type', 'application/json')
      res.write('{"hello":"world!"}')
      break

i.e. You could specify a Range or an Accept like a request would, but also be able to match from it:

req.set_header(AcceptEncoding::new(["gzip", "deflate", "identity"]));
let accept: Option<AcceptEncoding> = res.get_header(AcceptEncoding::new(["gzip", "deflate", "identity"]));
match accept?.get_preferred_match() {
  "gzip" => ...
   ...
};

in this case, Headers::get()/set() would have to end up being something like:

impl Headers {
    fn get<T>(&self, mut header: T) -> Option<T> where T: TypedHeader {
        self.vals.get(T::HEADER_NAME).map(|v| header.populate(&v))
    }

    fn set<T>(&mut self, header: T) where T: TypedHeader {
        self.vals
            .insert(T::HEADER_NAME.to_string(), header.to_string());
    }
}

@ririsoft
Copy link
Contributor

Hi @Fishrock123,

I did not know about negotiator, this is interesting. Thanks for sharing !

It seems we are ending with 2 close APIs with some subtle differences that suite best one case or another. The set method are the same (which is excellent), so let's focus on the get one.

// V1
impl Headers {
    fn get<T>(&self, mut header: T) -> Option<T> where T: TypedHeader {
        self.vals.get(T::HEADER_NAME).map(|v| header.populate(&v))
    }
}
// V2
impl Headers {
fn get<T>(&self) -> Option<Result<T, <T as FromStr>::Err>>  where T: TypedHeader {
       self.vals.get(T::HEADER_NAME).map(|v| T::from_str(&v))
   }
}

I need to mature more on the differences of the two design, please excuse me if I misunderstood some points, but here is what comes to my mind:

When fetching a header value from string we might want to do the following steps:

  1. Know whether it is missing or not.
  2. Validate its format against related HTTP RFC.
  3. Do additional computation specific to the header type.

My concern with V1 design is that it merges all steps above. If I get a None I do not know if this is the result of step 1, 2 or 3. In the case of AcceptEncoding this might not be an issue, but in the case of Range this is because the response flow and status code are different at each of the 3 steps above.

V1 could return a Option<Result<T>> like V2 to address step 2, but it still does not make possible to distinguish step 1 from step 3, unless I missed something.

@Fishrock123
Copy link
Member

// V2
impl Headers {
fn get<T>(&self) -> Option<Result<T, <T as FromStr>::Err>>  where T: TypedHeader {
      self.vals.get(T::HEADER_NAME).map(|v| T::from_str(&v))
   }
}
  1. Do additional computation specific to the header type.

I think that would happen either: when setting the header value, or, when requesting the header value.

Given that, we might be able to do a slight twist on v2 to get something that is even more mirrored for getting and setting:

trait TypedHeader: FromStr + ToString {
    const HEADER_NAME: &'static str;

    fn set_value(value: impl Into<HeaderValue>) -> Result<(), Error>; // #2, #3.
    fn get_value() -> Result<&HeaderValue, Error>; // #2, #3.
}

impl Headers {
    fn get<T>(&self) -> Option<T>  where T: TypedHeader { // #1
       self.vals.get(T::HEADER_NAME).map(|v| T::from_str(&v))
   }

    fn set<T>(&mut self, header: T) where T: TypedHeader {
        self.vals
            .insert(T::HEADER_NAME.to_string(), header.to_string());
    }
}

Perhaps that's not quite flexible enough for get_value() and set_value() though, in terms of arguments and return type. There might be a better way.

@yoshuawuyts
Copy link
Member Author

yoshuawuyts commented Jul 23, 2020

I've made a PR for another typed header impl in #203. I've also noted down the patterns we're using in http_types as part of this gist. The intent was never to have a separation between "typed headers" and regular headers, but instead use common method names + trait conversions to create a consistent API.

Header design overview

This is the current snapshot of the gist above. We're already using this API for http_types::security::ContentSecurityPolicy, and now also in #203. We need to adjust the unstable TraceContext headers to follow this API as well. What this API provides is:

  • 3 constructors: new, from_headers, TryFrom<HeaderValue>.
  • 4 conversions to header types: apply, name, value, ToHeaderValues.
  • 4 trait impls: Debug, Clone, Eq, PartialEq.

Together these form a base template that should work for all typed header implementations.

use crate::headers::{HeaderName, HeaderValue, Headers, ToHeaderValues};

struct CustomHeader {}

impl CustomHeader {
    /// Create a new instance of `CustomHeader`.
    fn new() -> Self {
        todo!();
    }

    /// Create an instance of `CustomHeader` from a `Headers` instance.
    fn from_headers(headers: impl AsRef<Headers>) -> crate::Result<Option<Self>> {
        todo!();
    }

    /// Insert a `HeaderName` + `HeaderValue` pair into a `Headers` instance.
    fn apply(&self, headers: impl AsMut<Headers>) {
        todo!();
    }

    /// Get the `HeaderName`.
    fn name(&self) -> HeaderName {
        todo!();
    }

    /// Get the `HeaderValue`.
    fn value(&self) -> HeaderValue {
        todo!();
    }
}

// Conversion from `CustomHeader` -> `HeaderValue`.
impl ToHeaderValues for CustomHeader {
    type Iter;
    fn to_header_values(&self) -> crate::Result<Self::Iter> {
        todo!()
    }
}

@Fishrock123
Copy link
Member

I think there's still a couple things missing here.

For example, from_headers() should probably return Option<Self>.

Also this doesn't really answer any questions regarding header value matching/verification...

I suppose for, say, content encoding matching you would do something like this:

if let Some(header) = ContentEncoding::from_headers(req) {
    if let Some(encoding) = header.accepts(&["br", "gzip"]) {
        // `encoding` is the preferred encoding here
    }
}

@yoshuawuyts
Copy link
Member Author

Oops, yeah from_headers ought to probably return Result<Option<Self>> in order to catch both malformed requests and the absence of requests.

I'm not quite sure what the issue is with the example? I'm not sure I would nest if let Some statements when using it, but overall that looks like a reasonable API to me?

@ririsoft
Copy link
Contributor

Oops, yeah from_headers ought to probably return Result<Option<Self>> in order to catch both malformed requests and the absence of requests.

While experiencing with such APIs on my side I wonder whether Option<Result<Self>> shouldn't be used instead ? The errors might occur only if the header exists after all. When using a Result<Option<Self>> I end with a lot more of Option::transpose or Result::transpose harness.

@Fishrock123
Copy link
Member

I'm not sure I would nest if let Some statements when using it, but overall that looks like a reasonable API to me?

How would you write that?

Also if we are considering Option<Result<>> or similar, it's probably best to just use Result<> with a "none" error variant, similar to http-rs/tide@3778706 ?

@Fishrock123
Copy link
Member

That being said on further thought maybe header errors should just be "none" unless inspected otherwise?

@Fishrock123
Copy link
Member

Fishrock123 commented Oct 13, 2020

Added to list:

@utherpally
Copy link

WWW-Authenticate header: From RFC 7235#appendix-A, the "realm" parameter is optional now.

@yoshuawuyts
Copy link
Member Author

@langbamit thanks for reporting; I've filed #280 to track it for the next major.

@yoshuawuyts
Copy link
Member Author

Idempotentency-Key IETF draft. Perhaps we should implement this as a separate crate (submodule) until it stabilizes?

@Fishrock123
Copy link
Member

IMO: implement draft headers under some compile time flag with unstable-like reduced guarantees.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants