How to fix headers

[paolobarbolini]

#626, #672 and #690 have shown how hard it is to do headers in email. #685 tries to fix the first issue by correctly encoding the Content-Disposition header. That PR though uses an hack to bypass the generic header value encoder and use the Content-Disposition specific encoder. How do we expand the fix to the rest of the headers? Here's a few option with advantages and disadvantages:

1. Just do the correct encoding for every header

...and keep using Strings to represent header values

Advantages

• Simple
• Keeps both typed and raw header APIs

Disadvantages

• Raw header APIs would be vulnerable to header value injections, since users may forget to run the header value through one of the encoders
• We have to support header decoding in order for example to allow appending values to headers
  • This also means having to support RFC 2047 and RFC 2231 decoding

2. Always encode raw headers, just like we do now

Advantages

• Solves the above header value injection vulnerability by always encoding raw headers
• Still fairly simple

Disadvantages

• We still have to support header value decoding, like the 1st option
• The raw header API would be a bit too limited, by not allowing pre-encoded raw header values

3. Add an HeaderValue struct

...which allows specifying whether the value is pre-encoded or needs encoding

Advantages

• Fixes the header injection vulnerability in the raw header API while allowing pre-encoded raw headers

Disadvantages

• Very easy to construct a raw HeaderValue when the provided value isn't getting encoded in any way, so it would still be easy to mistakenly construct a non encoded raw header
• We still have to do header value decoding :sob:

4. Have a rustls style builder ^1 (for building messages)

Advantages

• We don't have to implement header decoding anymore - yay #661 ([..] why do I need to implement parse for headers I intend to only write [..])
• We keep both the typed and raw APIs
• Can make the header builder less magic. Currently some headers are injected when the Message is built if they weren't provided already by the user

Disadvantages

• Stricter API for building emails (but also an advantage if we make good use of it)
• No support for reading back headers (not because we can't, but because that's the whole point of going this route)
• Not enough to fix the issue with option 3 regarding the raw header API

Example

let email = Message::builder()
    .from(from_address)
    .no_reply_to()
    .to(to_address)
    .no_cc()
    .bcc_many(bcc_addresses_iter)
    .subject("Hello!")
    .content_type(ContentType::TEXT_PLAIN)
    .default_message_id()
    .typed_header(custom_header)
    // Raw headers can be written only after all typed headers have been inserted
    .raw_header("Something", "Yes, it works!")
    .body(plaintext_email);

---

I've been thinking about this for weeks now and when I saw the rustls 0.20 builders an entire new pathway lit up in my mind.

[kevincox]

Another thing to consider at the same time is https://github.com/lettre/lettre/issues/688. To get the best possible wrapping the header itself should express breakpoints, otherwise it is only safe to generically wrap on whitespace.

For example the DKIM-Signature header's b parameter (the signature itself) allows arbitrary whitespace so that it can be wrapped nicely. The wrapping logic should know about this. (This example could be hacked around by just always breaking the signature into 77-char words because we know it is always longer than a line but this hack would produce lots of unnecessary whitespace for shorter fields with optional space. )