BUG/MINOR: mux-quic: ignore STOP_SENDING for locally closed stream

It is possible to receive a STOP_SENDING frame for a locally closed
stream. This was not properly managed as this would result in a BUG_ON()
crash from qcs_idle_open() call under qcc_recv_stop_sending().

Now, STOP_SENDING frames are ignored when received on streams already
locally closed. This has two consequences depending on the reason of
closure :

* if a RESET_STREAM was already emitted and closed the stream, this
  patch prevents to emit a new RESET_STREAM. This behavior is thus
  better.

* if stream was closed due to all data transmitted, no RESET_STREAM will
  be built. This is contrary to the RFC 9000 which advice to transmit
  it, even on "Data Sent" state. However, this is not mandatory so the
  new behavior is acceptable, even if it could be improved.

This crash has been detected on haproxy.org. This can be artifically
reproduced by adding the following snippet at the end of qc_send_mux()
when doing a request with a small payload response :
  qcc_recv_stop_sending(qc->qcc, 0, 0);

This must be backported up to 2.6.

diff --git a/src/mux_quic.c b/src/mux_quic.c
index d27a36c60e9f9014c398a25f8f3fadeb833f800e..0482e92bc7bf7dfa8d2c5f255a9fa5ec14654ee8 100644 (file)
--- a/src/mux_quic.c
+++ b/src/mux_quic.c
@@ -1067,6 +1067,27 @@ int qcc_recv_stop_sending(struct qcc *qcc, uint64_t id, uint64_t err)
                goto out;
 
        TRACE_PROTO("receiving STOP_SENDING", QMUX_EV_QCC_RECV|QMUX_EV_QCS_RECV, qcc->conn, qcs);
+
+       /* RFC 9000 3.5. Solicited State Transitions
+        *
+        * An endpoint is expected to send another STOP_SENDING frame if a
+        * packet containing a previous STOP_SENDING is lost.  However, once
+        * either all stream data or a RESET_STREAM frame has been received for
+        * the stream -- that is, the stream is in any state other than "Recv"
+        * or "Size Known" -- sending a STOP_SENDING frame is unnecessary.
+        */
+
+       /* TODO thanks to previous RFC clause, STOP_SENDING is ignored if current stream
+        * has already been closed locally. This is useful to not emit multiple
+        * RESET_STREAM for a single stream. This is functional if stream is
+        * locally closed due to all data transmitted, but in this case the RFC
+        * advices to use an explicit RESET_STREAM.
+        */
+       if (qcs_is_close_local(qcs)) {
+               TRACE_STATE("ignoring STOP_SENDING", QMUX_EV_QCC_RECV|QMUX_EV_QCS_RECV, qcc->conn, qcs);
+               goto out;
+       }
+
        qcs_idle_open(qcs);
 
        /* RFC 9000 3.5. Solicited State Transitions