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.
(cherry picked from commit aade4edc1a8d148447953e279c6a83bf3dea6661)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 36840d8ca8e298b4da4bd24d0f6128b46e0757e3)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit e77e79d29220759de8da22ed518e12cecefda520)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
diff --git a/src/mux_h2.c b/src/mux_h2.c
index abe78d3..5cdd43a 100644
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -303,6 +303,18 @@
static struct h2s *h2c_bck_stream_new(struct h2c *h2c, struct conn_stream *cs, struct session *sess);
static void h2s_alert(struct h2s *h2s);
+
+/* 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
@@ -1529,7 +1541,7 @@
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)
@@ -2446,7 +2458,7 @@
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)))) {
@@ -2705,7 +2717,7 @@
/* 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)))) {
@@ -3031,7 +3043,7 @@
}
}
- 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)) {
@@ -5559,7 +5571,7 @@
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;