BUG/MEDIUM: mux-h2: do not close the connection on aborted streams

We used to rely on a hint that a shutw() or shutr() without data is an
indication that the upper layer had performed a tcp-request content reject
and really wanted to kill the connection, but sadly there is another
situation where this happens, which is failed keep-alive request to a
server. In this case the upper layer stream silently closes to let the
client retry. In our case this had the side effect of killing all the
connection.

Instead of relying on such hints, let's address the problem differently
and rely on information passed by the upper layers about the intent to
kill the connection. During shutr/shutw, this is detected because the
flag CS_FL_KILL_CONN is set on the connstream. Then only in this case
we send a GOAWAY(ENHANCE_YOUR_CALM), otherwise we only send the reset.

This makes sure that failed backend requests only fail frontend requests
and not the whole connections anymore.

This fix relies on the two previous patches adding SI_FL_KILL_CONN and
CS_FL_KILL_CONN as well as the fix for the connection close, and it must
be backported to 1.9 and 1.8, though the code in 1.8 could slightly differ
(cs is always valid) :

  BUG/MEDIUM: mux-h2: wait for the mux buffer to be empty before closing the connection
  MINOR: stream-int: add a new flag to mention that we want the connection to be killed
  MINOR: connstream: have a new flag CS_FL_KILL_CONN to kill a connection

diff --git a/src/mux_h2.c b/src/mux_h2.c
index 503c40da1bdc9b205c40bde38087970ab54f5d10..f20d0b094097e81706d51eea42fa2a4f8c7a4a70 100644 (file)
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -3028,20 +3028,21 @@ static void h2_do_shutr(struct h2s *h2s)
        if (h2s->st == H2_SS_HLOC || h2s->st == H2_SS_ERROR || h2s->st == H2_SS_CLOSED)
                return;
 
-       /* if no outgoing data was seen on this stream, it means it was
-        * closed with a "tcp-request content" rule that is normally
-        * used to kill the connection ASAP (eg: limit abuse). In this
-        * case we send a goaway to close the connection.
+       /* a connstream may require us to immediately kill the whole connection
+        * for example because of a "tcp-request content reject" rule that is
+        * normally used to limit abuse. In this case we schedule a goaway to
+        * close the connection.
         */
+       if ((h2s->cs && h2s->cs->flags & CS_FL_KILL_CONN) &&
+           !(h2c->flags & (H2_CF_GOAWAY_SENT|H2_CF_GOAWAY_FAILED))) {
+               h2c_error(h2c, H2_ERR_ENHANCE_YOUR_CALM);
+               h2s_error(h2s, H2_ERR_ENHANCE_YOUR_CALM);
+       }
+
        if (!(h2s->flags & H2_SF_RST_SENT) &&
            h2s_send_rst_stream(h2c, h2s) <= 0)
                goto add_to_list;
 
-       if (!(h2s->flags & H2_SF_OUTGOING_DATA) &&
-           !(h2s->h2c->flags & (H2_CF_GOAWAY_SENT|H2_CF_GOAWAY_FAILED)) &&
-           h2c_send_goaway_error(h2c, h2s) <= 0)
-               return;
-
        if (!(h2c->wait_event.events & SUB_RETRY_SEND))
                tasklet_wakeup(h2c->wait_event.task);
        h2s_close(h2s);
@@ -3060,7 +3061,6 @@ add_to_list:
        }
        /* Let the handler know we want shutr */
        sw->handle = (void *)((long)sw->handle | 1);
-
 }
 
 static void h2_do_shutw(struct h2s *h2s)
@@ -3083,20 +3083,21 @@ static void h2_do_shutw(struct h2s *h2s)
                else
                        h2s->st = H2_SS_HLOC;
        } else {
-               /* if no outgoing data was seen on this stream, it means it was
-                * closed with a "tcp-request content" rule that is normally
-                * used to kill the connection ASAP (eg: limit abuse). In this
-                * case we send a goaway to close the connection.
+               /* a connstream may require us to immediately kill the whole connection
+                * for example because of a "tcp-request content reject" rule that is
+                * normally used to limit abuse. In this case we schedule a goaway to
+                * close the connection.
                 */
+               if ((h2s->cs && h2s->cs->flags & CS_FL_KILL_CONN) &&
+                   !(h2c->flags & (H2_CF_GOAWAY_SENT|H2_CF_GOAWAY_FAILED))) {
+                       h2c_error(h2c, H2_ERR_ENHANCE_YOUR_CALM);
+                       h2s_error(h2s, H2_ERR_ENHANCE_YOUR_CALM);
+               }
+
                if (!(h2s->flags & H2_SF_RST_SENT) &&
                    h2s_send_rst_stream(h2c, h2s) <= 0)
                        goto add_to_list;
 
-               if (!(h2s->flags & H2_SF_OUTGOING_DATA) &&
-                   !(h2s->h2c->flags & (H2_CF_GOAWAY_SENT|H2_CF_GOAWAY_FAILED)) &&
-                   h2c_send_goaway_error(h2c, h2s) <= 0)
-                       goto add_to_list;
-
                h2s_close(h2s);
        }
 
@@ -3117,7 +3118,6 @@ static void h2_do_shutw(struct h2s *h2s)
        }
        /* let the handler know we want to shutw */
        sw->handle = (void *)((long)(sw->handle) | 2);
-
 }
 
 static struct task *h2_deferred_shut(struct task *t, void *ctx, unsigned short state)