On Mon, Nov 19, 2018 at 11:55:04PM +0100, Willy Tarreau wrote:
> > I assume this is a bug in the HPACK encoder, given that in the static
> > table definition [1], accept-language has index 17, while
> > accept-ranges has 18, which is correctly documented in
> > src/hpack-tbl.c, but the comment on line 105 in src/hpack-enc.c makes
> > me doubt our implementation:
> > 
> > out->str[len++] = 0x51; // literal with indexing --
> > name="accept-ranges" (idx 17)
> > 
> > 
> > Too much string magic going on there for me to provide prompt
> > solution, but I assume this will be a quick fix for Willy.
> 
> Gloups! I'm quite ashamed, totally ashamed, almost red. I'll take a
> look at this tomorrow. Thanks for the report!

I've just pushed the fix. I'm attaching the backported version for your
convenience (as it will not apply as-is to 1.8).

Thanks!
Willy
>From 2a8572fa6edf400c66ce0e9263e57d7985b9e0d1 Mon Sep 17 00:00:00 2001
From: Willy Tarreau <w...@1wt.eu>
Date: Tue, 20 Nov 2018 04:47:38 +0100
Subject: BUG/MEDIUM: hpack: fix encoding of "accept-ranges" field

James Brown reported that when an "accept-ranges" header field is sent
through haproxy and converted from HTTP/1.1 to H2, it's mis-encoded as
"accept-language". It happens that it's one of the few very common header
fields encoded using its index value and that this index value was misread
in the spec as 17 instead of 18, resulting in the wrong name being sent.
Thanks to Lukas for spotting the issue in the HPACK encoder itself.

This fix must be backported to 1.8.

(cherry picked from commit 4bf194cbdbcda8ec4ce83d7f12d2fe9b08483c94)
---
 src/hpack-enc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/hpack-enc.c b/src/hpack-enc.c
index d1f68c5..99c7310 100644
--- a/src/hpack-enc.c
+++ b/src/hpack-enc.c
@@ -101,7 +101,7 @@ int hpack_encode_header(struct chunk *out, const struct ist 
n, const struct ist
        else if (isteq(n, ist("last-modified")))
                out->str[len++] = 0x6c; // literal with indexing -- 
name="last-modified" (idx 44)
        else if (isteq(n, ist("accept-ranges")))
-               out->str[len++] = 0x51; // literal with indexing -- 
name="accept-ranges" (idx 17)
+               out->str[len++] = 0x52; // literal with indexing -- 
name="accept-ranges" (idx 18)
        else if (isteq(n, ist("cache-control")))
                out->str[len++] = 0x58; // literal with indexing -- 
name="cache-control" (idx 24)
        else if (isteq(n, ist("content-length")))
-- 
2.8.0.rc2.1.gbe9624a

Reply via email to