BUG/MINOR: mux-h2: do not send GOAWAY if SETTINGS were not sent

It was reported in issue #13 that a GOAWAY frame was sent on timeout even
if no SETTINGS frame was sent. The approach imagined by then was to track
the fact that a SETTINGS frame was already sent to avoid this, but that's
already what is done through the state, though it doesn't stand due to the
fact that we switch the frame to the error state. Thus instead what we're
doing here is to instead set the GOAWAY_FAILED flag in h2c_error() before
switching to the ERROR state when the state indicates we've not yet sent
settings, and refrain from sending anything from the h2c_send_goaway_error()
function for such states.

This could be backported to all versions where it applies well.

diff --git a/src/mux_h2.c b/src/mux_h2.c
index a16d972c99d92bf1c9f9fd557b9bf204a1762b84..9949bbc5cdd9dbcfc6a6ac1e580bddee56305b1b 100644 (file)
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -1249,11 +1249,17 @@ static inline __maybe_unused int h2c_mux_busy(const struct h2c *h2c, const struc
        return 1;
 }
 
-/* marks an error on the connection */
+/* marks an error on the connection. Before settings are sent, we must not send
+ * a GOAWAY frame, and the error state will prevent h2c_send_goaway_error()
+ * from verifying this so we set H2_CF_GOAWAY_FAILED to make sure it will not
+ * even try.
+ */
 static inline __maybe_unused void h2c_error(struct h2c *h2c, enum h2_err err)
 {
        TRACE_POINT(H2_EV_H2C_ERR, h2c->conn, 0, 0, (void *)(long)(err));
        h2c->errcode = err;
+       if (h2c->st0 < H2_CS_SETTINGS1)
+               h2c->flags |= H2_CF_GOAWAY_FAILED;
        h2c->st0 = H2_CS_ERROR;
 }
 
@@ -1879,7 +1885,8 @@ static int h2c_bck_send_preface(struct h2c *h2c)
  * the message, it subscribes the requester (either <h2s> or <h2c>) to future
  * notifications. It sets H2_CF_GOAWAY_SENT on success, and H2_CF_GOAWAY_FAILED
  * on unrecoverable failure. It will not attempt to send one again in this last
- * case so that it is safe to use h2c_error() to report such errors.
+ * case, nor will it send one if settings were not sent (e.g. still waiting for
+ * a preface) so that it is safe to use h2c_error() to report such errors.
  */
 static int h2c_send_goaway_error(struct h2c *h2c, struct h2s *h2s)
 {
@@ -1889,7 +1896,7 @@ static int h2c_send_goaway_error(struct h2c *h2c, struct h2s *h2s)
 
        TRACE_ENTER(H2_EV_TX_FRAME|H2_EV_TX_GOAWAY, h2c->conn);
 
-       if (h2c->flags & H2_CF_GOAWAY_FAILED) {
+       if ((h2c->flags & H2_CF_GOAWAY_FAILED) || h2c->st0 < H2_CS_SETTINGS1) {
                ret = 1; // claim that it worked
                goto out;
        }