BUG/MEDIUM: mux-h2: do not abort HEADERS frame before decoding them

If a response HEADERS frame arrives on a closed connection (due to a
client abort sending an RST_STREAM), it's currently immediately rejected
with an RST_STREAM, like any other frame. This is incorrect, as HEADERS
frames must first be decoded to keep the HPACK decoder synchronized,
possibly breaking subsequent responses.

This patch excludes HEADERS/CONTINUATION/PUSH_PROMISE frames from the
central closed state test and leaves to the respective frame parsers
the responsibility to decode the frame then send RST_STREAM.

This fix must be backported to 1.9. 1.8 is not directly impacted since
it doesn't have response HEADERS nor trailers thus cannot recover from
such situations anyway.

diff --git a/src/mux_h2.c b/src/mux_h2.c
index 3a47d392312f9a0d2096924bc6429e6a93d23664..badd905d1042c7f5e46a6b98051df553e3c77855 100644 (file)
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -2364,7 +2364,7 @@ static void h2_process_demux(struct h2c *h2c)
                                goto strm_err;
                        }
 
-                       if (h2s->flags & H2_SF_RST_RCVD) {
+                       if (h2s->flags & H2_SF_RST_RCVD && h2_ft_bit(h2c->dft) & H2_FT_HDR_MASK) {
                                /* RFC7540#5.1:closed: an endpoint that
                                 * receives any frame other than PRIORITY after
                                 * receiving a RST_STREAM MUST treat that as a
@@ -2372,6 +2372,13 @@ static void h2_process_demux(struct h2c *h2c)
                                 *
                                 * Note that old streams fall into this category
                                 * and will lead to an RST being sent.
+                                *
+                                * However, we cannot generalize this to all frame types. Those
+                                * carrying compression state must still be processed before
+                                * being dropped or we'll desynchronize the decoder. This can
+                                * happen with request trailers received after sending an
+                                * RST_STREAM, or with header/trailers responses received after
+                                * sending RST_STREAM (aborted stream).
                                 */
                                h2s_error(h2s, H2_ERR_STREAM_CLOSED);
                                h2c->st0 = H2_CS_FRAME_E;