MEDIUM: h3: enforce GOAWAY by resetting higher unhandled stream
When a GOAWAY has been emitted, an ID is announced to represent handled
streams. H3 RFC suggests that higher streams should be resetted with the
error code H3_REQUEST_CANCELLED. This allows the peer to replay requests
on another connection.
For the moment, the impact of this change is limitted as GOAWAY is only
used on connection shutdown just before the MUX is freed. However, for
soft-stop support, a GOAWAY can be emitted in anticipation while keeping
the MUX to finish the active streams. In this case, new streams opened
by the client are resetted.
As a consequence of this change, app_ops.attach() operation has been
delayed at the very end of qcs_new(). This ensure that all qcs members
are initialized to support RESET_STREAM sending.
This should be backported up to 2.7.
diff --git a/src/h3.c b/src/h3.c
index 1572ac1..c912c0a 100644
--- a/src/h3.c
+++ b/src/h3.c
@@ -114,6 +114,7 @@
#define H3_CF_UNI_CTRL_SET 0x00000004 /* Remote H3 Control stream opened */
#define H3_CF_UNI_QPACK_DEC_SET 0x00000008 /* Remote QPACK decoder stream opened */
#define H3_CF_UNI_QPACK_ENC_SET 0x00000010 /* Remote QPACK encoder stream opened */
+#define H3_CF_GOAWAY_SENT 0x00000020 /* GOAWAY sent on local control stream */
/* Default settings */
static uint64_t h3_settings_qpack_max_table_capacity = 0;
@@ -1661,13 +1662,38 @@
static int h3_attach(struct qcs *qcs, void *conn_ctx)
{
- struct h3s *h3s;
+ struct h3c *h3c = conn_ctx;
+ struct h3s *h3s = NULL;
TRACE_ENTER(H3_EV_H3S_NEW, qcs->qcc->conn, qcs);
+ /* RFC 9114 5.2. Connection Shutdown
+ *
+ * Upon sending
+ * a GOAWAY frame, the endpoint SHOULD explicitly cancel (see
+ * Sections 4.1.1 and 7.2.3) any requests or pushes that have
+ * identifiers greater than or equal to the one indicated, in
+ * order to clean up transport state for the affected streams.
+ * The endpoint SHOULD continue to do so as more requests or
+ * pushes arrive.
+ */
+ if (h3c->flags & H3_CF_GOAWAY_SENT && qcs->id >= h3c->id_goaway &&
+ quic_stream_is_bidi(qcs->id)) {
+ /* Reject request and do not allocate a h3s context.
+ * TODO support push uni-stream rejection.
+ */
+ TRACE_STATE("reject stream higher than goaway", H3_EV_H3S_NEW, qcs->qcc->conn, qcs);
+ qcc_abort_stream_read(qcs);
+ qcc_reset_stream(qcs, H3_REQUEST_REJECTED);
+ goto done;
+ }
+
h3s = pool_alloc(pool_head_h3s);
- if (!h3s)
- return 1;
+ if (!h3s) {
+ TRACE_ERROR("h3s allocation failure", H3_EV_H3S_NEW, qcs->qcc->conn, qcs);
+ qcc_emit_cc_app(qcs->qcc, H3_INTERNAL_ERROR, 1);
+ goto err;
+ }
qcs->ctx = h3s;
h3s->h3c = conn_ctx;
@@ -1689,8 +1715,13 @@
h3s->type = H3S_T_UNKNOWN;
}
+ done:
TRACE_LEAVE(H3_EV_H3S_NEW, qcs->qcc->conn, qcs);
return 0;
+
+ err:
+ TRACE_DEVEL("leaving in error", H3_EV_H3S_NEW, qcs->qcc->conn, qcs);
+ return 1;
}
static void h3_detach(struct qcs *qcs)
@@ -1758,10 +1789,15 @@
b_force_xfer(res, &pos, b_data(&pos));
qcc_send_stream(qcs, 1);
+ h3c->flags |= H3_CF_GOAWAY_SENT;
TRACE_LEAVE(H3_EV_H3C_END, h3c->qcc->conn);
return 0;
err:
+ /* Consider GOAWAY as sent even if not really the case. This will
+ * block future stream opening using H3_REQUEST_REJECTED reset.
+ */
+ h3c->flags |= H3_CF_GOAWAY_SENT;
TRACE_DEVEL("leaving in error", H3_EV_H3C_END, h3c->qcc->conn);
return 1;
}
diff --git a/src/mux_quic.c b/src/mux_quic.c
index d68abbe..8577deb 100644
--- a/src/mux_quic.c
+++ b/src/mux_quic.c
@@ -135,13 +135,6 @@
}
}
- if (qcc->app_ops->attach) {
- if (qcc->app_ops->attach(qcs, qcc->ctx)) {
- TRACE_ERROR("app proto failure", QMUX_EV_QCS_NEW, qcc->conn, qcs);
- goto err;
- }
- }
-
/* If stream is local, use peer remote-limit, or else the opposite. */
if (quic_stream_is_bidi(id)) {
qcs->tx.msd = quic_stream_is_local(qcc, id) ? qcc->rfctl.msd_bidi_r :
@@ -174,6 +167,11 @@
qcs->err = 0;
+ if (qcc->app_ops->attach && qcc->app_ops->attach(qcs, qcc->ctx)) {
+ TRACE_ERROR("app proto failure", QMUX_EV_QCS_NEW, qcc->conn, qcs);
+ goto err;
+ }
+
out:
TRACE_LEAVE(QMUX_EV_QCS_NEW, qcc->conn, qcs);
return qcs;