Hi,

I've been pondering on this for a while and I finally convinced myself that
Varnish is doing it wrong.  If http_gzip_support is enabled and the object
is gzip'd we must add a Vary on Accept-Encoding on delivery to keep
intermediary caches happy.

This is done before vcl_deliver is called so it's possible to remove it
before delivering although I can't see why anyone might want to do this.

Alternatively we could add a parameter, e.g. http_gzip_vary, to controls
this but I'm not in favour of adding more knobs. ymmv.

Comments? OKs?

FWIW, nginx support this via gzip_vary (1). It's also mentioned in Google's
Optimizing caching docs (2) and in maxcdn blog post (3).

f.-

1. http://nginx.org/en/docs/http/ngx_http_gzip_module.html#gzip_vary
2. https://developers.google.com/speed/docs/best-practices/caching
3. http://blog.maxcdn.com/accept-encoding-its-vary-important/
commit 9942e20c0dadced6bd38c621f2fa78ab3bc1529d
Author: Federico G. Schwindt <[email protected]>
Date:   Tue Aug 19 09:46:55 2014 +0100

    Add A-E to Vary when gzip support is enabled and the object is gzip'ed
    
    This ensures intermediary caches handle variants correctly.
---
 bin/varnishd/cache/cache_req_fsm.c | 14 ++++++++
 bin/varnishtest/tests/g00007.vtc   | 65 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 79 insertions(+)

diff --git a/bin/varnishd/cache/cache_req_fsm.c b/bin/varnishd/cache/cache_req_fsm.c
index 7ab29ac..83bd580 100644
--- a/bin/varnishd/cache/cache_req_fsm.c
+++ b/bin/varnishd/cache/cache_req_fsm.c
@@ -88,6 +88,7 @@ static enum req_fsm_nxt
 cnt_deliver(struct worker *wrk, struct req *req)
 {
 	struct busyobj *bo;
+	char *vary;
 
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
 	CHECK_OBJ_NOTNULL(req, REQ_MAGIC);
@@ -131,6 +132,19 @@ cnt_deliver(struct worker *wrk, struct req *req)
 	    !RFC2616_Req_Gzip(req->http))
 		RFC2616_Weaken_Etag(req->resp);
 
+	if (cache_param->http_gzip_support &&
+	    ObjCheckFlag(req->objcore, &req->wrk->stats, OF_GZIPED)) {
+		if (http_GetHdrData(req->resp, H_Vary, "Accept-Encoding",
+		    NULL))
+			;
+		else if (http_GetHdr(req->resp, H_Vary, &vary)) {
+			http_Unset(req->resp, H_Vary);
+			http_PrintfHeader(req->resp,
+			    "Vary: %s, Accept-Encoding", vary);
+		} else
+			http_SetHeader(req->resp, "Vary: Accept-Encoding");
+	}
+
 	VCL_deliver_method(req->vcl, wrk, req, NULL, req->http->ws);
 	VSLb_ts_req(req, "Process", W_TIM_real(wrk));
 
diff --git a/bin/varnishtest/tests/g00007.vtc b/bin/varnishtest/tests/g00007.vtc
new file mode 100644
index 0000000..788bc89
--- /dev/null
+++ b/bin/varnishtest/tests/g00007.vtc
@@ -0,0 +1,65 @@
+varnishtest "Test Vary with gzip/gunzip"
+
+server s1 {
+	rxreq
+	txresp -body "foo"
+	rxreq
+	txresp -gzipbody "bar"
+	rxreq
+	txresp -body "baz"
+	rxreq
+	txresp -hdr "Vary: foo" -gzipbody "qux"
+	rxreq
+	txresp -hdr "Vary: Accept-Encoding" -gzipbody "foobar"
+} -start
+
+varnish v1 -vcl+backend {
+	sub vcl_backend_response {
+		if (bereq.url ~ "baz") {
+			set beresp.do_gzip = true;
+		}
+	}
+} -start
+
+client c1 {
+	txreq -url /foo
+	rxresp
+	expect resp.body == "foo"
+	expect resp.http.Vary == <undef>
+	txreq -url /foo -hdr "Accept-Encoding: gzip"
+	rxresp
+	expect resp.body == "foo"
+	expect resp.http.Vary == <undef>
+	txreq -url /bar
+	rxresp
+	expect resp.body == "bar"
+	expect resp.http.Vary == "Accept-Encoding"
+	txreq -url /bar -hdr "Accept-Encoding: gzip"
+	rxresp
+	expect resp.bodylen == 26
+	expect resp.http.Vary == "Accept-Encoding"
+	txreq -url /baz
+	rxresp
+	expect resp.body == "baz"
+	expect resp.http.Vary == "Accept-Encoding"
+	txreq -url /baz -hdr "Accept-Encoding: gzip"
+	rxresp
+	expect resp.bodylen == 23
+	expect resp.http.Vary == "Accept-Encoding"
+	txreq -url /qux
+	rxresp
+	expect resp.body == "qux"
+	expect resp.http.Vary == "foo, Accept-Encoding"
+	txreq -url /qux -hdr "Accept-Encoding: gzip"
+	rxresp
+	expect resp.bodylen == 26
+	expect resp.http.Vary == "foo, Accept-Encoding"
+	txreq -url /foobar
+	rxresp
+	expect resp.body == "foobar"
+	expect resp.http.Vary == "Accept-Encoding"
+	txreq -url /foobar -hdr "Accept-Encoding: gzip"
+	rxresp
+	expect resp.bodylen == 29
+	expect resp.http.Vary == "Accept-Encoding"
+} -run
_______________________________________________
varnish-dev mailing list
[email protected]
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

Reply via email to