BUG/MAJOR: mux-h1: Properly handle TCP to H1 upgrades

It is the second part and the most important of the fix.

Since the mux-h1 refactoring, and more specifically since the commit
c4bfa59f1 ("MAJOR: mux-h1: Create the client stream as later as possible"),
the upgrade from a TCP client connection to H1 is broken. Indeed, now the H1
mux is responsible to create the frontend conn-stream once the request
headers are fully received. But, to properly support TCP to H1 upgrades, we
must inherit from the existing conn-stream. To do so, if the conn-stream
already exists when the client H1 connection is created, we create a H1
stream in ST_ATTACHED state, but not ST_READY, and the conn-stream is
attached to it. Because the ST_READY state is not set, no data are xferred
to the data layer when h1_rcv_buf() is called and shutdowns are inhibited
except on client aborts. This way, the request is parsed the same way than
for a classical H1 connection. Once the request headers are fully received
and parsed, the data stream is upgraded and the ST_READY state is set.

A tricky case appears when an H2 upgrade is performed because the H2 preface
is matched. In this case, the conn-stream must be detached and destroyed
before switching to the H2 mux and releasing the current H1 mux. We must
also take care to detach and destroy the conn-stream when a timeout
occurres.

This patch relies on the following series of patches :

* BUG/MEDIUM: stream: Don't immediatly ack the TCP to H1 upgrades
* MEDIUM: http-ana: Do nothing in wait-for-request analyzer if not htx
* MINOR: stream: Add a function to validate TCP to H1 upgrades
* MEDIUM: mux-h1: Add ST_READY state for the H1 connections
* MINOR: mux-h1: Wake up instead of subscribe for reads after H1C creation
* MINOR: mux-h1: Try to wake up data layer first before calling its wake callback
* MINOR: stream-int: Take care of EOS in the SI wake callback function
* BUG/MINOR: stream: Don't update counters when TCP to H2 upgrades are performed

This fix is specific for 2.4. No backport needed.

diff --git a/src/mux_h1.c b/src/mux_h1.c
index af9d21e4d55648a8751d450180a3acbe992244b8..19f8240f4ff1af352466f85787db6a11796be74b 100644 (file)
--- a/src/mux_h1.c
+++ b/src/mux_h1.c
@@ -611,6 +611,28 @@ static struct conn_stream *h1s_new_cs(struct h1s *h1s, struct buffer *input)
        return NULL;
 }
 
