BUG/MEDIUM: mux-h2: Set EOI on the conn_stream during h2_rcv_buf()

Just like CS_FL_REOS previously, the CS_FL_EOI flag is abused as a proxy
for H2_SF_ES_RCVD. The problem is that this flag is consumed by the
application layer and is set immediately when an end of stream was met,
which is too early since the application must retrieve the rxbuf's
contents first. The effect is that some transfers are truncated (mostly
the first one of a connection in most tests).

The problem of mixing CS flags and H2S flags in the H2 mux is not new
(and is currently being addressed) but this specific one was emphasized
in commit 63768a63d ("MEDIUM: mux-h2: Don't mix the end of the message
with the end of stream") which was backported to 1.9. Note that other
flags, particularly CS_FL_REOS still need to be asynchronously reported,
though their impact seems more limited for now.

This patch makes sure that all internal uses of CS_FL_EOI are replaced
with a test on H2_SF_ES_RCVD (as there is a 1-to-1 equivalence) and that
CS_FL_EOI is only reported once the rxbuf is empty.

This should ideally be backported to 1.9 unless it causes too much
trouble due to the recent changes in this area, as 1.9 *seems* not
to be directly affected by this bug.

diff --git a/src/mux_h2.c b/src/mux_h2.c
index dea7be26de381e39ef1c793df04ffeef6ebba847..73d32a926b7cd347562da0fd58759162639971e3 100644 (file)
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -2008,8 +2008,6 @@ static struct h2s *h2c_frt_handle_headers(struct h2c *h2c, struct h2s *h2s)
                h2s->flags |= H2_SF_ES_RCVD;
 
        if (h2s->flags & H2_SF_ES_RCVD) {
-               if (h2s->cs)
-                       h2s->cs->flags |= CS_FL_EOI;
                if (h2s->st == H2_SS_OPEN)
                        h2s->st = H2_SS_HREM;
                else
@@ -2078,18 +2076,17 @@ static struct h2s *h2c_bck_handle_headers(struct h2c *h2c, struct h2s *h2s)
                return NULL;
        }
 
-       if (h2c->dff & H2_F_HEADERS_END_STREAM) {
+       if (h2c->dff & H2_F_HEADERS_END_STREAM)
                h2s->flags |= H2_SF_ES_RCVD;
-               if (h2s->cs)
-                       h2s->cs->flags |= CS_FL_EOI;
-       }
 
        if (h2s->cs && h2s->cs->flags & CS_FL_ERROR && h2s->st < H2_SS_ERROR)
                h2s->st = H2_SS_ERROR;
-       else if (h2s->cs && (h2s->cs->flags & CS_FL_EOI) && h2s->st == H2_SS_OPEN)
-               h2s->st = H2_SS_HREM;
-       else if ((!h2s->cs || h2s->cs->flags & CS_FL_EOI) && h2s->st == H2_SS_HLOC)
-               h2s_close(h2s);
+       else if (h2s->flags & H2_SF_ES_RCVD) {
+               if (h2s->st == H2_SS_OPEN)
+                       h2s->st = H2_SS_HREM;
+               else if (h2s->st == H2_SS_HLOC)
+                       h2s_close(h2s);
+       }
 
        return h2s;
 }
@@ -2152,15 +2149,12 @@ static int h2c_frt_handle_data(struct h2c *h2c, struct h2s *h2s)
 
        /* last frame */
        if (h2c->dff & H2_F_DATA_END_STREAM) {
-               if (h2s->cs)
-                       h2s->cs->flags |= CS_FL_EOI;
+               h2s->flags |= H2_SF_ES_RCVD;
                if (h2s->st == H2_SS_OPEN)
                        h2s->st = H2_SS_HREM;
                else
                        h2s_close(h2s);
 
-               h2s->flags |= H2_SF_ES_RCVD;
-
                if (h2s->flags & H2_SF_DATA_CLEN && h2s->body_len) {
                        /* RFC7540#8.1.2 */
                        error = H2_ERR_PROTOCOL_ERROR;
@@ -2321,7 +2315,8 @@ static void h2_process_demux(struct h2c *h2c)
                if (tmp_h2s != h2s && h2s && h2s->cs &&
                    (b_data(&h2s->rxbuf) ||
                     (H2_SS_MASK(h2s->st) & H2_SS_EOS_BITS) ||
-                    (h2s->cs->flags & (CS_FL_ERROR|CS_FL_ERR_PENDING|CS_FL_EOS|CS_FL_EOI)))) {
+                    (h2s->flags & H2_SF_ES_RCVD) ||
+                    (h2s->cs->flags & (CS_FL_ERROR|CS_FL_ERR_PENDING|CS_FL_EOS)))) {
                        /* we may have to signal the upper layers */
                        h2s->cs->flags |= CS_FL_RCV_MORE;
                        h2s_notify_recv(h2s);
@@ -2559,7 +2554,8 @@ static void h2_process_demux(struct h2c *h2c)
        if (h2s && h2s->cs &&
            (b_data(&h2s->rxbuf) ||
             (H2_SS_MASK(h2s->st) & H2_SS_EOS_BITS) ||
-            (h2s->cs->flags & (CS_FL_ERROR|CS_FL_ERR_PENDING|CS_FL_EOS|CS_FL_EOI)))) {
+            (h2s->flags & H2_SF_ES_RCVD) ||
+            (h2s->cs->flags & (CS_FL_ERROR|CS_FL_ERR_PENDING|CS_FL_EOS)))) {
                /* we may have to signal the upper layers */
                h2s->cs->flags |= CS_FL_RCV_MORE;
                h2s_notify_recv(h2s);
@@ -5315,6 +5311,8 @@ 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);
        else {
                cs->flags &= ~(CS_FL_RCV_MORE | CS_FL_WANT_ROOM);
+               if (h2s->flags & H2_SF_ES_RCVD)
+                       cs->flags |= CS_FL_EOI;
                if (H2_SS_MASK(h2s->st) & H2_SS_EOS_BITS)
                        cs->flags |= CS_FL_EOS;
                if (cs->flags & CS_FL_ERR_PENDING)