[master] 6acf177 Major tour through the Range code to make us more strict and RFC7233 conforming.
Poul-Henning Kamp
phk at FreeBSD.org
Mon May 4 21:50:01 CEST 2015
commit 6acf17779787ab832d4c2acf51d8941892e3e140
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date: Mon May 4 19:49:29 2015 +0000
Major tour through the Range code to make us more strict and RFC7233 conforming.
diff --git a/bin/varnishd/cache/cache_range.c b/bin/varnishd/cache/cache_range.c
index 1cb50fa..7562095 100644
--- a/bin/varnishd/cache/cache_range.c
+++ b/bin/varnishd/cache/cache_range.c
@@ -82,72 +82,64 @@ vrg_range_bytes(struct req *req, enum vdp_action act, void **priv,
/*--------------------------------------------------------------------*/
-void
-VRG_dorange(struct req *req, struct busyobj *bo, const char *r)
+static int
+vrg_dorange(struct req *req, const struct busyobj *bo, ssize_t len,
+ const char *r)
{
- ssize_t len, low, high, has_low;
+ ssize_t low, high, has_low, has_high, t;
struct vrg_priv *vrg_priv;
- CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
- CHECK_OBJ_NOTNULL(req->objcore, OBJCORE_MAGIC);
- assert(http_IsStatus(req->resp, 200));
-
- /* We must snapshot the length if we're streaming from the backend */
- if (bo != NULL)
- len = VBO_waitlen(req->wrk, bo, -1);
- else
- len = ObjGetLen(req->wrk, req->objcore);
-
+ (void)bo;
if (strncasecmp(r, "bytes=", 6))
- return;
+ return (__LINE__);
r += 6;
/* The low end of range */
has_low = low = 0;
while (vct_isdigit(*r)) {
has_low = 1;
+ t = low;
low *= 10;
- low += *r - '0';
- r++;
+ low += *r++ - '0';
+ if (low < t)
+ return (__LINE__);
}
- if (*r != '-')
- return;
- r++;
+ if (*r++ != '-')
+ return (__LINE__);
/* The high end of range */
- if (vct_isdigit(*r)) {
- high = 0;
- while (vct_isdigit(*r)) {
- high *= 10;
- high += *r - '0';
- r++;
- }
- if (high < low)
- return;
- if (!has_low) {
- low = len - high;
- if (low < 0)
- low = 0;
- high = len - 1;
- } else if (high >= len)
- high = len - 1;
- } else if (has_low)
- high = len - 1;
- else
- return;
+ has_high = high = 0;
+ while (vct_isdigit(*r)) {
+ has_high = 1;
+ t = high;
+ high *= 10;
+ high += *r++ - '0';
+ if (high < t)
+ return (__LINE__);
+ }
if (*r != '\0')
- return;
+ return (__LINE__);
- if (low >= len) {
- 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;
- }
+ if (has_high + has_low == 0)
+ return (__LINE__);
+
+ if (!has_low) {
+ if (high == 0)
+ return (__LINE__);
+ low = len - high;
+ if (low < 0)
+ low = 0;
+ high = len - 1;
+ } else if (high >= len || !has_high)
+ high = len - 1;
+
+ if (high < low)
+ return (__LINE__);
+
+ if (low >= len)
+ return (__LINE__);
http_PrintfHeader(req->resp, "Content-Range: bytes %jd-%jd/%jd",
(intmax_t)low, (intmax_t)high, (intmax_t)len);
@@ -161,4 +153,33 @@ VRG_dorange(struct req *req, struct busyobj *bo, const char *r)
vrg_priv->range_low = low;
vrg_priv->range_high = high + 1;
VDP_push(req, vrg_range_bytes, vrg_priv, 1);
+ return (0);
+}
+
+void
+VRG_dorange(struct req *req, struct busyobj *bo, const char *r)
+{
+ size_t len;
+ int i;
+
+ CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
+ CHECK_OBJ_NOTNULL(req->objcore, OBJCORE_MAGIC);
+ assert(http_IsStatus(req->resp, 200));
+
+ /* We must snapshot the length if we're streaming from the backend */
+ if (bo != NULL)
+ len = VBO_waitlen(req->wrk, bo, -1);
+ else
+ len = ObjGetLen(req->wrk, req->objcore);
+
+ i = vrg_dorange(req, bo, len, r);
+ if (i) {
+ VSLb(req->vsl, SLT_Debug, "RANGE_FAIL line %d", i);
+ http_Unset(req->resp, H_Content_Length);
+ if (bo == NULL)
+ http_PrintfHeader(req->resp,
+ "Content-Range: bytes */%jd", (intmax_t)len);
+ http_PutResponse(req->resp, "HTTP/1.1", 416, NULL);
+ req->wantbody = 0;
+ }
}
diff --git a/bin/varnishtest/tests/c00034.vtc b/bin/varnishtest/tests/c00034.vtc
index 9eeee15..f479498 100644
--- a/bin/varnishtest/tests/c00034.vtc
+++ b/bin/varnishtest/tests/c00034.vtc
@@ -5,12 +5,14 @@ server s1 {
txresp -bodylen 100
} -start
-varnish v1 -arg "-p http_range_support=off" -vcl+backend {
+varnish v1 -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"
rxresp
@@ -18,61 +20,61 @@ client c1 {
expect resp.bodylen == 100
} -run
-varnish v1 -cliok "param.set http_range_support on"
-
varnish v1 -expect s_resp_bodybytes == 100
+varnish v1 -cliok "param.set http_range_support on"
+
client c1 {
+ # Invalid range requests
+
txreq -hdr "Range: bytes =0-9"
rxresp
- expect resp.status == 200
- expect resp.bodylen == 100
+ expect resp.status == 416
+ expect resp.bodylen == 0
+ expect resp.http.content-range == "bytes */100"
txreq -hdr "Range: bytes=0- 9"
rxresp
- expect resp.status == 200
- expect resp.bodylen == 100
+ expect resp.status == 416
+ expect resp.bodylen == 0
txreq -hdr "Range: bytes =-9"
rxresp
- expect resp.status == 200
- expect resp.bodylen == 100
+ expect resp.status == 416
+ expect resp.bodylen == 0
txreq -hdr "Range: bytes =0-a"
rxresp
- expect resp.status == 200
- expect resp.bodylen == 100
+ expect resp.status == 416
+ expect resp.bodylen == 0
txreq -hdr "Range: bytes=-"
rxresp
- expect resp.status == 200
- expect resp.bodylen == 100
+ expect resp.status == 416
+ expect resp.bodylen == 0
txreq -hdr "Range: bytes=5-2"
rxresp
- expect resp.status == 200
- expect resp.bodylen == 100
-} -run
-
-varnish v1 -expect s_resp_bodybytes == 700
+ expect resp.status == 416
+ expect resp.bodylen == 0
-client c1 {
txreq -hdr "Range: bytes=-0"
rxresp
expect resp.status == 416
expect resp.bodylen == 0
- expect resp.http.content-range == "bytes */100"
txreq -hdr "Range: bytes=100-"
rxresp
expect resp.status == 416
expect resp.bodylen == 0
- expect resp.http.content-range == "bytes */100"
} -run
-varnish v1 -expect s_resp_bodybytes == 700
+varnish v1 -expect s_resp_bodybytes == 100
+delay .1
client c1 {
+ # Valid range requests
+
txreq -hdr "Range: bytes=0-49"
rxresp
expect resp.status == 206
@@ -110,4 +112,4 @@ client c1 {
expect resp.http.content-range == "bytes 0-99/100"
} -run
-varnish v1 -expect s_resp_bodybytes == 1001
+varnish v1 -expect s_resp_bodybytes == 401
diff --git a/bin/varnishtest/tests/r00466.vtc b/bin/varnishtest/tests/r00466.vtc
index b672681..813c1e7 100644
--- a/bin/varnishtest/tests/r00466.vtc
+++ b/bin/varnishtest/tests/r00466.vtc
@@ -28,11 +28,13 @@ varnish v1 -vcl+backend {
varnish v1 -cliok "param.set debug +syncvsl"
client c1 {
- txreq -url "/foo" -hdr "Range: 100-200"
+ txreq -url "/foo" -hdr "Range: bytes=1-2"
rxresp
- expect resp.status == 200
+ expect resp.status == 206
+ expect resp.http.Content-Length == 2
expect resp.http.X-Varnish == "1001"
+ # NB: Deliberately bogus Range header
txreq -url "/bar" -hdr "Range: 200-300"
rxresp
expect resp.status == 206
diff --git a/bin/varnishtest/tests/r01660.vtc b/bin/varnishtest/tests/r01660.vtc
index 1ce133c..5e2599d 100644
--- a/bin/varnishtest/tests/r01660.vtc
+++ b/bin/varnishtest/tests/r01660.vtc
@@ -14,5 +14,10 @@ varnish v1 -vcl+backend {
client c1 {
txreq -hdr "Range: 0-1"
rxresp
- expect resp.status == 200
+ expect resp.status == 416
+
+ txreq -hdr "Range: bytes=0-1"
+ rxresp
+ expect resp.status == 206
+ expect resp.http.content-length == 2
} -run
More information about the varnish-commit
mailing list