[master] 42baff6 Don't merge Content-Encoding from IMS object before VCL, in order to not confuse VCL by "backend" suddenly changing gzip status, but do unconditionally smash it in there afterwards, no matter what VCL does, so it will be correct relative to the existing body.

Poul-Henning Kamp phk at FreeBSD.org
Mon Mar 10 10:23:10 CET 2014


commit 42baff6df2736e86fb7c73a90686b83756575dd1
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Mon Mar 10 09:21:50 2014 +0000

    Don't merge Content-Encoding from IMS object before VCL, in order
    to not confuse VCL by "backend" suddenly changing gzip status, but do
    unconditionally smash it in there afterwards, no matter what VCL
    does, so it will be correct relative to the existing body.

diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index f18c69a..5b4028d 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -594,8 +594,9 @@ struct object {
 
 	uint8_t			*vary;
 
-	/* XXX: make bitmap */
-	uint8_t			gziped;
+	unsigned		gziped:1;
+	unsigned		changed_gzip:1;
+
 	/* Bit positions in the gzip stream */
 	ssize_t			gzip_start;
 	ssize_t			gzip_last;
@@ -998,7 +999,7 @@ void http_CopyHome(const struct http *hp);
 void http_Unset(struct http *hp, const char *hdr);
 void http_CollectHdr(struct http *hp, const char *hdr);
 void http_VSL_log(const struct http *hp);
-void http_Merge(const struct http *fm, struct http *to);
+void http_Merge(const struct http *fm, struct http *to, int not_ce);
 
 /* cache_http1_proto.c */
 
diff --git a/bin/varnishd/cache/cache_fetch.c b/bin/varnishd/cache/cache_fetch.c
index 27a2df8..b42674a 100644
--- a/bin/varnishd/cache/cache_fetch.c
+++ b/bin/varnishd/cache/cache_fetch.c
@@ -336,7 +336,8 @@ vbf_stp_startfetch(struct worker *wrk, struct busyobj *bo)
 
 	if (bo->ims_obj != NULL && bo->beresp->status == 304) {
 		bo->beresp->status = 200;
-		http_Merge(bo->ims_obj->http, bo->beresp);
+		http_Merge(bo->ims_obj->http, bo->beresp,
+		    bo->ims_obj->changed_gzip);
 		do_ims = 1;
 	} else
 		do_ims = 0;
@@ -464,6 +465,9 @@ vbf_stp_fetch(struct worker *wrk, struct busyobj *bo)
 	if (bo->do_gzip || (bo->is_gzip && !bo->do_gunzip))
 		obj->gziped = 1;
 
+	if (bo->do_gzip || bo->do_gunzip)
+		obj->changed_gzip = 1;
+
 	/*
 	 * Ready to fetch the body
 	 */
@@ -554,10 +558,22 @@ vbf_stp_condfetch(struct worker *wrk, struct busyobj *bo)
 	ssize_t sl, al, tl;
 	struct storage *st;
 	enum objiter_status ois;
+	char *p;
 
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
 	CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC);
 
+	if (bo->ims_obj->changed_gzip) {
+		/*
+		 * If we modified the gzip status of the IMS object, that
+		 * must control the C-E header, if any.
+		 */
+		http_Unset(bo->beresp, H_Content_Encoding);
+		if (http_GetHdr(bo->ims_obj->http, H_Content_Encoding, &p))
+			http_PrintfHeader(bo->beresp,
+			    "Content-Encoding: %s", p);
+	}
+
 	AZ(vbf_beresp2obj(bo));
 	obj = bo->fetch_obj;
 
diff --git a/bin/varnishd/cache/cache_http.c b/bin/varnishd/cache/cache_http.c
index 2a9b8de..6898987 100644
--- a/bin/varnishd/cache/cache_http.c
+++ b/bin/varnishd/cache/cache_http.c
@@ -641,13 +641,19 @@ http_FilterResp(const struct http *fm, struct http *to, unsigned how)
  */
 
 void
-http_Merge(const struct http *fm, struct http *to)
+http_Merge(const struct http *fm, struct http *to, int not_ce)
 {
 	unsigned u, v;
 	const char *p;
 
 	for (u = HTTP_HDR_FIRST; u < fm->nhd; u++)
 		fm->hdf[u] |= HDF_MARKER;
+	if (not_ce) {
+		u = http_findhdr(fm,
+		    H_Content_Encoding[0] - 1, H_Content_Encoding + 1);
+		if (u > 0)
+			fm->hdf[u] &= ~HDF_MARKER;
+	}
 	for (v = HTTP_HDR_FIRST; v < to->nhd; v++) {
 		p = strchr(to->hd[v].b, ':');
 		AN(p);
diff --git a/bin/varnishtest/tests/g00006.vtc b/bin/varnishtest/tests/g00006.vtc
new file mode 100644
index 0000000..740d4bd
--- /dev/null
+++ b/bin/varnishtest/tests/g00006.vtc
@@ -0,0 +1,72 @@
+varnishtest "IMS'ing g[un]zip'ed objects"
+
+server s1 {
+	rxreq
+	expect req.url == /1
+	txresp -hdr  "Last-Modified: Wed, 11 Sep 2013 13:36:55 GMT" -bodylen 20
+
+	rxreq
+	expect req.url == /1
+	expect req.http.if-modified-since == "Wed, 11 Sep 2013 13:36:55 GMT"
+	txresp -status 304 -nolen
+
+	rxreq
+	expect req.url == /2
+	txresp -hdr  "Last-Modified: Wed, 11 Sep 2013 13:36:55 GMT" -gzipbody "012345678901234567"
+
+	rxreq
+	expect req.url == /2
+	expect req.http.if-modified-since == "Wed, 11 Sep 2013 13:36:55 GMT"
+	txresp -status 304 -hdr "Content-Encoding: gzip,rot13" -nolen
+
+} -start
+
+varnish v1 -vcl+backend {
+	sub vcl_backend_response {
+		set beresp.http.foobar = beresp.http.content-encoding;
+		if (bereq.url == "/1") {
+			set beresp.do_gzip = true;
+		} else {
+			set beresp.do_gunzip = true;
+		}
+		set beresp.ttl = 1s;
+		set beresp.grace = 0s;
+		set beresp.keep = 60s;
+	}
+} -start
+
+client c1 {
+	txreq -url /1 -hdr "Accept-Encoding: gzip"
+	rxresp
+	expect resp.http.content-encoding == "gzip"
+	expect resp.http.foobar == ""
+	gunzip
+	expect resp.bodylen == 20
+
+	delay 1
+
+	txreq -url /1 -hdr "Accept-Encoding: gzip"
+	rxresp
+	expect resp.http.content-encoding == "gzip"
+	expect resp.http.foobar == ""
+	gunzip
+	expect resp.bodylen == 20
+
+	delay .2
+
+	txreq -url /2
+	rxresp
+	expect resp.http.content-encoding == "<undef>"
+	expect resp.http.foobar == "gzip"
+	expect resp.bodylen == 18
+
+	delay 1
+
+	txreq -url /2
+	rxresp
+	expect resp.http.content-encoding == "<undef>"
+	# Here we see the C-E of the IMS OBJ
+	expect resp.http.foobar == "gzip,rot13"
+	expect resp.bodylen == 18
+
+} -run



More information about the varnish-commit mailing list