BUG/MINOR: htx/mux-h1: Properly handle bodyless responses when splicing is used

There is a mechanisme in the H1 and H2 multiplexer to skip the payload when
a response is returned to the client when it must not contain any payload
(response to a HEAD request or a 204/304 response). However, this does not
work when the splicing is used. The H2 multiplexer does not support the
splicing, so there is no issue. But with the mux-h1, when data are sent
using the kernel splicing, the mux on the server side is not aware the
client side should skip the payload. And once the data are put in a pipe,
there is no way to stop the sending.

It is a defect of the current design. This will be easier to deal with this
case when the mux-to-mux forwarding will be implemented. But for now, to fix
the issue, we should add an HTX flag on the start-line to pass the info from
the client side to the server side and be able to disable the splicing in
necessary.

The associated reg-test was improved to be sure it does not fail when the
splicing is configured.

This patch should be backported as far as 2.4..

diff --git a/include/haproxy/htx-t.h b/include/haproxy/htx-t.h
index 1c970eb1b8ff759d2e0eeee6cdef8b91ed6c40df..687e403c7bc765cb53fd7f0df091f31c84e8a74c 100644 (file)
--- a/include/haproxy/htx-t.h
+++ b/include/haproxy/htx-t.h
@@ -140,6 +140,7 @@
 #define HTX_SL_F_HAS_AUTHORITY  0x00000400 /* The request authority is explicitly specified */
 #define HTX_SL_F_NORMALIZED_URI 0x00000800 /* The received URI is normalized (an implicit absolute-uri form) */
 #define HTX_SL_F_CONN_UPG       0x00001000 /* The message contains "connection: upgrade" header */
+#define HTX_SL_F_BODYLESS_RESP  0x00002000 /* The response to this message is bodyloess (only for reqyest) */
 
 /* 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/reg-tests/http-messaging/http_bodyless_response.vtc b/reg-tests/http-messaging/http_bodyless_response.vtc
index 9e0ce1c1b3f9d37957b5cb352a0d931a82e52274..5e95203661634d4ed108a5cc9f89a90ec4e94102 100644 (file)
--- a/reg-tests/http-messaging/http_bodyless_response.vtc
+++ b/reg-tests/http-messaging/http_bodyless_response.vtc
@@ -12,7 +12,7 @@ server s1 {
         rxreq
         txresp \
           -status 200 \
-          -bodylen 20000
+          -bodylen 50000
 
         rxreq
         txresp \
@@ -21,13 +21,14 @@ server s1 {
        chunkedlen 15
        chunkedlen 1024
        chunkedlen 4048
+       chunkedlen 50000
        chunkedlen 0
 
         rxreq
         txresp \
           -status 200 \
           -body "last response"
-} -repeat 2 -start
+} -repeat 3 -start
 
 haproxy h1 -conf {
     global
@@ -60,6 +61,14 @@ haproxy h1 -conf {
     listen fe2
         bind "fd@${fe2}"
         server s1 ${h1_int_addr}:${h1_int_port} proto h2
+
+    listen fe3
+        bind "fd@${fe3}"
+        # Rewrite the method to be sure to get the response payload
+        # on the server side
+        http-request set-method GET
+        option splice-response
+        server s1 ${s1_addr}:${s1_port}
 } -start
 
 client c1 -connect ${h1_fe1_sock} {
@@ -125,3 +134,35 @@ client c2 -connect ${h1_fe2_sock} {
         expect resp.status == 200
         expect resp.body == "last response"
 } -run
+
+client c3 -connect ${h1_fe3_sock} {
+        txreq \
+            -req "HEAD" \
+            -url "/req1"
+        rxresp
+        expect resp.status == 200
+        expect resp.body == ""
+
+        txreq \
+            -req "HEAD" \
+            -url "/req2"
+        rxresp
+        expect resp.status == 200
+        expect resp.body == ""
+
+        txreq \
+            -req "HEAD" \
+            -url "/req3"
+        rxresp
+        expect resp.status == 200
+        expect resp.body == ""
+
+        # The last one have a body and validate the connection was not closed
+        # unexpectedly and no payload was received for previous requests
+        txreq \
+            -req "GET" \
+            -url "/req4"
+        rxresp
+        expect resp.status == 200
+        expect resp.body == "last response"
+} -run

diff --git a/src/h1_htx.c b/src/h1_htx.c
index d913fe17bc03c58bd136c8532b2b173d13bca2e2..617169fdbfcbd6e3fe87d7bccfcd032d9fc501c5 100644 (file)
--- a/src/h1_htx.c
+++ b/src/h1_htx.c
@@ -152,7 +152,7 @@ static int h1_postparse_req_hdrs(struct h1m *h1m, union h1_sl *h1sl, struct htx
 {
        struct htx_sl *sl;
        struct ist meth, uri, vsn;
-       unsigned int flags;
+       unsigned int flags = 0;
 
        /* <h1sl> is always defined for a request */
        meth = h1sl->rq.m;
@@ -175,9 +175,11 @@ static int h1_postparse_req_hdrs(struct h1m *h1m, union h1_sl *h1sl, struct htx
                h1m->flags &= ~(H1_MF_CLEN|H1_MF_CHNK);
                h1m->curr_len = h1m->body_len = 0;
        }
