BUG/MEDIUM: h1-htx: don't stall on messages ending exactly at the buffer's end

h1_parse_msg_hdrs() realigns the input buffer when its data wrap, then
passes b_tail() as the end of the area to parse. But when the data end
exactly at the end of the buffer's area (head + data == size), they do not
wrap so no realign is performed, while b_tail() wraps to the buffer's
origin, blocking parsing of headers. This can only happen with pipelined
requests.

Because of this issue, the parsing of headers can be blocked waiting for
more data and the http-request timeout can be reached, especially if it
happens exactly on the end of headers for a bodyless request.

In #3540, the same issue led to a 414 error returned on a valid request.

Parsing of trailers don't have the issue because it compares b_peek(ofs) to
b_tail(), but it misses the case of a full buffer whose head is not at the
beginning (start == end, without wrapping).

To fix both, the area is realigned only if the data really wrap, and the
end of the area to parse is computed with b_stop(), which points to the
byte following the end of the data, even when they end exactly at the
end of the buffer's area: b_stop() only wraps after the end of the area,
never to the buffer's origin, contrary to b_tail().

This bug exists since the H1 mux was introduced.

This patch should fix the issue #3540. It must be backported to all supported
versions.

diff --git a/src/h1_htx.c b/src/h1_htx.c
index aaaca5ad4008ac1b9d609636da6f48d4226f052a..590a74bbdf792423cf6420a2ea857e8e6e98306e 100644 (file)
--- a/src/h1_htx.c
+++ b/src/h1_htx.c
@@ -437,7 +437,12 @@ int h1_parse_msg_hdrs(struct h1m *h1m, union h1_sl *h1sl, struct htx *dsthtx,
                /* If there no start-line, be sure to only parse the headers */
                h1m->flags |= H1_MF_HDRS_ONLY;
        }
-       ret = h1_headers_to_hdr_list(b_peek(srcbuf, ofs), b_tail(srcbuf),
+       /* The buffer was realigned above, so <b_stop()> safely points to the
+        * byte following the end of the data. Note that b_tail() must not be
+        * used instead: it wraps to the buffer's origin when the data end
+        * exactly at the end of the buffer.
+        */
+       ret = h1_headers_to_hdr_list(b_peek(srcbuf, ofs), b_stop(srcbuf),
                                     hdrs, sizeof(hdrs)/sizeof(hdrs[0]), h1m, h1sl);
        if (ret <= 0) {
                /* Incomplete or invalid message. If the input buffer only
@@ -1026,13 +1031,18 @@ int h1_parse_msg_tlrs(struct h1m *h1m, struct htx *dsthtx,
                goto output_full;
        }
 
-       /* Realing input buffer if necessary */
-       if (b_peek(srcbuf, ofs) > b_tail(srcbuf))
+       /* Realign input buffer if necessary */
+       if (b_head(srcbuf) + b_data(srcbuf) > b_wrap(srcbuf))
                b_slow_realign_ofs(srcbuf, trash.area, 0);
 
        tlr_h1m.flags = (H1_MF_NO_PHDR|H1_MF_HDRS_ONLY);
        tlr_h1m.err_pos = h1m->err_pos;
-       ret = h1_headers_to_hdr_list(b_peek(srcbuf, ofs), b_tail(srcbuf),
+       /* The buffer was realigned above, so <b_stop()> safely points to the
+        * byte following the end of the data. Note that b_tail() must not be
+        * used instead: it wraps to the buffer's origin when the data end
+        * exactly at the end of the buffer.
+        */
+       ret = h1_headers_to_hdr_list(b_peek(srcbuf, ofs), b_stop(srcbuf),
                                     hdrs, sizeof(hdrs)/sizeof(hdrs[0]), &tlr_h1m, NULL);
        if (ret <= 0) {
                /* Incomplete or invalid trailers. If the input buffer only