Cookie Header should be Map-like

[seanmonstar]

We only ever receive headers like this: Cookie: foo=bar; session=sean; hello=world, and having that be a vector of ["foo=bar", "session=sean", "hello=world"] doesn't really help anyone.

Instead, anyone needing to accept cookies would only be looking for a certain name anyways, so this code would be more appropriate:

let maybe_session = req.headers().get::<Cookie>().map(|cookie| cookie.get("session"));

Being a map also helps deal with a common mistake naïve users could make: duplicate names should just be dropped.

Creation of a Cookie header should likewise use a map-like interface.

let mut cookie = Cookie::new();
cookie.set("session", "sean");

// maybe an extend API also? Though we could easily defer to a later release
let pairs = [("foo", "bar"), ("hello", "world")];
cookie.extend(&pairs);

[seanmonstar]

For implementation, something like the VecMap inside Headers would probably be best, since it performs faster and with less memory than a HashMap when the number of items is small. I can't imagine the Cookie header, in the common case, having that many key-value pairs.

So, it could look like:

pub struct Cookie(VecMap<Cow<'static, str>, Cow<'static, str>>);

impl Cookie {
    pub fn new() -> Cookie;
    pub fn get(&self, name: &str) -> Option<&str>;
    pub fn set<K: Into<Cow<'static, str>>, V: Into<Cow<'static, str>>>(&mut self, name: K, value: V);
}

[dorfsmay]

1. We need to drop duplicate cookies, but in a very specific order. I believe the RFC does not specify the order, but most servers (all?) accept the first instance of each key only due to the way clients send them (higher scope first).

2. I was originally going to replace the vec of string by a HashMaps, but decided against it as this would break backwards compatibility, but totally agree that this is the way to go. As explained in feat(method): provide HashMap from Cookie header #1142, I feel that the current implementation of the Cookie header is not a good implementation of what a Cookie header is.

What is the process to inject a breaking change?

[seanmonstar]

Well, a breaking change would require a version bump (since we're before 1.0, that means the minor version). Good news is that we're about to do that anyways, with the async changes, to 0.11. So, if done quickly, it can ride that breakage together.

Regarding duplicate cookie names, I just did some more searching. The situation is worse than I thought. The spec explicitly leaves it undefined when there are multiple cookies with the same name. "[...] servers SHOULD NOT rely upon the order in which these cookies appear in the header." Thanks, RFC. It seems many libraries in many languages just grab the first one. That may not be the behavior determined in the spec, but it seems like it has become the de-facto behavior. The internet is fun...

[dorfsmay]

My understanding is that servers should not rely on it, but because all clients send cookies the same way (narrower, more pertinent scope first), accepting the first key of a series of duplicate is what all servers do, and not doing so could create security issues.

Should I take a first stab at implement a new struct for the "Cookie" header, something along the method in #1142, but as the struct instead?

[seanmonstar]

That should be fine. And anyone really needing the duplicate headers can always check headers.get_raw. I'd suggest starting with an implementation following #1145 (comment).