BUG/MINOR: mux-h2: always compare content-length to the sum of DATA frames

This is mandated by RFC7541#8.1.2.6. Till now we didn't have a copy of
the content-length header field. But now that it's already parsed, it's
easy to add the check.

The reg-test was updated to match the new behaviour as the previous one
expected unadvertised data to be silently discarded.

This should be backported to 1.9 along with previous patch (MEDIUM: h2:
always parse and deduplicate the content-length header) after it has got
a bit more exposure.

diff --git a/reg-tests/http-messaging/h00002.vtc b/reg-tests/http-messaging/h00002.vtc
index d2d0b9dc98fe68aa66df262588ee1d55c2fec735..481aded12aeea51ca4dba5de71d70f0c284e1995 100644 (file)
--- a/reg-tests/http-messaging/h00002.vtc
+++ b/reg-tests/http-messaging/h00002.vtc
@@ -30,10 +30,7 @@ server s1 {
          -body "response 3"
 
        barrier b4 sync
-       rxreq
-       txresp \
-         -status 200 \
-         -body "response 4"
+       # the next request is never received
 } -repeat 2 -start
 
 haproxy h1 -conf {
@@ -104,23 +101,19 @@ client c1h2 -connect ${h1_feh2_sock} {
                expect resp.body == "response 3"
        } -run
 
-       # fourth request is valid and advertises C-L:0, and close, and is
-       # followed by a string "this is not sent\r\n\r\n" which must be
-       # dropped.
+       # fourth request is valid and advertises C-L:2, and close, and is
+       # followed by a string "this is not sent\r\n\r\n" which causes a
+       # stream error of type PROTOCOL_ERROR.
        stream 7 {
                barrier b4 sync
                txreq \
                  -req "GET" \
                  -scheme "https" \
                  -url "/test4.html" \
-                 -hdr "content-length" "0" \
+                 -hdr "content-length" "2" \
                  -nostrend
                txdata -data "this is sent and ignored"
-               rxwinup
-               rxhdrs
-               expect resp.status == 200
-               rxdata -all
-               expect resp.body == "response 4"
+               rxrst
        } -run
 } -run
 
@@ -250,22 +243,18 @@ client c3h2 -connect ${h1_feh2_sock} {
                expect resp.body == "response 3"
        } -run
 
-       # fourth request is valid and advertises C-L:0, and close, and is
-       # followed by a string "this is not sent\r\n\r\n" which must be
-       # dropped.
+       # fourth request is valid and advertises C-L:2, and close, and is
+       # followed by a string "this is not sent\r\n\r\n" which results
+       # in a stream error.
        stream 7 {
                barrier b4 sync
                txreq \
                  -req "POST" \
                  -scheme "https" \
                  -url "/test24.html" \
-                 -hdr "content-length" "0" \
+                 -hdr "content-length" "2" \
                  -nostrend
                txdata -data "this is sent and ignored"
-               rxwinup
-               rxhdrs
-               expect resp.status == 200
-               rxdata -all
-               expect resp.body == "response 4"
+               rxrst
        } -run
 } -run

