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.

diff --git a/src/mux_h2.c b/src/mux_h2.c
index 767ffe965a28f98d8aa07769309e7b824909ef9c..b60e38b05b8ce0f73365cde44b3f81705d6c3540 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.
                 */