diff --git a/include/haproxy/h1_htx.h b/include/haproxy/h1_htx.h index 099055846..61b96e084 100644 --- a/include/haproxy/h1_htx.h +++ b/include/haproxy/h1_htx.h @@ -29,13 +29,13 @@ #include #include -size_t h1_parse_msg_hdrs(struct h1m *h1m, union h1_sl *h1sl, struct htx *dsthtx, - struct buffer *srcbuf, size_t ofs, size_t max); +int h1_parse_msg_hdrs(struct h1m *h1m, union h1_sl *h1sl, struct htx *dsthtx, + struct buffer *srcbuf, size_t ofs, size_t max); size_t h1_parse_msg_data(struct h1m *h1m, struct htx **dsthtx, struct buffer *srcbuf, size_t ofs, size_t max, struct buffer *htxbuf); -size_t h1_parse_msg_tlrs(struct h1m *h1m, struct htx *dsthtx, - struct buffer *srcbuf, size_t ofs, size_t max); +int h1_parse_msg_tlrs(struct h1m *h1m, struct htx *dsthtx, + struct buffer *srcbuf, size_t ofs, size_t max); /* Returns the URI of an HTX message in the most common format for a H1 peer. It * is the path part of an absolute URI when the URI was normalized, ortherwise diff --git a/reg-tests/http-messaging/http_request_buffer.vtc b/reg-tests/http-messaging/http_request_buffer.vtc index d9cc6aec8..8ed683be7 100644 --- a/reg-tests/http-messaging/http_request_buffer.vtc +++ b/reg-tests/http-messaging/http_request_buffer.vtc @@ -12,6 +12,12 @@ server s1 { rxreq expect req.bodylen == 257 txresp + + accept + + rxreq + expect req.bodylen == 2 + txresp } -start syslog S -level info { @@ -19,6 +25,10 @@ syslog S -level info { expect ~ "[^:\\[ ]*\\[[0-9]*\\]: .* .* fe1 fe1/ .* 408 .* - - cD-- .* .* \"GET /this-is-a-long-url-this-is-a-long-url-this-is-a-long-url-this-is-a-long-url-this-is-a-long-url-this-is-a-long-url-this-is-a-long-url HTTP/1\\.1\"" recv expect ~ "[^:\\[ ]*\\[[0-9]*\\]: .* .* fe1 be1/srv1 [0-9]*/[0-9]*/[0-9]*/[0-9]*/[0-9]* 200 .* - - ---- .* .* \"GET / HTTP/1\\.1\"" + recv + expect ~ "[^:\\[ ]*\\[[0-9]*\\]: .* .* fe1 be1/srv1 [0-9]*/[0-9]*/[0-9]*/[0-9]*/[0-9]* 200 .* - - ---- .* .* \"POST /1 HTTP/1\\.1\"" + recv + expect ~ "[^:\\[ ]*\\[[0-9]*\\]: .* .* fe1 be1/ [0-9]*/-1/-1/-1/[0-9]* -1 .* - - CR-- .* .* \"POST /2 HTTP/1\\.1\"" } -start haproxy h1 -conf { @@ -39,6 +49,8 @@ haproxy h1 -conf { use_backend be1 } -start +# 1 byte of the payload is missing. +# ==> The request must timed out with a 408 response client c1 -connect ${h1_fe1_sock} { send "GET" send " " @@ -66,11 +78,33 @@ client c1 -connect ${h1_fe1_sock} { expect resp.status == 408 } -run +# Payload is fully sent +# ==> Request must be sent to the server. A 200 must be received client c2 -connect ${h1_fe1_sock} { txreq -bodylen 257 rxresp expect resp.status == 200 } -run -syslog S -wait +# Payload is fully sent in 2 steps (with a small delay, smaller than the client +# timeout) and splitted on a chunk size. +# ==> Request must be sent to the server. A 200 must be received +client c3 -connect ${h1_fe1_sock} { + send "POST /1 HTTP/1.1\r\nTransfer-Encoding: chunked\r\n\r\n1\r\n1\r\n1" + delay 0.01 + send "\r\n1\r\n0\r\n\r\n" + rxresp + expect resp.status == 200 +} -run +# Last CRLF of the request payload is missing but payload is sent in 2 steps +# (with a small delay, smaller than the client timeout) and splitted on a chunk +# size. The client aborts before sending the last CRLF. +# ==> Request must be handled as an error with 'CR--' termination state. +client c4 -connect ${h1_fe1_sock} { + send "POST /2 HTTP/1.1\r\nTransfer-Encoding: chunked\r\n\r\n1\r\n1\r\n1" + delay 0.01 + send "\r\n1\r\n0\r\n" +} -run + +syslog S -wait diff --git a/src/h1_htx.c b/src/h1_htx.c index 8600c52d1..331b79d3f 100644 --- a/src/h1_htx.c +++ b/src/h1_htx.c @@ -165,9 +165,7 @@ static int h1_postparse_req_hdrs(struct h1m *h1m, union h1_sl *h1sl, struct htx if (h1_eval_htx_size(meth, uri, vsn, hdrs) > max) { if (htx_is_empty(htx)) goto error; - h1m_init_req(h1m); - h1m->flags |= (H1_MF_NO_PHDR|H1_MF_CLEAN_CONN_HDR); - return 0; + goto output_full; } /* By default, request have always a known length */ @@ -216,11 +214,15 @@ static int h1_postparse_req_hdrs(struct h1m *h1m, union h1_sl *h1sl, struct htx end: return 1; + output_full: + h1m_init_req(h1m); + h1m->flags |= (H1_MF_NO_PHDR|H1_MF_CLEAN_CONN_HDR); + return -2; error: h1m->err_pos = h1m->next; h1m->err_state = h1m->state; htx->flags |= HTX_FL_PARSING_ERROR; - return 0; + return -1; } /* Postprocess the parsed headers for a response and convert them into an htx @@ -274,9 +276,7 @@ static int h1_postparse_res_hdrs(struct h1m *h1m, union h1_sl *h1sl, struct htx if (h1_eval_htx_size(vsn, status, reason, hdrs) > max) { if (htx_is_empty(htx)) goto error; - h1m_init_res(h1m); - h1m->flags |= (H1_MF_NO_PHDR|H1_MF_CLEAN_CONN_HDR); - return 0; + goto output_full; } if (((h1m->flags & H1_MF_METH_CONNECT) && code >= 200 && code < 300) || code == 101) { @@ -309,26 +309,31 @@ static int h1_postparse_res_hdrs(struct h1m *h1m, union h1_sl *h1sl, struct htx end: return 1; + output_full: + h1m_init_res(h1m); + h1m->flags |= (H1_MF_NO_PHDR|H1_MF_CLEAN_CONN_HDR); + return -2; error: h1m->err_pos = h1m->next; h1m->err_state = h1m->state; htx->flags |= HTX_FL_PARSING_ERROR; - return 0; + return -1; } -/* Parse HTTP/1 headers. It returns the number of bytes parsed if > 0, or 0 if - * it couldn't proceed. Parsing errors are reported by setting the htx flag - * HTX_FL_PARSING_ERROR and filling h1m->err_pos and h1m->err_state fields. This - * functions is responsible to update the parser state and the start-line - * if not NULL. - * For the requests, must always be provided. For responses, may - * be NULL and flags HTTP_METH_CONNECT of HTTP_METH_HEAD may be set. +/* Parse HTTP/1 headers. It returns the number of bytes parsed on success, 0 if + * headers are incomplete, -1 if an error occurred or -2 if it needs more space + * to proceed while the output buffer is not empty. Parsing errors are reported + * by setting the htx flag HTX_FL_PARSING_ERROR and filling h1m->err_pos and + * h1m->err_state fields. This functions is responsible to update the parser + * state and the start-line if not NULL. For the requests, + * must always be provided. For responses, may be NULL and flags + * HTTP_METH_CONNECT of HTTP_METH_HEAD may be set. */ -size_t h1_parse_msg_hdrs(struct h1m *h1m, union h1_sl *h1sl, struct htx *dsthtx, - struct buffer *srcbuf, size_t ofs, size_t max) +int h1_parse_msg_hdrs(struct h1m *h1m, union h1_sl *h1sl, struct htx *dsthtx, + struct buffer *srcbuf, size_t ofs, size_t max) { struct http_hdr hdrs[global.tune.max_http_hdr]; - int ret = 0; + int total = 0, ret = 0; if (!max || !b_data(srcbuf)) goto end; @@ -352,6 +357,7 @@ size_t h1_parse_msg_hdrs(struct h1m *h1m, union h1_sl *h1sl, struct htx *dsthtx, goto error; goto end; } + total = ret; /* messages headers fully parsed, do some checks to prepare the body * parsing. @@ -363,8 +369,9 @@ size_t h1_parse_msg_hdrs(struct h1m *h1m, union h1_sl *h1sl, struct htx *dsthtx, h1m->err_state = h1m->state; goto vsn_error; } - if (!h1_postparse_req_hdrs(h1m, h1sl, dsthtx, hdrs, max)) - ret = 0; + ret = h1_postparse_req_hdrs(h1m, h1sl, dsthtx, hdrs, max); + if (ret < 0) + return ret; } else { if (h1sl && !h1_process_res_vsn(h1m, h1sl)) { @@ -372,8 +379,9 @@ size_t h1_parse_msg_hdrs(struct h1m *h1m, union h1_sl *h1sl, struct htx *dsthtx, h1m->err_state = h1m->state; goto vsn_error; } - if (!h1_postparse_res_hdrs(h1m, h1sl, dsthtx, hdrs, max)) - ret = 0; + ret = h1_postparse_res_hdrs(h1m, h1sl, dsthtx, hdrs, max); + if (ret < 0) + return ret; } /* Switch messages without any payload to DONE state */ @@ -384,13 +392,13 @@ size_t h1_parse_msg_hdrs(struct h1m *h1m, union h1_sl *h1sl, struct htx *dsthtx, } end: - return ret; + return total; error: h1m->err_pos = h1m->next; h1m->err_state = h1m->state; vsn_error: dsthtx->flags |= HTX_FL_PARSING_ERROR; - return 0; + return -1; } @@ -856,13 +864,15 @@ size_t h1_parse_msg_data(struct h1m *h1m, struct htx **dsthtx, return total; } -/* Parse HTTP/1 trailers. It returns the number of bytes parsed if > 0, or 0 if - * it couldn't proceed. Parsing errors are reported by setting the htx flags - * HTX_FL_PARSING_ERROR and filling h1m->err_pos and h1m->err_state fields. This - * functions is responsible to update the parser state . +/* Parse HTTP/1 trailers. It returns the number of bytes parsed on success, 0 if + * trailers are incomplete, -1 if an error occurred or -2 if it needs more space + * to proceed while the output buffer is not empty. Parsing errors are reported + * by setting the htx flags HTX_FL_PARSING_ERROR and filling h1m->err_pos and + * h1m->err_state fields. This functions is responsible to update the parser + * state . */ -size_t h1_parse_msg_tlrs(struct h1m *h1m, struct htx *dsthtx, - struct buffer *srcbuf, size_t ofs, size_t max) +int h1_parse_msg_tlrs(struct h1m *h1m, struct htx *dsthtx, + struct buffer *srcbuf, size_t ofs, size_t max) { struct http_hdr hdrs[global.tune.max_http_hdr]; struct h1m tlr_h1m; @@ -892,8 +902,7 @@ size_t h1_parse_msg_tlrs(struct h1m *h1m, struct htx *dsthtx, if (h1_eval_htx_hdrs_size(hdrs) > max) { if (htx_is_empty(dsthtx)) goto error; - ret = 0; - goto end; + goto output_full; } if (!htx_add_all_trailers(dsthtx, hdrs)) @@ -904,11 +913,13 @@ size_t h1_parse_msg_tlrs(struct h1m *h1m, struct htx *dsthtx, end: return ret; + output_full: + return -2; error: h1m->err_state = h1m->state; h1m->err_pos = h1m->next; dsthtx->flags |= HTX_FL_PARSING_ERROR; - return 0; + return -1; } /* Appends the H1 representation of the request line to the chunk . It diff --git a/src/mux_h1.c b/src/mux_h1.c index be9596bf1..20127cc8d 100644 --- a/src/mux_h1.c +++ b/src/mux_h1.c @@ -42,7 +42,6 @@ #define H1C_F_IN_ALLOC 0x00000010 /* mux is blocked on lack of input buffer */ #define H1C_F_IN_FULL 0x00000020 /* mux is blocked on input buffer full */ #define H1C_F_IN_SALLOC 0x00000040 /* mux is blocked on lack of stream's request buffer */ -/* 0x00000080 unused */ /* Flags indicating the connection state */ #define H1C_F_ST_EMBRYONIC 0x00000100 /* Set when a H1 stream with no conn-stream is attached to the connection */ @@ -73,7 +72,8 @@ #define H1S_F_RX_BLK 0x00100000 /* Don't process more input data, waiting sync with output side */ #define H1S_F_TX_BLK 0x00200000 /* Don't process more output data, waiting sync with input side */ -/* 0x00000004 unused */ +#define H1S_F_RX_CONGESTED 0x00000004 /* Cannot process input data RX path is congested (waiting for more space in channel's buffer) */ + #define H1S_F_REOS 0x00000008 /* End of input stream seen even if not delivered yet */ #define H1S_F_WANT_KAL 0x00000010 #define H1S_F_WANT_TUN 0x00000020 @@ -1370,13 +1370,14 @@ static int h1_search_websocket_key(struct h1s *h1s, struct h1m *h1m, struct htx /* * Parse HTTP/1 headers. It returns the number of bytes parsed if > 0, or 0 if * it couldn't proceed. Parsing errors are reported by setting H1S_F_*_ERROR - * flag. If relies on the function http_parse_msg_hdrs() to do the parsing. + * flag. If more room is requested, H1S_F_RX_CONGESTED flag is set. If relies on + * the function http_parse_msg_hdrs() to do the parsing. */ static size_t h1_handle_headers(struct h1s *h1s, struct h1m *h1m, struct htx *htx, struct buffer *buf, size_t *ofs, size_t max) { union h1_sl h1sl; - size_t ret = 0; + int ret = 0; TRACE_ENTER(H1_EV_RX_DATA|H1_EV_RX_HDRS, h1s->h1c->conn, h1s, 0, (size_t[]){max}); @@ -1386,13 +1387,18 @@ static size_t h1_handle_headers(struct h1s *h1s, struct h1m *h1m, struct htx *ht h1m->flags |= H1_MF_METH_HEAD; ret = h1_parse_msg_hdrs(h1m, &h1sl, htx, buf, *ofs, max); - if (!ret) { + if (ret <= 0) { TRACE_DEVEL("leaving on missing data or error", H1_EV_RX_DATA|H1_EV_RX_HDRS, h1s->h1c->conn, h1s); - if (htx->flags & HTX_FL_PARSING_ERROR) { + if (ret == -1) { h1s->flags |= H1S_F_PARSING_ERROR; TRACE_ERROR("parsing error, reject H1 message", H1_EV_RX_DATA|H1_EV_RX_HDRS|H1_EV_H1S_ERR, h1s->h1c->conn, h1s); h1_capture_bad_message(h1s->h1c, h1s, h1m, buf); } + else if (ret == -2) { + TRACE_STATE("RX path congested, waiting for more space", H1_EV_RX_DATA|H1_EV_RX_HDRS|H1_EV_H1S_BLK, h1s->h1c->conn, h1s); + h1s->flags |= H1S_F_RX_CONGESTED; + } + ret = 0; goto end; } @@ -1463,6 +1469,11 @@ static size_t h1_handle_data(struct h1s *h1s, struct h1m *h1m, struct htx **htx, *ofs += ret; end: + if (b_data(buf) != *ofs && (h1m->state == H1_MSG_DATA || h1m->state == H1_MSG_TUNNEL)) { + TRACE_STATE("RX path congested, waiting for more space", H1_EV_RX_DATA|H1_EV_RX_BODY|H1_EV_H1S_BLK, h1s->h1c->conn, h1s); + h1s->flags |= H1S_F_RX_CONGESTED; + } + TRACE_LEAVE(H1_EV_RX_DATA|H1_EV_RX_BODY, h1s->h1c->conn, h1s, 0, (size_t[]){ret}); return ret; } @@ -1471,22 +1482,28 @@ static size_t h1_handle_data(struct h1s *h1s, struct h1m *h1m, struct htx **htx, * Parse HTTP/1 trailers. It returns the number of bytes parsed if > 0, or 0 if * it couldn't proceed. Parsing errors are reported by setting H1S_F_*_ERROR * flag and filling h1s->err_pos and h1s->err_state fields. This functions is - * responsible to update the parser state . + * responsible to update the parser state . If more room is requested, + * H1S_F_RX_CONGESTED flag is set. */ static size_t h1_handle_trailers(struct h1s *h1s, struct h1m *h1m, struct htx *htx, struct buffer *buf, size_t *ofs, size_t max) { - size_t ret; + int ret; TRACE_ENTER(H1_EV_RX_DATA|H1_EV_RX_TLRS, h1s->h1c->conn, h1s, 0, (size_t[]){max}); ret = h1_parse_msg_tlrs(h1m, htx, buf, *ofs, max); - if (!ret) { + if (ret <= 0) { TRACE_DEVEL("leaving on missing data or error", H1_EV_RX_DATA|H1_EV_RX_BODY, h1s->h1c->conn, h1s); - if (htx->flags & HTX_FL_PARSING_ERROR) { + if (ret == -1) { h1s->flags |= H1S_F_PARSING_ERROR; TRACE_ERROR("parsing error, reject H1 message", H1_EV_RX_DATA|H1_EV_RX_TLRS|H1_EV_H1S_ERR, h1s->h1c->conn, h1s); h1_capture_bad_message(h1s->h1c, h1s, h1m, buf); } + else if (ret == -2) { + TRACE_STATE("RX path congested, waiting for more space", H1_EV_RX_DATA|H1_EV_RX_TLRS|H1_EV_H1S_BLK, h1s->h1c->conn, h1s); + h1s->flags |= H1S_F_RX_CONGESTED; + } + ret = 0; goto end; } @@ -1501,6 +1518,8 @@ static size_t h1_handle_trailers(struct h1s *h1s, struct h1m *h1m, struct htx *h * Process incoming data. It parses data and transfer them from h1c->ibuf into * . It returns the number of bytes parsed and transferred if > 0, or 0 if * it couldn't proceed. + * + * WARNING: H1S_F_RX_CONGESTED flag must be removed before processing input data. */ static size_t h1_process_demux(struct h1c *h1c, struct buffer *buf, size_t count) { @@ -1523,6 +1542,9 @@ static size_t h1_process_demux(struct h1c *h1c, struct buffer *buf, size_t count if (h1s->flags & H1S_F_RX_BLK) goto out; + /* Always remove congestion flags and try to process more input data */ + h1s->flags &= ~H1S_F_RX_CONGESTED; + do { size_t used = htx_used_space(htx); @@ -1596,7 +1618,7 @@ static size_t h1_process_demux(struct h1c *h1c, struct buffer *buf, size_t count } count -= htx_used_space(htx) - used; - } while (!(h1s->flags & (H1S_F_PARSING_ERROR|H1S_F_NOT_IMPL_ERROR|H1S_F_RX_BLK))); + } while (!(h1s->flags & (H1S_F_PARSING_ERROR|H1S_F_NOT_IMPL_ERROR|H1S_F_RX_BLK|H1S_F_RX_CONGESTED))); if (h1s->flags & (H1S_F_PARSING_ERROR|H1S_F_NOT_IMPL_ERROR)) { @@ -1674,8 +1696,16 @@ static size_t h1_process_demux(struct h1c *h1c, struct buffer *buf, size_t count h1s->cs->flags |= CS_FL_EOI; out: - if (h1s_data_pending(h1s) && !htx_is_empty(htx)) + /* When Input data are pending for this message, notify upper layer that + * the mux need more space in the HTX buffer to continue if : + * + * - The parser is blocked in MSG_DATA or MSG_TUNNEL state + * - Headers or trailers are pending to be copied. + */ + if (h1s->flags & (H1S_F_RX_CONGESTED)) { h1s->cs->flags |= CS_FL_RCV_MORE | CS_FL_WANT_ROOM; + TRACE_STATE("waiting for more room", H1_EV_RX_DATA|H1_EV_H1S_BLK, h1c->conn, h1s); + } else { h1s->cs->flags &= ~(CS_FL_RCV_MORE | CS_FL_WANT_ROOM); if (h1s->flags & H1S_F_REOS) {