BUG/MAJOR: mux-h2: Be sure to always report HTX parsing error to the app layer

If a parsing error is detected and the corresponding HTX flag is set
(HTX_FL_PARSING_ERROR), we must be sure to always report it to the app
layer. It is especially important when the error occurs during the response
parsing, on the server side. In this case, the RX buffer contains an empty
HTX message to carry the flag. And it remains in this state till the info is
reported to the app layer. This must be done otherwise, on the conn-stream,
the CS_FL_ERR_PENDING flag cannot be switched to CS_FL_ERROR and the
CS_FL_WANT_ROOM flag is always set when h2_rcv_buf() is called. The result
is a ping-pong loop between the mux and the stream.

Note that this patch fixes a bug. But it also reveals a design issue. The
error must not be reported at the HTX level. The error is already carried by
the conn-stream. There is no reason to duplicate it. In addition, it is
errorprone to have an empty HTX message only to report the error to the app
layer.

This patch should fix the issue #1561. It must be backported as far as 2.0
but the bug only affects HAProxy >= 2.4.

(cherry picked from commit ec361bbd843781fb15ebbfca6aea57455d3ac3f8)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

diff --git a/src/mux_h2.c b/src/mux_h2.c
index 767ffe9..b60e38b 100644 (file)
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -6358,7 +6358,7 @@ static size_t h2_rcv_buf(struct conn_stream *cs, struct buffer *buf, size_t coun
 
        /* transfer possibly pending data to the upper layer */
        h2s_htx = htx_from_buf(&h2s->rxbuf);
-       if (htx_is_empty(h2s_htx)) {
+       if (htx_is_empty(h2s_htx) && !(h2s_htx->flags & HTX_FL_PARSING_ERROR)) {
                /* Here htx_to_buf() will set buffer data to 0 because
                 * the HTX is empty.
                 */