[4.1] 94a7f42 Don't test gunzip for partial responses

Dridi Boukelmoune dridi.boukelmoune at gmail.com
Fri Feb 23 15:23:07 UTC 2018


commit 94a7f427c21d8bd0a91541b29141471c8cab220a
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date:   Tue Jan 9 13:29:17 2018 +0100

    Don't test gunzip for partial responses
    
    Some user agents like Safari may "probe" specific resources like medias
    before getting the full resources usually asking for the first 2 or 11
    bytes, probably to peek at magic numbers to figure early whether a
    potentially large resource may not be supported (read: video).
    
    If the user agent also advertises gzip support, and the transaction is
    known beforehand to not be cacheable, varnishd will forward the Range
    header to the backend:
    
        Accept-Encoding: gzip (when http_gzip_support is on)
        Range: bytes=0-1
    
    If the response happens to be both encoded and partial, the gunzip test
    cannot be performed. Otherwise we systematically end up with a broken
    transaction closed prematuraly:
    
        FetchError b tGunzip failed
        Gzip b u F - 2 0 0 0 0
    
    Refs #2530
    Refs #2554

diff --git a/bin/varnishd/cache/cache_fetch.c b/bin/varnishd/cache/cache_fetch.c
index 70f953f..b356828 100644
--- a/bin/varnishd/cache/cache_fetch.c
+++ b/bin/varnishd/cache/cache_fetch.c
@@ -546,6 +546,7 @@ static enum fetch_step
 vbf_stp_fetch(struct worker *wrk, struct busyobj *bo)
 {
 	const char *p;
+	unsigned is_partial;
 
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
 	CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC);
@@ -566,6 +567,8 @@ vbf_stp_fetch(struct worker *wrk, struct busyobj *bo)
 	 *
 	 */
 
+	is_partial = http_GetStatus(bo->beresp) == 206;
+
 	/* We do nothing unless the param is set */
 	if (!cache_param->http_gzip_support)
 		bo->do_gzip = bo->do_gunzip = 0;
@@ -608,7 +611,7 @@ vbf_stp_fetch(struct worker *wrk, struct busyobj *bo)
 			vbf_vfp_push(bo, &vfp_esi, 1);
 		} else if (bo->do_gzip) {
 			vbf_vfp_push(bo, &vfp_gzip, 1);
-		} else if (bo->is_gzip && !bo->do_gunzip) {
+		} else if (bo->is_gzip && !bo->do_gunzip && !is_partial) {
 			vbf_vfp_push(bo, &vfp_testgunzip, 1);
 		}
 	}
diff --git a/bin/varnishtest/tests/r02530.vtc b/bin/varnishtest/tests/r02530.vtc
new file mode 100644
index 0000000..c74391a
--- /dev/null
+++ b/bin/varnishtest/tests/r02530.vtc
@@ -0,0 +1,72 @@
+varnishtest "Don't test gunzip for partial responses"
+
+# The use of ETags is only here to ensure they aren't accidentally weakened.
+
+server s1 {
+	# pass'ed range request
+	rxreq
+	expect req.url == "/pass"
+	expect req.http.Accept-Encoding == gzip
+	expect req.http.Range == bytes=0-1
+	txresp -status 206 -nolen \
+		-hdr {ETag: "abc123"} \
+		-hdr "Accept-Ranges: bytes" \
+		-hdr "Content-Encoding: gzip" \
+		-hdr "Content-Length: 2" \
+		-hdr "Content-Range: bytes 0-1/*"
+	sendhex 1f8b
+
+	# unattended partial response
+	rxreq
+	expect req.url == "/miss"
+	expect req.http.Accept-Encoding == gzip
+	expect req.http.Range == <undef>
+	txresp -status 206 -nolen \
+		-hdr {ETag: "123abc"} \
+		-hdr "Accept-Ranges: bytes" \
+		-hdr "Content-Encoding: gzip" \
+		-hdr "Content-Length: 2" \
+		-hdr "Content-Range: bytes 0-1/*"
+	sendhex 1f8b
+} -start
+
+varnish v1 -vcl+backend {
+	sub vcl_recv {
+		if (req.url == "/pass") {
+			return (pass);
+		}
+	}
+} -start
+
+client c1 {
+	txreq -url "/pass" -hdr "Accept-Encoding: gzip" -hdr "Range: bytes=0-1"
+	rxresp
+	expect resp.status == 206
+	expect resp.http.Etag == {"abc123"}
+	expect resp.http.Accept-Ranges == bytes
+	expect resp.http.Content-Range ~ "^bytes 0-1/"
+	expect resp.http.Content-Length == 2
+	expect resp.bodylen == 2
+} -run
+
+varnish v1 -expect n_gzip == 0
+varnish v1 -expect n_gunzip == 0
+varnish v1 -expect SMA.s0.c_req == 0
+varnish v1 -expect SMA.Transient.c_req == 2
+
+# Invalid partial response, also in Transient
+client c1 {
+	txreq -url "/miss" -hdr "Accept-Encoding: gzip" -hdr "Range: bytes=0-1"
+	rxresp
+	expect resp.status == 206
+	expect resp.http.Etag == {"123abc"}
+	expect resp.http.Accept-Ranges == bytes
+	expect resp.http.Content-Range ~ "^bytes 0-1/"
+	expect resp.http.Content-Length == 2
+	expect resp.bodylen == 2
+} -run
+
+varnish v1 -expect n_gzip == 0
+varnish v1 -expect n_gunzip == 0
+varnish v1 -expect SMA.s0.c_req == 0
+varnish v1 -expect SMA.Transient.c_req == 4


More information about the varnish-commit mailing list