commit 9a408abbb8559df5718bc696bd9c3934c6500d63 Author: Willy Tarreau Date: Fri Aug 23 08:11:36 2019 +0200 BUG/MEDIUM: mux-h1: do not truncate trailing 0CRLF on buffer boundary The H1 message parser calls the various message block parsers with an offset indicating where in the buffer to start from, and only consumes the data at the end of the parsing. The headers and trailers parsers have a condition detecting if a headers or trailers block is too large to fit into the buffer. This is detected by an incomplete block while the buffer is full. Unfortunately it doesn't take into account the fact that the block may be parsed after other blocks that are still present in the buffer, resulting in aborting some transfers early as reported in issue #231. This typically happens if a trailers block is incomplete at the end of a buffer full of data, which typically happens with data sizes multiple of the buffer size minus less than the trailers block size. It also happens with the CRLF that follows the 0-sized chunk of any transfer-encoded contents is itself on such a boundary since this CRLF is technically part of the trailers block. This can be reproduced by asking a server to retrieve exactly 31532 or 31533 bytes of static data using chunked encoding with curl, which reports: transfer closed with outstanding read data remaining This issue was revealed in 2.0 and does not affect 1.9 because in 1.9 the trailers block was processed at once as part of the data block processing, and would simply give up and wait for the rest of the data to arrive. It's interesting to note that the headers block parsing is also affected by this issue but in practice it has a much more limited impact since a headers block is normally only parsed at the beginning of a buffer. The only case where it seems to matter is when dealing with a response buffer full of 100-continue header blocks followed by a regular header block, which will then be rejected for the same reason. This fix must be backported to 2.0 and partially to 1.9 (the headers block part). (cherry picked from commit 347f464d4e5a8a2bf3acd2411a6c8228e605e7f6) Signed-off-by: Willy Tarreau diff --git a/src/mux_h1.c b/src/mux_h1.c index fa694c41..01f225a2 100644 --- a/src/mux_h1.c +++ b/src/mux_h1.c @@ -995,10 +995,11 @@ static size_t h1_process_headers(struct h1s *h1s, struct h1m *h1m, struct htx *h ret = h1_headers_to_hdr_list(b_peek(buf, *ofs), b_tail(buf), hdrs, sizeof(hdrs)/sizeof(hdrs[0]), h1m, &h1sl); if (ret <= 0) { - /* Incomplete or invalid message. If the buffer is full, it's an - * error because headers are too large to be handled by the - * parser. */ - if (ret < 0 || (!ret && !buf_room_for_htx_data(buf))) + /* Incomplete or invalid message. If the input buffer only + * contains headers and is full, which is detected by it being + * full and the offset to be zero, it's an error because + * headers are too large to be handled by the parser. */ + if (ret < 0 || (!ret && !*ofs && !buf_room_for_htx_data(buf))) goto error; goto end; } @@ -1339,10 +1340,11 @@ static size_t h1_process_trailers(struct h1s *h1s, struct h1m *h1m, struct htx * ret = h1_headers_to_hdr_list(b_peek(buf, *ofs), b_tail(buf), hdrs, sizeof(hdrs)/sizeof(hdrs[0]), &tlr_h1m, NULL); if (ret <= 0) { - /* Incomplete or invalid trailers. If the buffer is full, it's - * an error because traliers are too large to be handled by the - * parser. */ - if (ret < 0 || (!ret && !buf_room_for_htx_data(buf))) + /* Incomplete or invalid trailers. If the input buffer only + * contains trailers and is full, which is detected by it being + * full and the offset to be zero, it's an error because + * trailers are too large to be handled by the parser. */ + if (ret < 0 || (!ret && !*ofs && !buf_room_for_htx_data(buf))) goto error; goto end; }