diff --git a/src/mux_h2.c b/src/mux_h2.c
index c0b62303b8b4eb7769fd67fad51c9c1dd202c9b5..586ff5163e07f60a6feb14111b5e8dbda51ea93f 100644 (file)
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -192,6 +192,7 @@ struct h2s {
        enum h2_err errcode; /* H2 err code (H2_ERR_*) */
        enum h2_ss st;
        uint16_t status;     /* HTTP response status */
+       unsigned long long body_len; /* remaining body length according to content-length if H2_SF_DATA_CLEN */
        struct buffer rxbuf; /* receive buffer, always valid (buf_empty or real buffer) */
        struct wait_event wait_event; /* Wait list, when we're attempting to send a RST but we can't send */
        struct wait_event *recv_wait; /* Address of the wait_event the conn_stream associated is waiting on */
@@ -871,6 +872,7 @@ static struct h2s *h2s_new(struct h2c *h2c, int id)
        h2s->errcode   = H2_ERR_NO_ERROR;
        h2s->st        = H2_SS_IDLE;
        h2s->status    = 0;
+       h2s->body_len  = 0;
        h2s->rxbuf     = BUF_NULL;
 
        if (h2c->flags & H2_CF_IS_BACK) {
@@ -1961,6 +1963,7 @@ static struct h2s *h2c_frt_handle_headers(struct h2c *h2c, struct h2s *h2s)
        h2s->st = H2_SS_OPEN;
        h2s->rxbuf = rxbuf;
        h2s->flags |= flags;
+       h2s->body_len = body_len;
 
  done:
        if (h2c->dff & H2_F_HEADERS_END_STREAM)
@@ -2002,7 +2005,6 @@ static struct h2s *h2c_frt_handle_headers(struct h2c *h2c, struct h2s *h2s)
  */
 static struct h2s *h2c_bck_handle_headers(struct h2c *h2c, struct h2s *h2s)
 {
-       unsigned long long body_len = 0;
        int error;
 
        if (!h2c->dfl) {
@@ -2018,7 +2020,7 @@ static struct h2s *h2c_bck_handle_headers(struct h2c *h2c, struct h2s *h2s)
        if (b_data(&h2c->dbuf) < h2c->dfl && !b_full(&h2c->dbuf))
                return NULL; // incomplete frame
 
-       error = h2c_decode_headers(h2c, &h2s->rxbuf, &h2s->flags, &body_len);
+       error = h2c_decode_headers(h2c, &h2s->rxbuf, &h2s->flags, &h2s->body_len);
 
        /* unrecoverable error ? */
        if (h2c->st0 >= H2_CS_ERROR)
@@ -2084,6 +2086,14 @@ static int h2c_frt_handle_data(struct h2c *h2c, struct h2s *h2s)
                goto strm_err;
        }
 
+       if ((h2s->flags & H2_SF_DATA_CLEN) && h2c->dfl > h2s->body_len) {
+               /* RFC7540#8.1.2 */
+               error = H2_ERR_PROTOCOL_ERROR;
+               goto strm_err;
+       }
+
+       printf("bl=%d dfl=%d dpl=%d\n", (int)h2s->body_len, (int)h2c->dfl, (int)h2c->dpl);
+
        if (!h2_frt_transfer_data(h2s))
                return 0;
 
@@ -2109,12 +2119,17 @@ static int h2c_frt_handle_data(struct h2c *h2c, struct h2s *h2s)
        if (h2c->st0 == H2_CS_FRAME_P)
                return 0;
 
-
        /* last frame */
        if (h2c->dff & H2_F_DATA_END_STREAM) {
                h2s->st = H2_SS_HREM;
                h2s->flags |= H2_SF_ES_RCVD;
                h2s->cs->flags |= CS_FL_REOS;
+
+               if (h2s->flags & H2_SF_DATA_CLEN && h2s->body_len) {
+                       /* RFC7540#8.1.2 */
+                       error = H2_ERR_PROTOCOL_ERROR;
+                       goto strm_err;
+               }
        }
 
        return 1;
@@ -3606,6 +3621,9 @@ try_again:
                h2c->dfl    -= flen;
                h2c->rcvd_c += flen;
                h2c->rcvd_s += flen;  // warning, this can also affect the closed streams!
+
+               if (h2s->flags & H2_SF_DATA_CLEN)
+                       h2s->body_len -= flen;
                goto try_again;
        }
        else if (unlikely(b_space_wraps(csbuf))) {
@@ -3675,6 +3693,9 @@ try_again:
        h2c->rcvd_c += flen;
        h2c->rcvd_s += flen;  // warning, this can also affect the closed streams!
 
+       if (h2s->flags & H2_SF_DATA_CLEN)
+               h2s->body_len -= flen;
+
        if (h2c->dfl > h2c->dpl) {
                /* more data available, transfer stalled on stream full */
                h2c->flags |= H2_CF_DEM_SFULL;