BUG/MEDIUM: mux-h2: Don't handle pending read0 too early on streams

This patch is similar to the previous one on the fcgi. Same is true for the
H2. But the bug is far harder to trigger because of the protocol cinematic. But
it may explain strange aborts in some edge cases.

A read0 received on the connection must not be handled too early by H2 streams.
If the demux buffer is not empty, the pending read0 must not be considered. The
H2 streams must not be passed in half-closed remote state in
h2s_wake_one_stream() and the CS_FL_EOS flag must not be set on the associated
conn-stream in h2_rcv_buf(). To sum up, it means, if there are still data
pending in the demux buffer, no abort must be reported to the streams.

To fix the issue, a dedicated function has been added, responsible for detecting
pending read0 for a H2 connection. A read0 is reported only if the demux buffer
is empty. This function is used instead of conn_xprt_read0_pending() at some
places.

Note that the HREM stream state should not be used to report aborts. It is
performed on h2s_wake_one_stream() function and it is a legacy of the very first
versions of the mux-h2.

This patch should be backported as far as 2.0. In the 1.8, the code is too
different to apply it like that. But it is probably useless because the mux-h2
can only be installed on the client side.

diff --git a/src/mux_h2.c b/src/mux_h2.c
index d145b8a970c8bd6969e588f2a341f6186f0a6b93..9c87218c6994f82f0dfe5e5f8e448b85dde95feb 100644 (file)
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -551,6 +551,18 @@ static void h2_trace(enum trace_level level, uint64_t mask, const struct trace_s
        }
 }
 
+
+/* Detect a pending read0 for a H2 connection. It happens if a read0 is pending
+ * on the connection AND if there is no more data in the demux buffer. The
+ * function returns 1 to report a read0 or 0 otherwise.
+ */
+static int h2c_read0_pending(struct h2c *h2c)
+{
+       if (conn_xprt_read0_pending(h2c->conn) && !b_data(&h2c->dbuf))
+               return 1;
+       return 0;
+}
+
 /* returns true if the connection is allowed to expire, false otherwise. A
  * connection may expire when:
  *   - it has no stream
@@ -1901,7 +1913,7 @@ static void h2s_wake_one_stream(struct h2s *h2s)
                return;
        }
 
-       if (conn_xprt_read0_pending(h2s->h2c->conn)) {
+       if (h2c_read0_pending(h2s->h2c)) {
                if (h2s->st == H2_SS_OPEN)
                        h2s->st = H2_SS_HREM;
                else if (h2s->st == H2_SS_HLOC)
@@ -3050,7 +3062,7 @@ static void h2_process_demux(struct h2c *h2c)
 
                if (tmp_h2s != h2s && h2s && h2s->cs &&
                    (b_data(&h2s->rxbuf) ||
-                    conn_xprt_read0_pending(h2c->conn) ||
+                    h2c_read0_pending(h2c) ||
                     h2s->st == H2_SS_CLOSED ||
                     (h2s->flags & H2_SF_ES_RCVD) ||
                     (h2s->cs->flags & (CS_FL_ERROR|CS_FL_ERR_PENDING|CS_FL_EOS)))) {
@@ -3213,7 +3225,7 @@ static void h2_process_demux(struct h2c *h2c)
        /* we can go here on missing data, blocked response or error */
        if (h2s && h2s->cs &&
            (b_data(&h2s->rxbuf) ||
-            conn_xprt_read0_pending(h2c->conn) ||
+            h2c_read0_pending(h2c) ||
             h2s->st == H2_SS_CLOSED ||
             (h2s->flags & H2_SF_ES_RCVD) ||
             (h2s->cs->flags & (CS_FL_ERROR|CS_FL_ERR_PENDING|CS_FL_EOS)))) {
@@ -3630,7 +3642,7 @@ static int h2_process(struct h2c *h2c)
                }
        }
 
-       if (conn->flags & CO_FL_ERROR || conn_xprt_read0_pending(conn) ||
+       if (conn->flags & CO_FL_ERROR || h2c_read0_pending(h2c) ||
            h2c->st0 == H2_CS_ERROR2 || h2c->flags & H2_CF_GOAWAY_FAILED ||
            (eb_is_empty(&h2c->streams_by_id) && h2c->last_sid >= 0 &&
             h2c->max_id >= h2c->last_sid)) {
@@ -5796,7 +5808,7 @@ static size_t h2_rcv_buf(struct conn_stream *cs, struct buffer *buf, size_t coun
                cs->flags &= ~(CS_FL_RCV_MORE | CS_FL_WANT_ROOM);
                if (h2s->flags & H2_SF_ES_RCVD)
                        cs->flags |= CS_FL_EOI;
-               if (conn_xprt_read0_pending(h2c->conn) || h2s->st == H2_SS_CLOSED)
+               if (h2c_read0_pending(h2c) || h2s->st == H2_SS_CLOSED)
                        cs->flags |= CS_FL_EOS;
                if (cs->flags & CS_FL_ERR_PENDING)
                        cs->flags |= CS_FL_ERROR;