[master] 4ab110047 vrg: Consistency check of backend responses

[Dridi Boukelmoune]

commit 4ab110047130e3a89936104d74f4729b650676f9
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date:   Mon Aug 23 17:27:36 2021 +0200

    vrg: Consistency check of backend responses
    
    That is, only when http_range_support is on, which is the default.
    
    Refs #3246

diff --git a/bin/varnishd/cache/cache_fetch.c b/bin/varnishd/cache/cache_fetch.c
index 16ab14558..eb09990ff 100644
--- a/bin/varnishd/cache/cache_fetch.c
+++ b/bin/varnishd/cache/cache_fetch.c
@@ -492,6 +492,11 @@ vbf_stp_startfetch(struct worker *wrk, struct busyobj *bo)
 	    http_GetHdrField(bo->beresp, H_Connection, "close", NULL))
 		bo->htc->doclose = SC_RESP_CLOSE;
 
+	if (VRG_CheckBo(bo) < 0) {
+		VDI_Finish(bo);
+		return (F_STP_ERROR);
+	}
+
 	if (wrk->handling == VCL_RET_ABANDON || wrk->handling == VCL_RET_FAIL ||
 	    wrk->handling == VCL_RET_ERROR) {
 		/* do not count deliberately ending the backend connection as
diff --git a/bin/varnishd/cache/cache_range.c b/bin/varnishd/cache/cache_range.c
index 75c434a94..20f326455 100644
--- a/bin/varnishd/cache/cache_range.c
+++ b/bin/varnishd/cache/cache_range.c
@@ -255,3 +255,73 @@ const struct vdp VDP_range = {
 	.bytes =	vrg_range_bytes,
 	.fini =		vrg_range_fini,
 };
+
+/*--------------------------------------------------------------------*/
+
+int
+VRG_CheckBo(struct busyobj *bo)
+{
+	ssize_t rlo, rhi, crlo, crhi, crlen, clen;
+	const char *err;
+
+	CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC);
+
+	if (!cache_param->http_range_support)
+		return (0);
+
+	err = http_GetRange(bo->bereq0, &rlo, &rhi);
+	clen = http_GetContentLength(bo->beresp);
+	crlen = http_GetContentRange(bo->beresp, &crlo, &crhi);
+
+	if (err != NULL) {
+		VSLb(bo->vsl, SLT_Error, "Invalid range header (%s)", err);
+		return (-1);
+	}
+
+	if (crlen < -1) {
+		VSLb(bo->vsl, SLT_Error, "Invalid content-range header");
+		return (-1);
+	}
+
+	if (clen < -1) {
+		VSLb(bo->vsl, SLT_Error, "Invalid content-length header");
+		return (-1);
+	}
+
+	if (crlo < 0 || crhi < 0) {
+		AZ(http_GetHdr(bo->beresp, H_Content_Range, NULL));
+		return (0);
+	}
+
+	if (rlo < 0 && rhi < 0) {
+		VSLb(bo->vsl, SLT_Error, "Unexpected content-range header");
+		return (-1);
+	}
+
+#define RANGE_CHECK(val, op, crval, what)			\
+	do {							\
+		if (val >= 0 && !(val op crval)) {		\
+			VSLb(bo->vsl, SLT_Error,		\
+			    "Expected " what " %zd, got %zd",	\
+			    crval, val);			\
+			return (-1);				\
+		}						\
+	} while (0)
+
+	crlen = crhi - crlo + 1;
+	RANGE_CHECK(clen, ==, crlen, "content length");
+
+	/* NB: if the client didn't specify a low range the high range
+	 * was adjusted based on the resource length, and a high range
+	 * is allowed to be out of bounds so at this point there is
+	 * nothing left to check.
+	 */
+	if (rlo < 0)
+		return (0);
+
+	RANGE_CHECK(rlo, ==, crlo, "low range");
+	RANGE_CHECK(rhi, >=, crhi, "minimum high range");
+#undef RANGE_CHECK
+
+	return (0);
+}
diff --git a/bin/varnishd/cache/cache_varnishd.h b/bin/varnishd/cache/cache_varnishd.h
index 653976cdc..47213fff6 100644
--- a/bin/varnishd/cache/cache_varnishd.h
+++ b/bin/varnishd/cache/cache_varnishd.h
@@ -396,6 +396,9 @@ void Pool_PurgeStat(unsigned nobj);
 int Pool_Task_Any(struct pool_task *task, enum task_prio prio);
 void pan_pool(struct vsb *);
 
