BUG/MINOR: h3: properly handle interim response on BE side

On backend side, H3 layer is responsible to decode a HTTP/3 response
into an HTX message. Multiple responses may be received on a single
stream with interim status codes prior to the final one.

h3_resp_headers_to_htx() is the function used solely on backend side
responsible for H3 response to HTX transcoding. This patch extends it to
be able to properly support interim responses. When such a response is
received, the new flag H3_SF_RECV_INTERIM is set. This is converted to
QMUX qcs flag QC_SF_EOI_SUSPENDED.

The objective of this latter flag is to prevent stream EOI to be
reported during stream rcv_buf callback, even if HTX message contains
EOM and is empty. QC_SF_EOI_SUSPENDED will be cleared when the final
response is finally converted, which unblock stream EOI notification for
next rcv_buf invocations. Note however that HTX EOM is untouched : it is
always set for both interim and final response reception.

As a minor adjustment, HTX_SL_F_BODYLESS is always set for interim
responses.

Contrary to frontend interim response handling, a flag is necessary on
QMUX layer. This is because H3 to HTX transcoding and rcv_buf callback
are two distinct operations, called under different context (MUX vs
stream tasklet).

Also note that H3 layer has two distinct flags for interim response
handling, one only used as a server (FE side) and the other as a client
(BE side). It was preferred to used two distinct flags which is
considered less error-prone, contrary to a single unified flag which
would require to always set the proxy side to ensure it is relevant or
not.

No need to backport.

