[master] 5384df7 Don't test gunzip for partial responses

[Dridi Boukelmoune]

commit 5384df74abd77a54d03727e847e8610c7af068a8
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 c31d88f..9faf4ee 100644
--- a/bin/varnishd/cache/cache_fetch.c
+++ b/bin/varnishd/cache/cache_fetch.c
@@ -501,6 +501,8 @@ vbf_stp_fetchbody(struct worker *wrk, struct busyobj *bo)
 static int
 vbf_figure_out_vfp(struct busyobj *bo)
 {
+	unsigned is_partial;
+
 	/*
 	 * The VCL variables beresp.do_g[un]zip tells us how we want the
 	 * object processed before it is stored.
@@ -514,7 +516,7 @@ vbf_figure_out_vfp(struct busyobj *bo)
 	 *
 	 */
 
-	/* No body or no GZIP supprt -> done */
+	/* No body or no GZIP support -> done */
 	if (bo->htc->body_status == BS_NONE ||
 	    bo->htc->content_length == 0 ||
 	    !cache_param->http_gzip_support) {
@@ -524,6 +526,7 @@ vbf_figure_out_vfp(struct busyobj *bo)
 		return (0);
 	}
 
+	is_partial = http_GetStatus(bo->beresp) == 206;
 	bo->is_gzip = http_HdrIs(bo->beresp, H_Content_Encoding, "gzip");
 	bo->is_gunzip = !http_GetHdr(bo->beresp, H_Content_Encoding, NULL);
 	assert(bo->is_gzip == 0 || bo->is_gunzip == 0);
@@ -552,7 +555,7 @@ vbf_figure_out_vfp(struct busyobj *bo)
 	if (bo->do_gzip)
 		return (VFP_Push(bo->vfc, &VFP_gzip) == NULL ? -1 : 0);
 
-	if (bo->is_gzip && !bo->do_gunzip)
+	if (bo->is_gzip && !bo->do_gunzip && !is_partial)
 		return (VFP_Push(bo->vfc, &VFP_testgunzip) == NULL ? -1 : 0);
 
 	return (0);
diff --git a/bin/varnishtest/tests/r02530.vtc b/bin/varnishtest/tests/r02530.vtc
new file mode 100644
index 0000000..27b24b2
--- /dev/null
+++ b/bin/varnishtest/tests/r02530.vtc
@@ -0,0 +1,74 @@
+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 n_test_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 n_test_gunzip == 0
+varnish v1 -expect SMA.s0.c_req == 0
+varnish v1 -expect SMA.Transient.c_req == 4