Hi Lukas,
On Sat, Sep 01, 2018 at 08:02:45PM +0200, Lukas Tribus wrote:
> Hi Willy,
>
>
> haproxy is currently unable to handle CONTINUATION [1] frames (see
> commit 61290ec77 - [2]).
>
> If a client emits a CONTINUATION frame, we will break the connection
> and send GOAWAY due to INTERNAL_ERROR. This of course leads to
> interoperability issues.
Yes, but arguably these issues should be rare because a client is
expected to fill its frames, so all requests smaller than 16 kB
compressed make no sense to be sent with CONTINUATION.
> Notably, older Chrome/Chromium releases (50.0.2645.4 and older),
> lacking Chromium commit a8bff211e ("Increase sent control frame
> fragment size to 16 kB") [3], will cap the maximum control frame size
> at 1024 bytes, triggering CONTINUATION, when large headers are used
> (long URIs, or large cookies).
Hmmm this is a particularly abusive behaviour considering that the
sole purpose of the CONTINUATION frames was to support extremely
large headers (and these ones were debated a lot) :-(
> How to reproduce?
> In Chrome/Chromium 50.0.2645.4 or older, access a very long URI on a
> h2 enabled haproxy instance. For example on cdn.haproxy.com [4].
>
> Why are old Chrome/Chromium relevant?
> Chrome 49 is the latest release that runs under Windows XP. Yes, I
> know, but apparently there are large market segments out there where
> this has a real business impact.
Sure!
> Chrome 49 and Windows XP support
> situation a side, this is not a Chrome bug, but a lack of support of a
> H2 feature in haproxy. Other corner cases may trigger this issue as
> well.
Definitely.
> At least 4 different people reported this as an actual
> interoperability problem with H2 on discourse [5], [6], additionally
> we have 2 reports of results of a RFC7540 test [7], [8], which also
> fails due to at least lack of CONTINUATION support.
>
> While this issue is arguably not major - there would have been more
> reports otherwise, I do think we should address this issue.
I wanted to address it but the CONTINUATION frame is the worst design
mistake of the H2 protocol and results in layering violations which
make it particularly problematic to implement. In short, while all
frames are independant on each other, this one must absolutely appear
after an existing HEADERS frame and requires to maintain a huge state
of unparsed data to feed to the HPACK decompressor at once after
reassembly. It's even documented in the spec that it can be the source
of a DoS... I'm not saying I don't want to implement them at all, it's
just that in the current state of things, I don't have a clean solution
to propose. We'll have more options once the connection layers are
reversed because we'll have the option to try to parse both HEADERS
and CONTINUATION at once into a temporary buffer and try to decompress
the result. But in any case, CONTINUATION frames support will always
be limited (ie too large HEADERS+CONTINUATION may lead to connection
aborts by placing the decompressor in an unrecoverable state).
> As far as I can see, other major h2 implementations do implement
> CONTINUATION support, including nginx and nghttp2.
And that's sad. It would have been better if most didn't implement this
crap, to get rid of the non-legitimate use cases once for all :-/
I will probably revisit this point after 1.9, at least for protocol
completeness, maybe with an option such as
"waste-memory-and-cpu-to-support-stupid-h2-continuation-frames" or
something as explicit to save people from enabling them by accident :-/
Cheers,
Willy