+       else if (h1sl->rq.meth == HTTP_METH_HEAD)
+               flags |= HTX_SL_F_BODYLESS_RESP;
 
 
-       flags = h1m_htx_sl_flags(h1m);
+       flags |= h1m_htx_sl_flags(h1m);
        if ((flags & (HTX_SL_F_CONN_UPG|HTX_SL_F_BODYLESS)) == HTX_SL_F_CONN_UPG) {
                int i;
 
@@ -235,7 +237,7 @@ static int h1_postparse_res_hdrs(struct h1m *h1m, union h1_sl *h1sl, struct htx
 {
        struct htx_sl *sl;
        struct ist vsn, status, reason;
-       unsigned int flags;
+       unsigned int flags = 0;
        uint16_t code = 0;
 
        if (h1sl) {
@@ -293,13 +295,14 @@ static int h1_postparse_res_hdrs(struct h1m *h1m, union h1_sl *h1sl, struct htx
                h1m->flags &= ~(H1_MF_CLEN|H1_MF_CHNK);
                h1m->flags |= H1_MF_XFER_LEN;
                h1m->curr_len = h1m->body_len = 0;
+               flags |= HTX_SL_F_BODYLESS_RESP;
        }
        else if (h1m->flags & (H1_MF_CLEN|H1_MF_CHNK)) {
                /* Responses with a known body length. */
                h1m->flags |= H1_MF_XFER_LEN;
        }
 
-       flags = h1m_htx_sl_flags(h1m);
+       flags |= h1m_htx_sl_flags(h1m);
        sl = htx_add_stline(htx, HTX_BLK_RES_SL, flags, vsn, status, reason);
        if (!sl || !htx_add_all_headers(htx, hdrs))
                goto error;

diff --git a/src/mux_h1.c b/src/mux_h1.c
index 40f94cad5bd5b6f00b74716cfce4149cca989675..667eff50beb14641ae17e6eb857200691e7ddf12 100644 (file)
--- a/src/mux_h1.c
+++ b/src/mux_h1.c
@@ -1906,7 +1906,8 @@ static size_t h1_process_demux(struct h1c *h1c, struct buffer *buf, size_t count
        }
 
        /* Here h1s_sc(h1s) is always defined */
-       if (h1m->state == H1_MSG_DATA || (h1m->state == H1_MSG_TUNNEL)) {
+       if ((!(h1m->flags & H1_MF_RESP) || !(h1s->flags & H1S_F_BODYLESS_RESP)) &&
+           (h1m->state == H1_MSG_DATA || h1m->state == H1_MSG_TUNNEL)) {
                TRACE_STATE("notify the mux can use splicing", H1_EV_RX_DATA|H1_EV_RX_BODY, h1c->conn, h1s);
                se_fl_set(h1s->sd, SE_FL_MAY_SPLICE);
        }
@@ -2028,7 +2029,7 @@ static size_t h1_make_reqline(struct h1s *h1s, struct h1m *h1m, struct htx *htx,
        h1m->flags |= H1_MF_XFER_LEN;
        if (sl->flags & HTX_SL_F_BODYLESS)
                h1m->flags |= H1_MF_CLEN;
-       if (h1s->meth == HTTP_METH_HEAD)
+       if ((sl->flags & HTX_SL_F_BODYLESS_RESP) || h1s->meth == HTTP_METH_HEAD)
                h1s->flags |= H1S_F_BODYLESS_RESP;
 
        if (h1s->flags & H1S_F_RX_BLK) {
@@ -2105,7 +2106,7 @@ static size_t h1_make_stline(struct h1s *h1s, struct h1m *h1m, struct htx *htx,
                h1m->flags |= H1_MF_XFER_LEN;
        if (h1s->status < 200)
                h1s->flags |= H1S_F_HAVE_O_CONN;
-       else if (h1s->status == 204 || h1s->status == 304)
+       else if ((sl->flags & HTX_SL_F_BODYLESS_RESP) || h1s->status == 204 || h1s->status == 304)
                h1s->flags |= H1S_F_BODYLESS_RESP;
 
        h1m->state = H1_MSG_HDR_NAME;

diff --git a/src/mux_h2.c b/src/mux_h2.c
index 2e21088ae1d2f517ea113f23d8b93adf6cbf6a61..4120e0040541c18c8770be2ae14b53c1341e4040 100644 (file)
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -5195,7 +5195,7 @@ static size_t h2s_snd_fhdrs(struct h2s *h2s, struct htx *htx)
                        BUG_ON(sl); /* Only one start-line expected */
                        sl = htx_get_blk_ptr(htx, blk);
                        h2s->status = sl->info.res.status;
-                       if (h2s->status == 204 || h2s->status == 304)
+                       if ((sl->flags & HTX_SL_F_BODYLESS_RESP) || h2s->status == 204 || h2s->status == 304)
                                h2s->flags |= H2_SF_BODYLESS_RESP;
                        if (h2s->status < 100 || h2s->status > 999) {
                                TRACE_ERROR("will not encode an invalid status code", H2_EV_TX_FRAME|H2_EV_TX_HDR|H2_EV_H2S_ERR, h2c->conn, h2s);
@@ -5507,7 +5507,7 @@ static size_t h2s_snd_bhdrs(struct h2s *h2s, struct htx *htx)
                        sl = htx_get_blk_ptr(htx, blk);
                        meth = htx_sl_req_meth(sl);
                        uri  = htx_sl_req_uri(sl);
-                       if (sl->info.req.meth == HTTP_METH_HEAD)
+                       if ((sl->flags & HTX_SL_F_BODYLESS_RESP) || sl->info.req.meth == HTTP_METH_HEAD)
                                h2s->flags |= H2_SF_BODYLESS_RESP;
                        if (unlikely(uri.len == 0)) {
                                TRACE_ERROR("no URI in HTX request", H2_EV_TX_FRAME|H2_EV_TX_HDR|H2_EV_H2S_ERR, h2c->conn, h2s);