BUG/MEDIUM: h2/htx: Correctly handle interim responses when HTX is enabled

1xx responses does not work in HTTP2 when the HTX is enabled. First of all, when
a response is parsed, only one HEADERS frame is expected. So when an interim
response is received, the flag H2_SF_HEADERS_RCVD is set and the next HEADERS
frame (for another interim repsonse or the final one) is parsed as a trailers
one. Then when the response is sent, because an EOM block is found at the end of
the interim HTX response, the ES flag is added on the frame, closing too early
the stream. Here, it is a design problem of the HTX. Iterim responses are
considered as full messages, leading to some ambiguities when HTX messages are
processed. This will not be fixed now, but we need to keep it in mind for future
improvements.

To fix the parsing bug, the flag H2_MSGF_RSP_1XX is added when the response
headers are decoded. When this flag is set, an EOM block is added into the HTX
message, despite the fact that there is no ES flag on the frame. And we don't
set the flag H2_SF_HEADERS_RCVD on the corresponding H2S. So the next HEADERS
frame will not be parsed as a trailers one.

To fix the sending bug, the ES flag is not set on the frame when an interim
response is processed and the flag H2_SF_HEADERS_SENT is not set on the
corresponding H2S.

This patch must be backported to 1.9.

diff --git a/include/common/h2.h b/include/common/h2.h
index c434ed9fb02a6dc6afc703d5c6d3d8fa8aebd447..9b9684d2de59732784f3af20959c09a75fe95fc2 100644 (file)
--- a/include/common/h2.h
+++ b/include/common/h2.h
@@ -177,6 +177,7 @@ enum h2_err {
 #define H2_MSGF_BODY           0x0001    // a body is present
 #define H2_MSGF_BODY_CL        0x0002    // content-length is present
 #define H2_MSGF_BODY_TUNNEL    0x0004    // a tunnel is in use (CONNECT)
+#define H2_MSGF_RSP_1XX        0x0010    // a 1xx ( != 101) HEADERS frame was received
 
 #define H2_MAX_STREAM_ID       ((1U << 31) - 1)
 #define H2_MAX_FRAME_LEN       ((1U << 24) - 1)

diff --git a/src/h2.c b/src/h2.c
index d7031c75d91c5be9b89a9be741588c8bd1f9a82d..a4578f468d0afe32b31092b7e40b829f8f5ba14b 100644 (file)
--- a/src/h2.c
+++ b/src/h2.c
@@ -780,6 +780,17 @@ static struct htx_sl *h2_prepare_htx_stsline(uint32_t fields, struct ist *phdr,
 
        sl->info.res.status = h * 100 + t * 10 + u;
 
+       /* On 1xx responses (except 101) there is no ES on the HEADERS frame but
+        * there is no body. So remove the flag H2_MSGF_BODY and add
+        * H2_MSGF_RSP_1XX to notify the decoder another HEADERS frame is
+        * expected.
+        */
+       if (sl->info.res.status < 200 &&
+           (sl->info.res.status == 100 || sl->info.res.status >= 102)) {
+               *msgf |= H2_MSGF_RSP_1XX;
+               *msgf &= ~H2_MSGF_BODY;
+       }
+
        return sl;
  fail:
        return NULL;

diff --git a/src/mux_h2.c b/src/mux_h2.c
index 62b8b00144c0911dab5681e85f2141c872017c1c..17b8afaefb0a32b275b5b1771ce27914acb03b83 100644 (file)
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -3411,10 +3411,14 @@ next_frame:
        }
 
  done:
-       /* indicate that a HEADERS frame was received for this stream */
-       *flags |= H2_SF_HEADERS_RCVD;
+       /* indicate that a HEADERS frame was received for this stream, except
+        * for 1xx responses. For 1xx responses, another HEADERS frame is
+        * expected.
+        */
+       if (!(msgf & H2_MSGF_RSP_1XX))
+               *flags |= H2_SF_HEADERS_RCVD;
 
-       if (h2c->dff & H2_F_HEADERS_END_STREAM) {
+       if ((h2c->dff & H2_F_HEADERS_END_STREAM) || (msgf & H2_MSGF_RSP_1XX)) {
                /* Mark the end of message, either using EOM in HTX or with the
                 * trailing CRLF after the end of trailers. Note that DATA_CHNK
                 * is not set during headers with END_STREAM.
@@ -4255,11 +4259,12 @@ static size_t h2s_htx_frt_make_resp_headers(struct h2s *h2s, struct htx *htx)
                }
        }
 
-       /* we may need to add END_STREAM.
+       /* we may need to add END_STREAM except for 1xx responses.
         * FIXME: we should also set it when we know for sure that the
         * content-length is zero as well as on 204/304
         */
-       if (blk_end && htx_get_blk_type(blk_end) == HTX_BLK_EOM)
+       if (blk_end && htx_get_blk_type(blk_end) == HTX_BLK_EOM &&
+           (h2s->status >= 200 || h2s->status == 101))
                es_now = 1;
 
        if (h2s->cs->flags & CS_FL_SHW)
@@ -4273,7 +4278,12 @@ static size_t h2s_htx_frt_make_resp_headers(struct h2s *h2s, struct htx *htx)
 
        /* commit the H2 response */
        b_add(&h2c->mbuf, outbuf.data);
-       h2s->flags |= H2_SF_HEADERS_SENT;
+
+       /* indicates the HEADERS frame was sent, except for 1xx responses. For
+        * 1xx responses, another HEADERS frame is expected.
+        */
+       if (h2s->status >= 200 || h2s->status == 101)
+               h2s->flags |= H2_SF_HEADERS_SENT;
 
        if (es_now) {
                h2s->flags |= H2_SF_ES_SENT;