+static struct conn_stream *h1s_upgrade_cs(struct h1s *h1s, struct buffer *input)
+{
+       TRACE_ENTER(H1_EV_STRM_NEW, h1s->h1c->conn, h1s);
+
+       if (stream_upgrade_from_cs(h1s->cs, input) < 0) {
+               TRACE_DEVEL("leaving on stream upgrade failure", H1_EV_STRM_NEW|H1_EV_STRM_END|H1_EV_STRM_ERR, h1s->h1c->conn, h1s);
+               goto err;
+       }
+
+       if (global.tune.options & GTUNE_USE_SPLICE) {
+               TRACE_STATE("notify the mux can use splicing", H1_EV_STRM_NEW, h1s->h1c->conn, h1s);
+               h1s->cs->flags |= CS_FL_MAY_SPLICE;
+       }
+
+       h1s->h1c->flags |= H1C_F_ST_READY;
+       TRACE_LEAVE(H1_EV_STRM_NEW, h1s->h1c->conn, h1s);
+       return h1s->cs;
+
+  err:
+       return NULL;
+}
+
 static struct h1s *h1s_new(struct h1c *h1c)
 {
        struct h1s *h1s;
@@ -807,6 +829,21 @@ static int h1_init(struct connection *conn, struct proxy *proxy, struct session
                if (!h1c_bck_stream_new(h1c, conn_ctx, sess))
                        goto fail;
        }
+       else if (conn_ctx) {
+               /* Upgraded frontend connection (from TCP) */
+               struct conn_stream *cs = conn_ctx;
+
+               if (!h1c_frt_stream_new(h1c))
+                       goto fail;
+
+               h1c->h1s->cs = cs;
+               cs->ctx = h1c->h1s;
+
+               /* Attach the CS but Not ready yet */
+               h1c->flags = (h1c->flags & ~H1C_F_ST_EMBRYONIC) | H1C_F_ST_ATTACHED;
+               TRACE_DEVEL("Inherit the CS from TCP connection to perform an upgrade",
+                           H1_EV_H1C_NEW|H1_EV_STRM_NEW, h1c->conn, h1c->h1s);
+       }
 
        if (t) {
                h1_set_idle_expiration(h1c);
@@ -1573,9 +1610,21 @@ static size_t h1_process_input(struct h1c *h1c, struct buffer *buf, size_t count
                        goto end;
                }
 
-               if (!h1s_new_cs(h1s, buf)) {
-                       h1c->flags |= H1C_F_ST_ERROR;
-                       goto err;
+               if (!(h1c->flags & H1C_F_ST_ATTACHED)) {
+                       TRACE_DEVEL("request headers fully parsed, create and attach the CS", H1_EV_RX_DATA, h1c->conn, h1s);
+                       BUG_ON(h1s->cs);
+                       if (!h1s_new_cs(h1s, buf)) {
+                               h1c->flags |= H1C_F_ST_ERROR;
+                               goto err;
+                       }
+               }
+               else {
+                       TRACE_DEVEL("request headers fully parsed, upgrade the inherited CS", H1_EV_RX_DATA, h1c->conn, h1s);
+                       BUG_ON(h1s->cs == NULL);
+                       if (!h1s_upgrade_cs(h1s, buf)) {
+                               h1c->flags |= H1C_F_ST_ERROR;
+                               goto err;
+                       }
                }
        }
 
@@ -2437,6 +2486,13 @@ static int h1_process(struct h1c * h1c)
                        /* Try to match H2 preface before parsing the request headers. */
                        if (b_isteq(&h1c->ibuf, 0, b_data(&h1c->ibuf), ist(H2_CONN_PREFACE)) > 0) {
                                h1c->flags |= H1C_F_UPG_H2C;
+                               if (h1c->flags & H1C_F_ST_ATTACHED) {
+                                       /* Force the REOS here to be sure to release the CS.
+                                          Here ATTACHED implies !READY, and h1s defined
+                                       */
+                                       BUG_ON(!h1s ||  (h1c->flags & H1C_F_ST_READY));
+                                       h1s->flags |= H1S_F_REOS;
+                               }
                                TRACE_STATE("release h1c to perform H2 upgrade ", H1_EV_RX_DATA|H1_EV_H1C_WAKE);
                                goto release;
                        }
@@ -2538,8 +2594,22 @@ static int h1_process(struct h1c * h1c)
        return 0;
 
   release:
-       h1_release(h1c);
-       TRACE_DEVEL("leaving after releasing the connection", H1_EV_H1C_WAKE);
+       if (h1c->flags & H1C_F_ST_ATTACHED) {
+               /* Don't release the H1 connetion right now, we must destroy the
+                * attached CS first. Here, the H1C must not be READY */
+               BUG_ON(!h1s || h1c->flags & H1C_F_ST_READY);
+
+               if (conn_xprt_read0_pending(conn) || (h1s->flags & H1S_F_REOS))
+                       h1s->cs->flags |= CS_FL_EOS;
+               if ((h1c->flags & H1C_F_ST_ERROR) || (conn->flags & CO_FL_ERROR))
+                       h1s->cs->flags |= CS_FL_ERROR;
+               h1_alert(h1s);
+               TRACE_DEVEL("waiting to release the CS before releasing the connection", H1_EV_H1C_WAKE);
+       }
+       else {
+               h1_release(h1c);
+               TRACE_DEVEL("leaving after releasing the connection", H1_EV_H1C_WAKE);
+       }
        return -1;
 }
 
@@ -2672,6 +2742,17 @@ static struct task *h1_timeout_task(struct task *t, void *context, unsigned shor
                        }
                }
 
+               if (h1c->flags & H1C_F_ST_ATTACHED) {
+                       /* Don't release the H1 connetion right now, we must destroy the
+                        * attached CS first. Here, the H1C must not be READY */
+                       h1c->h1s->cs->flags |= (CS_FL_EOS|CS_FL_ERROR);
+                       h1_alert(h1c->h1s);
+                       h1_refresh_timeout(h1c);
+                       HA_SPIN_UNLOCK(OTHER_LOCK, &idle_conns[tid].takeover_lock);
+                       TRACE_DEVEL("waiting to release the CS before releasing the connection", H1_EV_H1C_WAKE);
+                       return t;
+               }
+
                /* We're about to destroy the connection, so make sure nobody attempts
                 * to steal it from us.
                 */