From 788227e66823b231b19ac73ee4ff15f39701c791 Mon Sep 17 00:00:00 2001 From: Dridi Boukelmoune Date: Wed, 27 Mar 2024 10:14:44 +0100 Subject: [PATCH 1/2] range: New http_GetContentRange() semantics This function returns the content length and puts range indices in lo and hi arguments. A content length of -1 used to be interpreted as something to ignore, but there is a case where the content length may be unknown. Since we can't represent a zero-length range, because both bounds are inclusive, zero now denotes the lack of content-range header. Unknown range units are treated as errors as they wouldn't pass the busyobj check for the range header, even for pass transactions. The only way to use "extension" range units is to turn http_range_support off. The calling convention for http_GetContentRange() remains otherwise the same, and for good measure http_GetContentLength() received a similar description. Fixes #4089 --- bin/varnishd/cache/cache_http.c | 34 +++++++++++++++++++++++++++----- bin/varnishd/cache/cache_range.c | 12 +++++------ bin/varnishtest/tests/c00034.vtc | 34 +++++++++++++++++++++++++++++++- 3 files changed, 68 insertions(+), 12 deletions(-) diff --git a/bin/varnishd/cache/cache_http.c b/bin/varnishd/cache/cache_http.c index 63818eaad3c..394aa7ed105 100644 --- a/bin/varnishd/cache/cache_http.c +++ b/bin/varnishd/cache/cache_http.c @@ -873,7 +873,14 @@ http_GetHdrField(const struct http *hp, hdr_t hdr, return (i); } -/*--------------------------------------------------------------------*/ +/*-------------------------------------------------------------------- + * Parse content-length header + * + * Return: + * -2 invalid + * -1 unknown length + * >= 0 body length + */ ssize_t http_GetContentLength(const struct http *hp) @@ -895,6 +902,20 @@ http_GetContentLength(const struct http *hp) return (cl); } +/*-------------------------------------------------------------------- + * Parse content-range header + * + * Return: + * -2 invalid + * -1 unknown length + * 0 not applicable + * > 0 full body length + * + * lo, hi: + * -1 unknown position + * >= 0 position + */ + ssize_t http_GetContentRange(const struct http *hp, ssize_t *lo, ssize_t *hi) { @@ -911,14 +932,14 @@ http_GetContentRange(const struct http *hp, ssize_t *lo, ssize_t *hi) *lo = *hi = -1; if (!http_GetHdr(hp, H_Content_Range, &b)) - return (-1); + return (0); // No content-range, ignore t = strchr(b, ' '); if (t == NULL) return (-2); // Missing space after range unit if (!http_range_at(b, bytes, t - b)) - return (-1); // Unknown range unit, ignore + return (-2); // Unknown range unit b = t + 1; if (*b == '*') { // Content-Range: bytes */123 @@ -937,6 +958,8 @@ http_GetContentRange(const struct http *hp, ssize_t *lo, ssize_t *hi) if (*b != '/') return (-2); if (b[1] == '*') { // Content-Range: bytes 1-2/* + if (*lo == -1 && *hi == -1) + return (-2); // Content-Range: bytes */* cl = -1; b += 2; } else { @@ -950,10 +973,11 @@ http_GetContentRange(const struct http *hp, ssize_t *lo, ssize_t *hi) return (-2); if (*lo > *hi) return (-2); - assert(cl >= -1); + if (cl == -1) + return (-1); + assert(cl > 0); if (*lo >= cl || *hi >= cl) return (-2); - AN(cl); return (cl); } diff --git a/bin/varnishd/cache/cache_range.c b/bin/varnishd/cache/cache_range.c index d7398f33f1e..73d1860309e 100644 --- a/bin/varnishd/cache/cache_range.c +++ b/bin/varnishd/cache/cache_range.c @@ -265,16 +265,16 @@ VRG_CheckBo(struct busyobj *bo) return (-1); } - if (crlo < 0 && crhi < 0 && crlen < 0) { - AZ(http_GetHdr(bo->beresp, H_Content_Range, NULL)); - return (0); - } - - if (rlo < 0 && rhi < 0) { + if (rlo < 0 && rhi < 0 && crlen != 0) { VSLb(bo->vsl, SLT_Error, "Unexpected content-range header"); return (-1); } + if (crlo < 0 && crhi < 0 && crlen == 0) { + AZ(http_GetHdr(bo->beresp, H_Content_Range, NULL)); + return (0); + } + if (crlo < 0) { // Content-Range: bytes */123 assert(crhi < 0); assert(crlen > 0); diff --git a/bin/varnishtest/tests/c00034.vtc b/bin/varnishtest/tests/c00034.vtc index 4858d3967a2..af063f166e1 100644 --- a/bin/varnishtest/tests/c00034.vtc +++ b/bin/varnishtest/tests/c00034.vtc @@ -134,7 +134,7 @@ varnish v1 -vsl_catchup server s1 { loop 2 { rxreq - txresp -nolen -hdr "Content-Length: 100" + txresp -hdr "Content-Length: 100" send "0123456789" send "0123456789" send "0123456789" @@ -210,8 +210,13 @@ varnish v1 -vsl_catchup server s1 { rxreq txresp + rxreq txresp -hdr "Accept-Ranges: foobar" + + rxreq + expect req.http.Range == foobar=1-2 + txresp -status 206 -hdr "Content-Range: foobar 1-2/3" } -start varnish v1 -vcl+backend { @@ -228,10 +233,16 @@ client c7 { rxresp expect resp.http.foobar == "" expect resp.http.accept-ranges == + txreq rxresp expect resp.http.foobar == foobar expect resp.http.accept-ranges == foobar + + txreq -hdr "Range: foobar=1-2" + rxresp + expect resp.status == 503 + expect resp.http.content-range == } -run server s1 { @@ -265,6 +276,18 @@ server s1 { rxreq expect req.http.range == "bytes=0-0" txresp -hdr "content-range: bytes */*" -bodylen 100 + + rxreq + expect req.http.range == "bytes=0-0" + txresp -hdr "content-range: bytes 0-0/*" -bodylen 100 + + # issue 4089 + rxreq + expect req.http.range == "bytes=10-19" + txresp -status 206 -hdr "content-range: bytes 10-19/*" \ + -hdr "transfer-encoding: chunked" + chunkedlen 10 + chunkedlen 0 } -start varnish v1 -vcl+backend { @@ -305,6 +328,15 @@ client c8 { txreq -hdr "range: bytes=0-0" -hdr "return: pass" rxresp expect resp.status == 503 + + txreq -hdr "range: bytes=0-0" -hdr "return: pass" + rxresp + expect resp.status == 503 + + # issue 4089 + txreq -hdr "range: bytes=10-19" -hdr "return: pass" + rxresp + expect resp.status == 206 } -run # range filter without a range header From e4000944d541a576bb3ebe514ba542e21306ebe4 Mon Sep 17 00:00:00 2001 From: Dridi Boukelmoune Date: Wed, 27 Mar 2024 10:25:54 +0100 Subject: [PATCH 2/2] vrt: Register http_GetContentRange() change --- include/vrt.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/include/vrt.h b/include/vrt.h index 0f1df8984b7..2f1138ed73b 100644 --- a/include/vrt.h +++ b/include/vrt.h @@ -57,6 +57,8 @@ * Whenever something is deleted or changed in a way which is not * binary/load-time compatible, increment MAJOR version * + * NEXT (2024-09-15) + * [cache.h] http_GetContentRange() changed * 19.0 (2024-03-18) * [cache.h] (struct req).filter_list renamed to vdp_filter_list * order of vcl/vmod and director COLD events reversed to directors first