diff --git a/include/haproxy/mux_quic-t.h b/include/haproxy/mux_quic-t.h
index 6b7f1218a39a2a66512d7ed3fadbfaaf53d19a0c..906b383f2e5090d4e9ef9286c4706db3c78d6073 100644 (file)
--- a/include/haproxy/mux_quic-t.h
+++ b/include/haproxy/mux_quic-t.h
@@ -275,6 +275,7 @@ static forceinline char *qcc_show_flags(char *buf, size_t len, const char *delim
 #define QC_SF_TO_STOP_SENDING   0x00000200  /* a STOP_SENDING must be sent */
 #define QC_SF_UNKNOWN_PL_LENGTH 0x00000400  /* HTX EOM may be missing from the stream layer */
 #define QC_SF_RECV_RESET        0x00000800  /* a RESET_STREAM was received */
+#define QC_SF_EOI_SUSPENDED     0x00001000  /* EOI must not be reported even if HTX EOM is present - useful when transferring HTTP interim responses */
 
 /* This function is used to report flags in debugging tools. Please reflect
  * below any single-bit flag addition above in the same order via the

diff --git a/src/h3.c b/src/h3.c
index f3267db01b85ebe29d68cac08b80ad332d68b1b6..c93752334c455d7efac3735c1dec3c0d54306c54 100644 (file)
--- a/src/h3.c
+++ b/src/h3.c
@@ -154,6 +154,7 @@ DECLARE_STATIC_POOL(pool_head_h3c, "h3c", sizeof(struct h3c));
 #define H3_SF_UNI_NO_H3    0x00000002  /* unidirectional stream does not carry H3 frames */
 #define H3_SF_HAVE_CLEN    0x00000004  /* content-length header is present; relevant either for request or response depending on the side of the connection */
 #define H3_SF_SENT_INTERIM 0x00000008  /* last response sent is 1xx interim. Used on FE side only. */
+#define H3_SF_RECV_INTERIM 0x00000010  /* last response sent is 1xx interim. Used on BE side only. */
 
 struct h3s {
        struct h3c *h3c;
@@ -1092,6 +1093,9 @@ static ssize_t h3_req_headers_to_htx(struct qcs *qcs, const struct buffer *buf,
  * transcoded into HTX buffer of <qcs> stream. <fin> must be set if this is the
  * last data to transfer for this stream.
  *
+ * If the response handled was an interim one (1xx code), H3_SF_RECV_INTERIM
+ * flag will be set on h3 stream instance.
+ *
  * Returns the number of consumed bytes or a negative error code.
  */
 static ssize_t h3_resp_headers_to_htx(struct qcs *qcs, const struct buffer *buf,
@@ -1241,10 +1245,9 @@ static ssize_t h3_resp_headers_to_htx(struct qcs *qcs, const struct buffer *buf,
                goto out;
        }
 
-       if (fin)
-               sl->flags |= HTX_SL_F_BODYLESS;
-
        sl->info.res.status = h * 100 + t * 10 + u;
+       if (fin || (sl->info.res.status >= 100 && sl->info.res.status < 200))
+               sl->flags |= HTX_SL_F_BODYLESS;
 
        /* now treat standard headers */
        while (1) {
@@ -1319,6 +1322,17 @@ static ssize_t h3_resp_headers_to_htx(struct qcs *qcs, const struct buffer *buf,
        if (fin)
                htx->flags |= HTX_FL_EOM;
 
+       /* Detect if an interim or final response was handled. */
+       if (sl->info.res.status >= 100 && sl->info.res.status < 200) {
+               TRACE_USER("receiving interim response", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
+               BUG_ON(!conn_is_back(qcs->qcc->conn)); /* H3_SF_RECV_INTERIM is BE side only. */
+               h3s->flags |= H3_SF_RECV_INTERIM;
+       }
+       else {
+               TRACE_USER("receiving final response", H3_EV_RX_FRAME|H3_EV_RX_HDR, qcs->qcc->conn, qcs);
+               h3s->flags &= ~H3_SF_RECV_INTERIM;
+       }
+
  out:
        if (appbuf)
                htx_to_buf(htx, appbuf);
@@ -1794,10 +1808,24 @@ static ssize_t h3_rcv_buf(struct qcs *qcs, struct buffer *b, int fin)
                        break;
                case H3_FT_HEADERS:
                        if (h3s->st_req == H3S_ST_REQ_BEFORE) {
-                               ret = !conn_is_back(qcs->qcc->conn) ?
-                                 h3_req_headers_to_htx(qcs, b, flen, last_stream_frame) :
-                                 h3_resp_headers_to_htx(qcs, b, flen, last_stream_frame);
-                               h3s->st_req = H3S_ST_REQ_HEADERS;
+                               if (!conn_is_back(qcs->qcc->conn)) {
+                                       ret = h3_req_headers_to_htx(qcs, b, flen, last_stream_frame);
+                                       h3s->st_req = H3S_ST_REQ_HEADERS;
+                               }
+                               else {
+                                       ret = h3_resp_headers_to_htx(qcs, b, flen, last_stream_frame);
+
+                                       /* Check if an interim or final response was parsed. */
+                                       if (h3s->flags & H3_SF_RECV_INTERIM) {
+                                               /* Interim resp: do not advance <st_req>. */
+                                               qcs->flags |= QC_SF_EOI_SUSPENDED;
+                                       }
+                                       else {
+                                               /* Final resp: mark <st_req> as headers received. */
+                                               h3s->st_req = H3S_ST_REQ_HEADERS;
+                                               qcs->flags &= ~QC_SF_EOI_SUSPENDED;
+                                       }
+                               }
                        }
                        else {
                                ret = h3_trailers_to_htx(qcs, b, flen, last_stream_frame);

diff --git a/src/qmux_http.c b/src/qmux_http.c
index 57b1eef350d0604188358c57e2f922bfde6a2cd2..9a310a397370df33b1b6eb627ecf94070e1ccda0 100644 (file)
--- a/src/qmux_http.c
+++ b/src/qmux_http.c
@@ -33,8 +33,10 @@ size_t qcs_http_rcv_buf(struct qcs *qcs, struct buffer *buf, size_t count,
        cs_htx = htx_from_buf(buf);
        if (htx_is_empty(cs_htx) && htx_used_space(qcs_htx) <= count) {
                /* EOM will be copied to cs_htx via b_xfer(). */
-               if (qcs_htx->flags & HTX_FL_EOM)
+               if ((qcs_htx->flags & HTX_FL_EOM) &&
+                   !(qcs->flags & QC_SF_EOI_SUSPENDED)) {
                        *fin = 1;
+               }
 
                htx_to_buf(cs_htx, buf);
                htx_to_buf(qcs_htx, &qcs->rx.app_buf);
@@ -48,7 +50,8 @@ size_t qcs_http_rcv_buf(struct qcs *qcs, struct buffer *buf, size_t count,
        /* Copy EOM from src to dst buffer if all data copied. */
        if (htx_is_empty(qcs_htx) && (qcs_htx->flags & HTX_FL_EOM)) {
                cs_htx->flags |= HTX_FL_EOM;
-               *fin = 1;
+               if (!(qcs->flags & QC_SF_EOI_SUSPENDED))
+                       *fin = 1;
        }
 
        cs_htx->extra = qcs_htx->extra ? (qcs_htx->data + qcs_htx->extra) : 0;