BUG/MEDIUM: h2: implement missing support for chunked encoded uploads

Upload requests not carrying a content-length nor tunnelling data must
be sent chunked-encoded over HTTP/1. The code was planned but for some
reason forgotten during the implementation, leading to such payloads to
be sent as tunnelled data.

Browsers always emit a content length in uploads so this problem doesn't
happen for most sites. However some applications may send data frames
after a request without indicating it earlier.

The only way to detect that a client will need to send data is that the
HEADERS frame doesn't hold the ES bit. In this case it's wise to look
for the content-length header. If it's not there, either we're in tunnel
(CONNECT method) or chunked-encoding (other methods).

This patch implements this.

The following request is sent using content-length :

    curl --http2 -sk https://127.0.0.1:4443/s2 -XPOST -T /large/file

and these ones using chunked-encoding :

    curl --http2 -sk https://127.0.0.1:4443/s2 -XPUT -T /large/file
    curl --http2 -sk https://127.0.0.1:4443/s2 -XPUT -T - < /dev/urandom

Thanks to Robert Samuel Newson for raising this issue with details.
This fix must be backported to 1.8.

diff --git a/src/h2.c b/src/h2.c
index 7d9ddd5026025f489dd523638f274ab0ce504c8c..5c83d6b67c6f47993996bdbc4c954f85042def6c 100644 (file)
--- a/src/h2.c
+++ b/src/h2.c
@@ -262,6 +262,14 @@ int h2_make_h1_request(struct http_hdr *list, char *out, int osize, unsigned int
                *(out++) = '\n';
        }
 
+       if ((*msgf & (H2_MSGF_BODY|H2_MSGF_BODY_TUNNEL|H2_MSGF_BODY_CL)) == H2_MSGF_BODY) {
+               /* add chunked encoding */
+               if (out + 28 > out_end)
+                       goto fail;
+               memcpy(out, "transfer-encoding: chunked\r\n", 28);
+               out += 28;
+       }
+
        /* now we may have to build a cookie list. We'll dump the values of all
         * visited headers.
         */

diff --git a/src/mux_h2.c b/src/mux_h2.c
index 5655bd8e0a67c1e900ce3872d7457fdd8d2e1e14..62a9f04d1e6d6156a9beab795f1902e6dca8b5dd 100644 (file)
--- a/src/mux_h2.c
+++ b/src/mux_h2.c
@@ -2745,6 +2745,7 @@ static int h2_frt_transfer_data(struct h2s *h2s, struct buffer *buf, int count)
        struct h2c *h2c = h2s->h2c;
        int block1, block2;
        unsigned int flen = h2c->dfl;
+       unsigned int chklen = 0;
 
        h2s->cs->flags &= ~CS_FL_RCV_MORE;
        h2c->flags &= ~H2_CF_DEM_SFULL;
@@ -2780,14 +2781,35 @@ static int h2_frt_transfer_data(struct h2s *h2s, struct buffer *buf, int count)
                        return 0;
        }
 
+       /* chunked-encoding requires more room */
+       if (h2s->flags & H2_SF_DATA_CHNK) {
+               chklen = MIN(flen, count);
+               chklen = (chklen < 16) ? 1 : (chklen < 256) ? 2 :
+                       (chklen < 4096) ? 3 : (chklen < 65536) ? 4 :
+                       (chklen < 1048576) ? 4 : 8;
+               chklen += 4; // CRLF, CRLF
+       }
+
        /* does it fit in output buffer or should we wait ? */
-       if (flen > count) {
-               flen = count;
-               if (!flen) {
-                       h2c->flags |= H2_CF_DEM_SFULL;
-                       h2s->cs->flags |= CS_FL_RCV_MORE;
-                       return 0;
-               }
+       if (flen + chklen > count) {
+               if (chklen >= count)
+                       goto full;
+               flen = count - chklen;
+       }
+
+       if (h2s->flags & H2_SF_DATA_CHNK) {
+               /* emit the chunk size */
+               unsigned int chksz = flen;
+               char str[10];
+               char *beg;
+
+               beg = str + sizeof(str);
+               *--beg = '\n';
+               *--beg = '\r';
+               do {
+                       *--beg = hextab[chksz & 0xF];
+               } while (chksz >>= 4);
+               bi_putblk(buf, beg, str + sizeof(str) - beg);
        }
 
        /* Block1 is the length of the first block before the buffer wraps,
@@ -2804,6 +2826,11 @@ static int h2_frt_transfer_data(struct h2s *h2s, struct buffer *buf, int count)
        if (block2)
                bi_putblk(buf, b_ptr(h2c->dbuf, block1), block2);
 
+       if (h2s->flags & H2_SF_DATA_CHNK) {
+               /* emit the CRLF */
+               bi_putblk(buf, "\r\n", 2);
+       }
+
        /* now mark the input data as consumed (will be deleted from the buffer
         * by the caller when seeing FRAME_A after sending the window update).
         */
@@ -2814,15 +2841,22 @@ static int h2_frt_transfer_data(struct h2s *h2s, struct buffer *buf, int count)
 
        if (h2c->dfl > h2c->dpl) {
                /* more data available, transfer stalled on stream full */
-               h2c->flags |= H2_CF_DEM_SFULL;
-               h2s->cs->flags |= CS_FL_RCV_MORE;
-               return flen;
+               goto more;
        }
 
  end_transfer:
        /* here we're done with the frame, all the payload (except padding) was
         * transferred.
         */
+
+       if (h2c->dff & H2_F_DATA_END_STREAM && h2s->flags & H2_SF_DATA_CHNK) {
+               /* emit the trailing 0 CRLF CRLF */
+               if (count < 5)
+                       goto more;
+               chklen += 5;
+               bi_putblk(buf, "0\r\n\r\n", 5);
+       }
+
        h2c->rcvd_c += h2c->dpl;
        h2c->rcvd_s += h2c->dpl;
        h2c->dpl = 0;
@@ -2837,7 +2871,13 @@ static int h2_frt_transfer_data(struct h2s *h2s, struct buffer *buf, int count)
                h2s->flags |= H2_SF_ES_RCVD;
        }
 
-       return flen;
+       return flen + chklen;
+ full:
+       flen = chklen = 0;
+ more:
+       h2c->flags |= H2_CF_DEM_SFULL;
+       h2s->cs->flags |= CS_FL_RCV_MORE;
+       return flen + chklen;
 }
 
 /*