+/* cache_range.c */
+int VRG_CheckBo(struct busyobj *);
+
 /* cache_req.c */
 struct req *Req_New(const struct worker *, struct sess *);
 void Req_Release(struct req *);
diff --git a/bin/varnishtest/tests/c00034.vtc b/bin/varnishtest/tests/c00034.vtc
index a605977c4..44773ffe2 100644
--- a/bin/varnishtest/tests/c00034.vtc
+++ b/bin/varnishtest/tests/c00034.vtc
@@ -190,6 +190,7 @@ client c5 {
 
 server s1 {
 	rxreq
+	expect req.url == "/3"
 	txresp -hdr "Content-length: 3" -body "BLA"
 } -start
 
@@ -234,3 +235,42 @@ client c7 {
 	expect resp.http.foobar == foobar
 	expect resp.http.accept-ranges == foobar
 } -run
+
+server s1 {
+	rxreq
+	expect req.http.range == "bytes=10-19"
+	txresp -status 206 -hdr "content-range: bytes 0-49/100" -bodylen 40
+
+	rxreq
+	expect req.http.range == "bytes=10-19"
+	txresp -status 206 -hdr "content-range: bytes 10-19/100" -bodylen 40
+
+	rxreq
+	expect req.url == "/4"
+	expect req.http.range == <undef>
+	txresp -hdr "content-range: bytes 0-49/100" -bodylen 40
+} -start
+
+varnish v1 -vcl+backend {
+	sub vcl_recv {
+		if (req.http.return == "pass") {
+			return (pass);
+		}
+	}
+}
+
+client c8 {
+	# range vs content-range vs content-length
+
+	txreq -hdr "range: bytes=10-19" -hdr "return: pass"
+	rxresp
+	expect resp.status == 503
+
+	txreq -hdr "range: bytes=10-19" -hdr "return: pass"
+	rxresp
+	expect resp.status == 503
+
+	txreq -url /4
+	rxresp
+	expect resp.status == 503
+} -run
diff --git a/bin/varnishtest/tests/r00466.vtc b/bin/varnishtest/tests/r00466.vtc
index d530b443c..94c700a22 100644
--- a/bin/varnishtest/tests/r00466.vtc
+++ b/bin/varnishtest/tests/r00466.vtc
@@ -36,10 +36,14 @@ client c1 {
 	expect resp.status == 206
 	expect resp.http.Content-Length == 2
 	expect resp.http.X-Varnish == "1001"
+} -run
+
+varnish v1 -cliok "param.set http_range_support off"
 
+client c2 {
 	# NB: Deliberately bogus Range header
 	txreq -url "/bar" -hdr "Range: 200-300"
 	rxresp
 	expect resp.status == 206
-	expect resp.http.X-Varnish == "1003"
+	expect resp.http.X-Varnish == "1004"
 } -run
diff --git a/bin/varnishtest/tests/r02530.vtc b/bin/varnishtest/tests/r02530.vtc
index c458b6a5b..f664caee4 100644
--- a/bin/varnishtest/tests/r02530.vtc
+++ b/bin/varnishtest/tests/r02530.vtc
@@ -30,6 +30,8 @@ server s1 {
 	sendhex 1f8b
 } -start
 
+varnish v1 -cliok "param.set http_range_support off"
+
 varnish v1 -vcl+backend {
 	sub vcl_recv {
 		if (req.url == "/pass") {