RepoMirrors
/
haproxy
mirror of http://git.haproxy.org/git/haproxy.git/
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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.
This commit is contained in:
Christopher Faulet 2019-02-19 15:14:23 +01:00
parent 834eee7928
commit 0b46548a68
3 changed files with 28 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
include/common/h2.h
View File

@ -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)

11
src/h2.c
View File

@ -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;

22
src/mux_h2.c
View File

@ -3411,10 +3411,14 @@ static int h2c_decode_headers(struct h2c *h2c, struct buffer *rxbuf, uint32_t *f
}
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;