Hi, Someone at the SF summit mentioned this so here's a patch for handling not satisfiable ranges (error code 416) and fixing some corner cases. I took the liberty of renaming some variables to follow the spec more closely. I've also changed V1D_Deliver() to only add Accept-Ranges for 200 responses. I believe this makes more sense and it's what another web server does but I'm fine either way. Tests updated.
Comments? OKs?
From 803195b3db3046b97de1c38fa90e75e2a64c5df9 Mon Sep 17 00:00:00 2001 From: "Federico G. Schwindt" <[email protected]> Date: Sat, 6 Dec 2014 13:14:57 +0000 Subject: [PATCH] Improve Range handling Add support for 416 and fix some corner cases. --- bin/varnishd/cache/cache_range.c | 85 +++++++++++++++++--------------- bin/varnishd/http1/cache_http1_deliver.c | 3 +- bin/varnishtest/tests/c00034.vtc | 76 +++++++++++++++------------- 3 files changed, 89 insertions(+), 75 deletions(-) diff --git a/bin/varnishd/cache/cache_range.c b/bin/varnishd/cache/cache_range.c index 2ba4587..3da1870 100644 --- a/bin/varnishd/cache/cache_range.c +++ b/bin/varnishd/cache/cache_range.c @@ -39,9 +39,9 @@ struct vrg_priv { unsigned magic; #define VRG_PRIV_MAGIC 0xb886e711 - ssize_t range_low; - ssize_t range_high; - ssize_t range_off; + ssize_t range_first; + ssize_t range_last; + ssize_t range_offset; }; static int __match_proto__(vdp_bytes) @@ -61,22 +61,22 @@ vrg_range_bytes(struct req *req, enum vdp_action act, void **priv, return (0); } CAST_OBJ_NOTNULL(vrg_priv, *priv, VRG_PRIV_MAGIC); - l = vrg_priv->range_low - vrg_priv->range_off; + l = vrg_priv->range_first - vrg_priv->range_offset; if (l > 0) { if (l > len) l = len; - vrg_priv->range_off += l; + vrg_priv->range_offset += l; p += l; len -= l; } - l = vrg_priv->range_high - vrg_priv->range_off; + l = vrg_priv->range_last - vrg_priv->range_offset; if (l > len) l = len; if (l > 0) retval = VDP_bytes(req, act, p, l); else if (act > VDP_NULL) retval = VDP_bytes(req, act, p, 0); - vrg_priv->range_off += len; + vrg_priv->range_offset += len; return (retval); } @@ -85,8 +85,9 @@ vrg_range_bytes(struct req *req, enum vdp_action act, void **priv, void VRG_dorange(struct req *req, struct busyobj *bo, const char *r) { - ssize_t len, low, high, has_low; + ssize_t len, first, last; struct vrg_priv *vrg_priv; + int byte_range; CHECK_OBJ_NOTNULL(req, REQ_MAGIC); CHECK_OBJ_NOTNULL(req->objcore, OBJCORE_MAGIC); @@ -98,65 +99,69 @@ VRG_dorange(struct req *req, struct busyobj *bo, const char *r) else len = ObjGetLen(req->wrk, req->objcore); - if (strncmp(r, "bytes=", 6)) + if (strncasecmp(r, "bytes=", 6)) return; r += 6; - /* The low end of range */ - has_low = low = 0; - if (!vct_isdigit(*r) && *r != '-') - return; + /* first-byte-pos */ + byte_range = first = 0; while (vct_isdigit(*r)) { - has_low = 1; - low *= 10; - low += *r - '0'; + byte_range = 1; + first *= 10; + first += *r - '0'; r++; } - if (low >= len) - return; - if (*r != '-') return; r++; - /* The high end of range */ + /* last-byte-pos or suffix-length */ if (vct_isdigit(*r)) { - high = 0; + last = 0; while (vct_isdigit(*r)) { - high *= 10; - high += *r - '0'; + last *= 10; + last += *r - '0'; r++; } - if (!has_low) { - low = len - high; - if (low < 0) - low = 0; - high = len - 1; - } - } else - high = len - 1; - if (*r != '\0') + if (last < first) + return; + if (!byte_range) { + first = len - last; + if (first < 0) + first = 0; + last = len - 1; + } else if (last >= len) + last = len - 1; + } else if (byte_range) + last = len - 1; + else return; - if (high >= len) - high = len - 1; + if (*r != '\0') + return; - if (low > high) + if (first >= len || (!byte_range && last == 0)) { + http_PrintfHeader(req->resp, "Content-Range: bytes */%jd", + (intmax_t)len); + http_Unset(req->resp, H_Content_Length); + http_PutResponse(req->resp, "HTTP/1.1", 416, NULL); + req->wantbody = 0; return; + } http_PrintfHeader(req->resp, "Content-Range: bytes %jd-%jd/%jd", - (intmax_t)low, (intmax_t)high, (intmax_t)len); + (intmax_t)first, (intmax_t)last, (intmax_t)len); http_Unset(req->resp, H_Content_Length); http_PrintfHeader(req->resp, "Content-Length: %jd", - (intmax_t)(1 + high - low)); + (intmax_t)(1 + last - first)); http_PutResponse(req->resp, "HTTP/1.1", 206, NULL); vrg_priv = WS_Alloc(req->ws, sizeof *vrg_priv); XXXAN(vrg_priv); INIT_OBJ(vrg_priv, VRG_PRIV_MAGIC); - vrg_priv->range_off = 0; - vrg_priv->range_low = low; - vrg_priv->range_high = high + 1; + vrg_priv->range_offset = 0; + vrg_priv->range_first = first; + vrg_priv->range_last = last + 1; VDP_push(req, vrg_range_bytes, vrg_priv, 1); } diff --git a/bin/varnishd/http1/cache_http1_deliver.c b/bin/varnishd/http1/cache_http1_deliver.c index c0eaeca..85b656f 100644 --- a/bin/varnishd/http1/cache_http1_deliver.c +++ b/bin/varnishd/http1/cache_http1_deliver.c @@ -121,9 +121,10 @@ V1D_Deliver(struct req *req, struct busyobj *bo) * can generate a correct C-L header. */ if (cache_param->http_range_support && http_IsStatus(req->resp, 200)) { - http_SetHeader(req->resp, "Accept-Ranges: bytes"); if (req->wantbody && http_GetHdr(req->http, H_Range, &r)) VRG_dorange(req, bo, r); + if (http_IsStatus(req->resp, 200)) + http_SetHeader(req->resp, "Accept-Ranges: bytes"); } diff --git a/bin/varnishtest/tests/c00034.vtc b/bin/varnishtest/tests/c00034.vtc index 23910de..eb5b975 100644 --- a/bin/varnishtest/tests/c00034.vtc +++ b/bin/varnishtest/tests/c00034.vtc @@ -5,13 +5,11 @@ server s1 { txresp -bodylen 100 } -start -varnish v1 -vcl+backend { - +varnish v1 -arg "-p http_range_support=off" -vcl+backend { sub vcl_backend_response { set beresp.do_stream = false; } } -start -varnish v1 -cliok "param.set http_range_support off" client c1 { txreq -hdr "Range: bytes=0-9" @@ -45,61 +43,71 @@ client c1 { expect resp.status == 200 expect resp.bodylen == 100 + txreq -hdr "Range: bytes=-" + rxresp + expect resp.status == 200 + expect resp.bodylen == 100 + + txreq -hdr "Range: bytes=5-2" + rxresp + expect resp.status == 200 + expect resp.bodylen == 100 } -run -varnish v1 -expect s_resp_bodybytes == 500 +varnish v1 -expect s_resp_bodybytes == 700 client c1 { - - txreq -hdr "Range: bytes=0-9" + txreq -hdr "Range: bytes=-0" rxresp - expect resp.status == 206 - expect resp.bodylen == 10 + expect resp.status == 416 + expect resp.bodylen == 0 + expect resp.http.content-range == "bytes */100" - txreq -hdr "Range: bytes=10-19" + txreq -hdr "Range: bytes=100-" rxresp - expect resp.status == 206 - expect resp.bodylen == 10 + expect resp.status == 416 + expect resp.bodylen == 0 + expect resp.http.content-range == "bytes */100" +} -run - txreq -hdr "Range: bytes=90-" - rxresp - expect resp.status == 206 - expect resp.bodylen == 10 +varnish v1 -expect s_resp_bodybytes == 700 - txreq -hdr "Range: bytes=-9" +client c1 { + txreq -hdr "Range: bytes=0-49" rxresp expect resp.status == 206 - expect resp.bodylen == 9 + expect resp.http.content-length == 50 + expect resp.http.content-range == "bytes 0-49/100" - txreq -hdr "Range: bytes=-" + txreq -hdr "Range: bytes=50-99" rxresp expect resp.status == 206 - expect resp.bodylen == 100 - - txreq -hdr "Range: bytes=102-102" - rxresp - expect resp.status == 200 - expect resp.bodylen == 100 + expect resp.http.content-length == 50 + expect resp.http.content-range == "bytes 50-99/100" - txreq -hdr "Range: bytes=99-" + txreq -hdr "Range: bytes=-50" rxresp expect resp.status == 206 - expect resp.bodylen == 1 + expect resp.http.content-length == 50 + expect resp.http.content-range == "bytes 50-99/100" - txreq -hdr "Range: bytes=99-70" + txreq -hdr "Range: bytes=50-" rxresp - expect resp.status == 200 - expect resp.bodylen == 100 + expect resp.status == 206 + expect resp.http.content-length == 50 + expect resp.http.content-range == "bytes 50-99/100" - txreq -hdr "Range: bytes=-" + txreq -hdr "Range: bytes=0-0" rxresp expect resp.status == 206 - expect resp.bodylen == 100 + expect resp.http.content-length == 1 + expect resp.http.content-range == "bytes 0-0/100" - txreq -hdr "Range: bytes=-101" + txreq -hdr "Range: bytes=-2000" rxresp expect resp.status == 206 - expect resp.bodylen == 100 + expect resp.http.content-length == 100 + expect resp.http.content-range == "bytes 0-99/100" } -run -varnish v1 -expect s_resp_bodybytes == 1040 +varnish v1 -expect s_resp_bodybytes == 1001 -- 2.1.3
_______________________________________________ varnish-dev mailing list [email protected] https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev
