feat(headers): add strict-transport-security header

[samfoo]

Strict-Transport-Security allows servers to inform user-agents that
they'd like them to always contact the secure host (https) instead of
the insecure one (http).

Closes #589

[seanmonstar]

Impressive work! I'll look through this, but seems travis is reporting this doesn't build on stable...

[seanmonstar]

It seems slightly odd that the header outputs with the boolean at the end, but the struct has it first.

Also, booleans have a habit of being confusing without the explicit name right next to it. How about instead:

pub struct StrictTransportSecurity {
    pub max_age: u64,
    pub includes_subdomains: bool,
}

Additionally, and I'm not sure if the value this might provide carries its own weight, but maybe StrictTransportSecurity::including_subdomain(1_000_000) and StrictTransportSecurity::excluding_subdomain(1_000_000) constructors might be useful?

[seanmonstar]

I appreciated the design of the StsParser, it seems well designed to handle many different directives. However, I have 2 concerns.

1. There's only 2 possible directives that are defined and understood for this header.
2. Several Strings are allocated through the parsing process, including ones that are discarded immediately, such as from pop_whitespace. Parsing could be done with less allocations, which would make it faster and use less memory.

I'd recommend a parsing strategy that splits on ;, and then again on =. That, along with a trim(), could be compared with max-age and includeSubdomains. An example can be seen in the Range header.

[samfoo]

@seanmonstar, I would have preferred a simple split, but the spec allows for quoted strings (which could contain ; and, in the future, more than just the two directives. It specifically says that directives not supported by a UA should be ignored, which is why I implemented the way I did. Perhaps that's too much YAGNI? To be honest, a split is probably fine for 100% of cases at present. Thoughts?

I'm happy to alter the parsing if that's the concern. My rust is a bit ahem neolithic, so it's probably just my lack of understanding idioms. I'll go through and clean up pointlessly allocated Strings

I used a tuple struct reluctantly, since it seemed to be the way most of the other headers were implemented. I'd prefer a field struct too, so I'll update that as well as the constructors.

[seanmonstar]

The usage of tuple structs is usually for when it's a single value, and so something like ContentLength(100) reads better than ContentLength { value: 100 } (at least, in my opinion).

And, nice catch on it being valid to have foo="bar; baz", which is different from foo=bar; baz. While in the current case, any instance where there is a ; inside a quoted string means it's a value that no directive knows, I'm not against future-proofing. If that check is kept, I'd prefer if the parser could just work on an Iterator<Item=u8>, such as raw.iter(), and then instead of pushing bytes into temporary Strings, the positions could just be recorded instead, since only a &str slice is needed for the matching.

[samfoo]

I'm hoping this addresses the performance/allocation concerns. There's now only one allocation per-directive-name -- try as I might to please the borrow-checker gods, I cannot seem to convince that compiler that I'm not really borrowing *self twice.

[Ryman]

If this is the allocation you are referring, I think you can convince the compiler by changing a number of the signatures to fn foo(&mut self) -> &'a str to unlink the return value's lifetime from the mutable borrow of self (lifetime elision) and instead link it to the structs buffer.

[samfoo]

Thanks @Ryman! Changed so now there's no allocation.

(sidenote: I was confused because the error message was talking about borrowing *self, not the &str...)

[seanmonstar]

I noticed a few other details, which I tried to just fix myself in the parser (duplicate directives should be an error, some unwrap()s I'd rather not have, and bounds checks after already checking self.eof()), and I ended up just rewriting the parsing to use splits as I had mentioned earlier in this thread. Please don't take this the wrong way, I greatly appreciate the work and PR that you sent. I simply felt there were a couple flaws that made me feel I was fighting the parser when I tried to correct them.

I've rebased my fixes onto your commit, and merged manually as 7c2e512

[samfoo]

@seanmonstar No worries, mate. No offence taken. Like I said before, I didn't think it was necessarily the best code to start with. Glad to have it merged.