98 lines
4.0 KiB
Diff
98 lines
4.0 KiB
Diff
From b5d2b9e154d78e4075db163826c5e0f6d31b2ab1 Mon Sep 17 00:00:00 2001
|
|
From: Willy Tarreau <w@1wt.eu>
|
|
Date: Wed, 11 Aug 2021 15:39:13 +0200
|
|
Subject: [PATCH] BUG/MEDIUM: h2: give :authority precedence over Host
|
|
MIME-Version: 1.0
|
|
Content-Type: text/plain; charset=utf8
|
|
Content-Transfer-Encoding: 8bit
|
|
|
|
The wording regarding Host vs :authority in RFC7540 is ambiguous as it
|
|
says that an intermediary must produce a host header from :authority if
|
|
Host is missing, but, contrary to HTTP/1.1, doesn't say anything regarding
|
|
the possibility that Host and :authority differ, which leaves Host with
|
|
higher precedence there. In addition it mentions that clients should use
|
|
:authority *instead* of Host, and that H1->H2 should use :authority only
|
|
if the original request was in authority form. This leaves some gray
|
|
area in the middle of the chain for fully valid H2 requests arboring a
|
|
Host header that are forwarded to the other side where it's possible to
|
|
drop the Host header and use the authority only after forwarding to a
|
|
second H2 layer, thus possibly seeing two different values of Host at
|
|
a different stage. There's no such issue when forwarding from H2 to H1
|
|
as the authority is dropped only only the Host is kept.
|
|
|
|
Note that the following request is sufficient to re-normalize such a
|
|
request:
|
|
|
|
http-request set-header host %[req.hdr(host)]
|
|
|
|
The new spec in progress (draft-ietf-httpbis-http2bis-03) addresses
|
|
this trouble by being a bit is stricter on these rules. It clarifies
|
|
that :authority must always be used instead of Host and that Host ought
|
|
to be ignored. This is much saner as it avoids to convey two distinct
|
|
values along the chain. This becomes the protocol-level equivalent of:
|
|
|
|
http-request set-uri %[url]
|
|
|
|
So this patch does exactly this, which we were initially a bit reluctant
|
|
to do initially by lack of visibility about other implementations'
|
|
expectations. In addition it slightly simplifies the Host header field
|
|
creation by always placing it first in the list of headers instead of
|
|
last; this could also speed up the look up a little bit.
|
|
|
|
This needs to be backported to 2.0. Non-HTX versions are safe regarding
|
|
this because they drop the URI during the conversion to HTTP/1.1 so
|
|
only Host is used and transmitted.
|
|
|
|
Thanks to Tim Düsterhus for reporting that one.
|
|
---
|
|
src/h2.c | 23 +++++++++++++++++++++--
|
|
1 file changed, 21 insertions(+), 2 deletions(-)
|
|
|
|
diff --git a/src/h2.c b/src/h2.c
|
|
index 280bbd7..bbc5853 100644
|
|
--- a/src/h2.c
|
|
+++ b/src/h2.c
|
|
@@ -456,10 +456,27 @@ int h2_make_htx_request(struct http_hdr *list, struct htx *htx, unsigned int *ms
|
|
if (!sl)
|
|
goto fail;
|
|
fields |= H2_PHDR_FND_NONE;
|
|
+
|
|
+ /* http2bis draft recommends to drop Host in favor of :authority when
|
|
+ * the latter is present. This is required to make sure there is no
|
|
+ * discrepancy between the authority and the host header, especially
|
|
+ * since routing rules usually involve Host. Here we already know if
|
|
+ * :authority was found so we can emit it right now and mark the host
|
|
+ * as filled so that it's skipped later.
|
|
+ */
|
|
+ if (fields & H2_PHDR_FND_AUTH) {
|
|
+ if (!htx_add_header(htx, ist("host"), phdr_val[H2_PHDR_IDX_AUTH]))
|
|
+ goto fail;
|
|
+ fields |= H2_PHDR_FND_HOST;
|
|
+ }
|
|
}
|
|
|
|
- if (isteq(list[idx].n, ist("host")))
|
|
+ if (isteq(list[idx].n, ist("host"))) {
|
|
+ if (fields & H2_PHDR_FND_HOST)
|
|
+ continue;
|
|
+
|
|
fields |= H2_PHDR_FND_HOST;
|
|
+ }
|
|
|
|
if (isteq(list[idx].n, ist("content-length"))) {
|
|
ret = h2_parse_cont_len_header(msgf, &list[idx].v, body_len);
|
|
@@ -531,7 +548,9 @@ int h2_make_htx_request(struct http_hdr *list, struct htx *htx, unsigned int *ms
|
|
/* update the start line with last detected header info */
|
|
sl->flags |= sl_flags;
|
|
|
|
- /* complete with missing Host if needed */
|
|
+ /* complete with missing Host if needed (we may validate this test if
|
|
+ * no regular header was found).
|
|
+ */
|
|
if ((fields & (H2_PHDR_FND_HOST|H2_PHDR_FND_AUTH)) == H2_PHDR_FND_AUTH) {
|
|
/* missing Host field, use :authority instead */
|
|
if (!htx_add_header(htx, ist("host"), phdr_val[H2_PHDR_IDX_AUTH]))
|
|
--
|
|
1.7.10.4
|
|
|