fix: canonicalize Host header into :authority for outbound HTTP/2

[magurotuna]

When sending HTTP/2 requests or push promises, promote the first valid Host header value to :authority and strip all Host headers from regular fields. This prevents emitting conflicting :authority and Host on the wire, aligning with RFC 9113 and the behavior of other HTTP/2 client implementations such as curl, Go's std, and Python's httpx.

Fixes #876

[seanmonstar]

@cratelyn @olix0r I feel like we considered this a long time ago, and for transparent proxy purposes, chose not to normalize? But I can't remember if we decided that explicitly, or not. Checking if you have cases where this would be problematic.

[cratelyn]

@cratelyn @olix0r I feel like we considered this a long time ago, and for transparent proxy purposes, chose not to normalize? But I can't remember if we decided that explicitly, or not. Checking if you have cases where this would be problematic.

@seanmonstar i wasn't around for that earlier consideration, but i pinged Oliver to take a look at this. i suspect he'll be better positioned to remember the details here 🙂 thank you for checking in with us!

[cratelyn]

hi @seanmonstar, thank you for your patience.

looking at what this function does...

/// Canonicalize `Host` header into `:authority` pseudo-header for HTTP/2.
///
/// - If a `Host` header is present, attempt to parse its first value as a URI authority.
/// - On success, override `:authority` with the parsed value.
/// - Always remove all `Host` headers from the regular header map.

i believe the relevant portions of RFC 9113 are found in section 8.3.1. the strict rules i see for clients are:

• Clients that generate HTTP/2 requests directly MUST use the ":authority" pseudo-header field
• Clients MUST NOT generate a request with a Host header field that differs from the ":authority" pseudo-header field

so host headers are fine for clients to emit, so long as they do not differ from the ":authority" pseudo-header. removing them unconditionally would, i think, be going a step too far.

for intermediaries, the rules i see are:

• An intermediary that forwards a request over HTTP/2 MUST construct an ":authority" pseudo-header field using the authority information from the control data of the original request, unless the original request's target URI does not contain authority information (in which case it MUST NOT generate ":authority"
• An intermediary that forwards a request over HTTP/2 MAY retain any Host header field.
• An intermediary that needs to generate a Host header field (which might be necessary to construct an HTTP/1.1 request) MUST use the value from the ":authority" pseudo-header field as the value of the Host field, unless the intermediary also changes the request target. This replaces any existing Host field to avoid potential vulnerabilities in HTTP routing.

i think it's reasonable for h2 to enforce RFC 9113 compliance, and avoiding mismatched :authority and Host values seems like a wise decision, particularly because these MUST NOT differ.

i don't think that we should always remove all Host headers, however. nothing says clients MUST NOT generate them, and intermediaries MAY retain them.

ideally, it'd be nice to leave a way for intermediaries to retain the host header field, per https://www.rfc-editor.org/rfc/rfc9113.html#section-8.3.1-2.3.6. that's likely important for intermediaries like us (linkerd) that transport HTTP/1 traffic over an HTTP/2 intermediary layer.

[cratelyn]

nit: this is placed in the middle of the file's use imports. we should move this somewhere lower down. conventionally, following the pattern of other code in this repo, this should go after the struct definitions (including the eoponymous Headers type).

[cratelyn]

for future maintainers, it would also be helpful to add mention of the specific passages of RFC 9113 that this relates to. that would help keep the rationale of this code less implicit.