Fix for #1109

I've opted for a less intrusive fix that Martin proposed, but my fix is largely a rework of his suggested patch. Testcase by: Martin

Fix for #1109
I've opted for a less intrusive fix that Martin proposed, but my fix is largely a rework of his suggested patch. Testcase by: Martin
e2d35b4a · Poul-Henning Kamp · Tollef Fog Heen · 20396aa7 · e2d35b4a · e2d35b4a
Commit e2d35b4a authored Apr 10, 2012 by Poul-Henning Kamp Committed by Tollef Fog Heen May 24, 2012
Hide whitespace changes
Inline Side-by-side

Showing with 61 additions and 11 deletions

cache_esi_deliver.c bin/varnishd/cache_esi_deliver.c +18 -11

r01109.vtc bin/varnishtest/tests/r01109.vtc +43 -0

No files found.
--- a/bin/varnishd/cache_esi_deliver.c
+++ b/bin/varnishd/cache_esi_deliver.c
@@ -440,12 +440,10 @@ ved_deliver_byterange(const struct sess *sp, ssize_t low, ssize_t high)
 	ssize_t l, lx;
 	u_char *p;

-//printf("BR %jd %jd\n", low, high);
 	lx = 0;
 	VTAILQ_FOREACH(st, &sp->obj->store, list) {
 		p = st->ptr;
 		l = st->len;
-//printf("[0-] %jd %jd\n", lx, lx + l);
 		if (lx + l < low) {
 			lx += l;
 			continue;
@@ -458,16 +456,14 @@ ved_deliver_byterange(const struct sess *sp, ssize_t low, ssize_t high)
 			l -= (low - lx);
 			lx = low;
 		}
-//printf("[1-] %jd %jd\n", lx, lx + l);
 		if (lx + l >= high)
 			l = high - lx;
-//printf("[2-] %jd %jd\n", lx, lx + l);
 		assert(lx >= low && lx + l <= high);
 		if (l != 0)
 			(void)WRW_Write(sp->wrk, p, l);
-		if (lx + st->len > high)
+		if (p + l < st->ptr + st->len)
 			return(p[l]);
-		lx += st->len;
+		lx += l;
 	}
 	INCOMPL();
 }
@@ -478,10 +474,12 @@ ESI_DeliverChild(const struct sess *sp)
 	struct storage *st;
 	struct object *obj;
 	ssize_t start, last, stop, lpad;
-	u_char *p, cc;
+	u_char cc;
 	uint32_t icrc;
 	uint32_t ilen;
 	uint8_t *dbits;
+	int i, j;
+	uint8_t tailbuf[8];

 	if (!sp->obj->gziped) {
 		VTAILQ_FOREACH(st, &sp->obj->store, list)
@@ -561,12 +559,21 @@ ESI_DeliverChild(const struct sess *sp)
 	}
 	if (lpad > 0)
 		(void)WRW_Write(sp->wrk, dbits + 1, lpad);
+
+	/* We need the entire tail, but it may not be in one storage segment */
 	st = VTAILQ_LAST(&sp->obj->store, storagehead);
-	assert(st->len > 8);
+	for (i = sizeof tailbuf; i > 0; i -= j) {
+		j = st->len;
+		if (j > i)
+			j = i;
+		memcpy(tailbuf + i - j, st->ptr + st->len - j, j);
+		st = VTAILQ_PREV(st, storagehead, list);
+		assert(i == j || st != NULL);
+	}
+
+	icrc = vle32dec(tailbuf);
+	ilen = vle32dec(tailbuf + 4);

-	p = st->ptr + st->len - 8;
-	icrc = vle32dec(p);
-	ilen = vle32dec(p + 4);
 	sp->wrk->crc = crc32_combine(sp->wrk->crc, icrc, ilen);
 	sp->wrk->l_crc += ilen;
 }
--- a/bin/varnishtest/tests/r01109.vtc
+++ b/bin/varnishtest/tests/r01109.vtc
+varnishtest "Test case for #1109 - Gzip+ESI broken for large included objects"
+
+server s1 {
+	rxreq
+	expect req.url == "/test1"
+	txresp -body {<html>start<esi:include src="/include1"/>end}
+	rxreq
+	expect req.url == "/include1"
+	# This tests ESI+gzip delivery when the ESI-included object
+	# has more than one storage chunk
+	txresp -bodylen 4082
+
+	rxreq
+	txresp -body {<html>start<esi:include src="/include2"/>end}
+	expect req.url == "/test2"
+
+	rxreq
+	expect req.url == "/include2"
+	# This tests gzip trailer extraction for ESI+gzip CRC calculation
+	# when the trailer spans two storage chunks
+	txresp -bodylen 4074
+} -start
+
+varnish v1 -arg "-pfetch_chunksize=4k" -arg "-pgzip_level=0" -vcl+backend {
+	sub vcl_fetch {
+		if (req.url ~ "/test") {
+			set beresp.do_esi = true;
+		}
+		set beresp.do_gzip = true;
+	}
+} -start
+
+client c1 {
+	txreq -url "/test1" -hdr "Accept-Encoding: gzip"
+	rxresp
+	gunzip
+	expect resp.bodylen == 4096
+
+	txreq -url "/test2" -hdr "Accept-Encoding: gzip"
+	rxresp
+	gunzip
+	expect resp.bodylen == 4088
+} -run