Major tour through the Range code to make us more strict and RFC7233 conforming.

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;
+	}
 }

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

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

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