Hyper does not re-join cookie headers split by http/2

[NeoLegends]

As per https://tools.ietf.org/html/rfc7540#section-8.1.2.5 http/2 allows splitting cookie headers into multiple parts if it improves compression efficiency. The spec mandates that the header needs to be rejoined if the associated request is proxied to a http/1 server, though.

To allow for better compression efficiency, the Cookie header field
MAY be split into separate header fields, each with one or more
cookie-pairs. If there are multiple Cookie header fields after
decompression, these MUST be concatenated into a single octet string
using the two-octet delimiter of 0x3B, 0x20 (the ASCII string "; ")
before being passed into a non-HTTP/2 context, such as an HTTP/1.1
connection, or a generic HTTP server application.

Currently, the Hyper server exposes the split cookie header as separate entries in the HeaderMap, but if the request is then proxied to a h1 server, the Client does not re-join them again (although it probably should).

I do not think it's desirable to always and forcibly re-join cookie headers before sending them out on http/1 connections, given that hyper is a low-level library and users might want to experiment with that. It might also be a breaking change for users of this library.

Therefore, to limit the blast radius of the change, I propose to only re-join headers iff the incoming request was h2 and the outgoing one is h1. Hyper neatly exposes the HTTP version on the request, which should let one do that. I'd be happy to send a PR implementing that.

Eventually, I think, hyper's server should re-join the cookie header transparently, before invoking the actual server-Service. This would then conform with the last sentence of the spec paragraph above.

[nox]

I guess it could do it there:

hyper/src/proto/h1/role.rs

Lines 1033 to 1036 in 4e9a006

	Version::HTTP_2 => {
	debug!("request with HTTP2 version coerced to HTTP/1.1");
	extend(dst, b"HTTP/1.1");
	}

Edit: fixed the link, which was previously pointing at the server encode function.

[seanmonstar]

Just curious, is this something that hyper itself should do? Or a responsibility of the proxy using hyper?

[NeoLegends]

This is a really interesting point. I've had some discussion about this with @nox on Discord, and IMO this mainly depends on whether you would consider the proxy service "generic" (as per the spec).

Since there are http-2 only proxies or services, I am not sure whether hyper should blindly merge Cookie values on http/2 connections before handing the request off to the service.

@nox and me came to the conclusion that it would make sense to implement the minimal approach first, where the hyper client merges cookie headers if it's coercing a h2 request into a h1 request, such that the "bad" requests at least don't leave the server in their broken (from an h1 perspective) state. I filed #2529 to implement this. Would appreciate your review!

In our production service we added a layer that merged the cookies, so working around this w/o hyper support is definitely not hard -- just surprising.

[nox]

Just curious, is this something that hyper itself should do? Or a responsibility of the proxy using hyper?

I feel like the only reason something would ever pass a h2 request to a h1 origin is that it is a proxy, and then the less expensive way to merge those headers is in the header encode loop deep in the role.rs file. But I don't have a strong opinion on this.