diff --git a/bin/varnishd/http1/cache_http1_vfp.c b/bin/varnishd/http1/cache_http1_vfp.c index 13e042e..8d3ffba 100644 --- a/bin/varnishd/http1/cache_http1_vfp.c +++ b/bin/varnishd/http1/cache_http1_vfp.c @@ -91,76 +91,118 @@ v1f_read(const struct vfp_ctx *vc, struct http_conn *htc, void *d, ssize_t len) /*-------------------------------------------------------------------- - * Read a chunked HTTP object. + * read (CR)?LF at the end of a chunk + */ +static enum vfp_status +v1f_chunk_end(struct vfp_ctx *vc, struct http_conn *htc) +{ + char c; + + if (v1f_read(vc, htc, &c, 1) <= 0) + return (VFP_Error(vc, "chunked read err")); + if (c == '\r' && v1f_read(vc, htc, &c, 1) <= 0) + return (VFP_Error(vc, "chunked read err")); + if (c != '\n') + return (VFP_Error(vc, "chunked tail no NL")); + return (VFP_OK); +} + + +/*-------------------------------------------------------------------- + * Parse a chunk header and, for VFP_OK, return size in a pointer * * XXX: Reading one byte at a time is pretty pessimal. */ -static enum vfp_status v_matchproto_(vfp_pull_f) -v1f_pull_chunked(struct vfp_ctx *vc, struct vfp_entry *vfe, void *ptr, - ssize_t *lp) +static enum vfp_status +v1f_chunked_hdr(struct vfp_ctx *vc, struct http_conn *htc, ssize_t *szp) { - struct http_conn *htc; char buf[20]; /* XXX: 20 is arbitrary */ - char *q; unsigned u; uintmax_t cll; - ssize_t cl, l, lr; + ssize_t cl, lr; + char *q; CHECK_OBJ_NOTNULL(vc, VFP_CTX_MAGIC); - CHECK_OBJ_NOTNULL(vfe, VFP_ENTRY_MAGIC); - CAST_OBJ_NOTNULL(htc, vfe->priv1, HTTP_CONN_MAGIC); - AN(ptr); - AN(lp); + CHECK_OBJ_NOTNULL(htc, HTTP_CONN_MAGIC); + AN(szp); + assert(*szp == -1); - l = *lp; - *lp = 0; - if (vfe->priv2 == -1) { - /* Skip leading whitespace */ - do { - lr = v1f_read(vc, htc, buf, 1); - if (lr <= 0) - return (VFP_Error(vc, "chunked read err")); - } while (vct_islws(buf[0])); - - if (!vct_ishex(buf[0])) - return (VFP_Error(vc, "chunked header non-hex")); - - /* Collect hex digits, skipping leading zeros */ - for (u = 1; u < sizeof buf; u++) { - do { - lr = v1f_read(vc, htc, buf + u, 1); - if (lr <= 0) - return (VFP_Error(vc, - "chunked read err")); - } while (u == 1 && buf[0] == '0' && buf[u] == '0'); - if (!vct_ishex(buf[u])) - break; - } + /* Skip leading whitespace */ + do { + lr = v1f_read(vc, htc, buf, 1); + if (lr <= 0) + return (VFP_Error(vc, "chunked read err")); + } while (vct_isows(buf[0])); - if (u >= sizeof buf) - return (VFP_Error(vc, "chunked header too long")); + if (!vct_ishex(buf[0])) + return (VFP_Error(vc, "chunked header non-hex")); - /* Skip trailing white space */ - while (vct_islws(buf[u]) && buf[u] != '\n') { + /* Collect hex digits, skipping leading zeros */ + for (u = 1; u < sizeof buf; u++) { + do { lr = v1f_read(vc, htc, buf + u, 1); if (lr <= 0) return (VFP_Error(vc, "chunked read err")); - } + } while (u == 1 && buf[0] == '0' && buf[u] == '0'); + if (!vct_ishex(buf[u])) + break; + } - if (buf[u] != '\n') - return (VFP_Error(vc, "chunked header no NL")); + if (u >= sizeof buf) + return (VFP_Error(vc, "chunked header too long")); - buf[u] = '\0'; + /* Skip trailing white space */ + while (vct_isows(buf[u])) { + lr = v1f_read(vc, htc, buf + u, 1); + if (lr <= 0) + return (VFP_Error(vc, "chunked read err")); + } + + if (buf[u] == '\r' && v1f_read(vc, htc, buf + u, 1) <= 0) + return (VFP_Error(vc, "chunked read err")); + if (buf[u] != '\n') + return (VFP_Error(vc, "chunked header no NL")); - cll = strtoumax(buf, &q, 16); - if (q == NULL || *q != '\0') - return (VFP_Error(vc, "chunked header number syntax")); - cl = (ssize_t)cll; - if (cl < 0 || (uintmax_t)cl != cll) - return (VFP_Error(vc, "bogusly large chunk size")); + buf[u] = '\0'; - vfe->priv2 = cl; + cll = strtoumax(buf, &q, 16); + if (q == NULL || *q != '\0') + return (VFP_Error(vc, "chunked header number syntax")); + cl = (ssize_t)cll; + if (cl < 0 || (uintmax_t)cl != cll) + return (VFP_Error(vc, "bogusly large chunk size")); + + *szp = cl; + return (VFP_OK); +} + + +/*-------------------------------------------------------------------- + * Read a chunked HTTP object. + * + */ + +static enum vfp_status v_matchproto_(vfp_pull_f) +v1f_chunked_pull(struct vfp_ctx *vc, struct vfp_entry *vfe, void *ptr, + ssize_t *lp) +{ + static enum vfp_status vfps; + struct http_conn *htc; + ssize_t l, lr; + + CHECK_OBJ_NOTNULL(vc, VFP_CTX_MAGIC); + CHECK_OBJ_NOTNULL(vfe, VFP_ENTRY_MAGIC); + CAST_OBJ_NOTNULL(htc, vfe->priv1, HTTP_CONN_MAGIC); + AN(ptr); + AN(lp); + + l = *lp; + *lp = 0; + if (vfe->priv2 == -1) { + vfps = v1f_chunked_hdr(vc, htc, &vfe->priv2); + if (vfps != VFP_OK) + return (vfps); } if (vfe->priv2 > 0) { if (vfe->priv2 < l) @@ -170,30 +212,27 @@ v1f_pull_chunked(struct vfp_ctx *vc, struct vfp_entry *vfe, void *ptr, return (VFP_Error(vc, "chunked insufficient bytes")); *lp = lr; vfe->priv2 -= lr; - if (vfe->priv2 == 0) - vfe->priv2 = -1; - return (VFP_OK); + if (vfe->priv2 != 0) + return (VFP_OK); + + vfe->priv2 = -1; + return (v1f_chunk_end(vc, htc)); } AZ(vfe->priv2); - if (v1f_read(vc, htc, buf, 1) <= 0) - return (VFP_Error(vc, "chunked read err")); - if (buf[0] == '\r' && v1f_read(vc, htc, buf, 1) <= 0) - return (VFP_Error(vc, "chunked read err")); - if (buf[0] != '\n') - return (VFP_Error(vc, "chunked tail no NL")); - return (VFP_END); + vfps = v1f_chunk_end(vc, htc); + return (vfps == VFP_OK ? VFP_END : vfps); } static const struct vfp v1f_chunked = { .name = "V1F_CHUNKED", - .pull = v1f_pull_chunked, + .pull = v1f_chunked_pull, }; /*--------------------------------------------------------------------*/ static enum vfp_status v_matchproto_(vfp_pull_f) -v1f_pull_straight(struct vfp_ctx *vc, struct vfp_entry *vfe, void *p, +v1f_straight_pull(struct vfp_ctx *vc, struct vfp_entry *vfe, void *p, ssize_t *lp) { ssize_t l, lr; @@ -224,13 +263,13 @@ v1f_pull_straight(struct vfp_ctx *vc, struct vfp_entry *vfe, void *p, static const struct vfp v1f_straight = { .name = "V1F_STRAIGHT", - .pull = v1f_pull_straight, + .pull = v1f_straight_pull, }; /*--------------------------------------------------------------------*/ static enum vfp_status v_matchproto_(vfp_pull_f) -v1f_pull_eof(struct vfp_ctx *vc, struct vfp_entry *vfe, void *p, ssize_t *lp) +v1f_eof_pull(struct vfp_ctx *vc, struct vfp_entry *vfe, void *p, ssize_t *lp) { ssize_t l, lr; struct http_conn *htc; @@ -255,7 +294,7 @@ v1f_pull_eof(struct vfp_ctx *vc, struct vfp_entry *vfe, void *p, ssize_t *lp) static const struct vfp v1f_eof = { .name = "V1F_EOF", - .pull = v1f_pull_eof, + .pull = v1f_eof_pull, }; /*-------------------------------------------------------------------- diff --git a/bin/varnishtest/tests/r01184.vtc b/bin/varnishtest/tests/r01184.vtc index d5aba32..f7cd8d7 100644 --- a/bin/varnishtest/tests/r01184.vtc +++ b/bin/varnishtest/tests/r01184.vtc @@ -62,6 +62,7 @@ server s1 { sendhex " 10 45 f3 a9 83 b8 18 1c 7b c2 30 55 04 17 13 c4" sendhex " 0f 07 5f 7a 38 f4 8e 50 b3 37 d4 3a 32 4a 34 07" sendhex " FF FF FF FF FF FF FF FF 72 ea 06 5f b3 1c fa dd" + send "\n" expect_close } -start @@ -93,6 +94,7 @@ server s1 { sendhex " 10 45 f3 a9 83 b8 18 1c 7b c2 30 55 04 17 13 c4" sendhex " 0f 07 5f 7a 38 f4 8e 50 b3 37 d4 3a 32 4a 34 07" sendhex " FF FF FF FF FF FF FF FF 72 ea 06 5f b3 1c fa dd" + send "\n" expect_close } -start diff --git a/bin/varnishtest/tests/r01729.vtc b/bin/varnishtest/tests/r01729.vtc index 883a60c..f6a01e9 100644 --- a/bin/varnishtest/tests/r01729.vtc +++ b/bin/varnishtest/tests/r01729.vtc @@ -11,7 +11,7 @@ server s1 { send "\r\n" send "14\r\n" send "0123456789" - send "0123456789" + send "0123456789\n" send "0\r\n" send "\r\n" @@ -29,7 +29,7 @@ client c1 { send "\r\n" send "14\r\n" send "0123456789" - send "0123456789" + send "0123456789\n" send "0\r\n" send "\r\n" @@ -45,7 +45,7 @@ client c1 { send "\r\n" send "14\r\n" send "0123456789" - send "0123456789" + send "0123456789\n" send "0\r\n" send "\r\n" diff --git a/bin/varnishtest/tests/f00016.vtc b/bin/varnishtest/tests/f00016.vtc new file mode 100644 index 0000000..a38b8b1 --- /dev/null +++ b/bin/varnishtest/tests/f00016.vtc @@ -0,0 +1,69 @@ +varnishtest "Do not tolerate anything else than CRLF as chunked ending" + +server s0 { + rxreq + expect_close +} -dispatch + +varnish v1 -vcl+backend {} -start + +logexpect l1 -v v1 { + expect * 1001 FetchError "chunked tail no NL" + expect * 1004 FetchError "chunked tail no NL" + expect * 1007 FetchError "chunked header non-hex" + expect * 1010 FetchError "chunked header non-hex" +} -start + +client c1 { + non_fatal + txreq -req POST -hdr "Transfer-encoding: chunked" + send "1\r\n" + send "This is more than one byte of data\r\n" + send "0\r\n" + send "\r\n" + fatal + rxresp + expect resp.status == 503 + expect_close +} -run + +client c2 { + non_fatal + txreq -req POST -hdr "Transfer-encoding: chunked" + send "1\r\n" + send "Z 2\r\n" + send "3d\r\n" + send "0\r\n\r\nPOST /evil HTTP/1.1\r\nHost: whatever\r\nContent-Length: 5\r\n\r\n" + send "0\r\n" + send "\r\n" + fatal + rxresp + expect resp.status == 503 + expect_close +} -run + +client c3 { + non_fatal + txreq -req POST -hdr "Transfer-encoding: chunked" + send "d\r\n" + send "Spurious CRLF\r\n\r\n" + send "0\r\n" + send "\r\n" + fatal + rxresp + expect resp.status == 503 + expect_close +} -run + +client c4 { + non_fatal + txreq -req POST -hdr "Transfer-encoding: chunked" + send "\n0\r\n" + send "\r\n" + fatal + rxresp + expect resp.status == 503 + expect_close +} -run + +logexpect l1 -wait