[PATCH 3/3] Fix ved_deliver_byterange() and ESI_DeliverChild() when ESI-included objects span storage chunks

[Martin Blix Grydeland]

ved_deliver_byterange() would fail to return the correct next byte
when the gzipped ESI-included object spans storage chunks.

ESI_DeliverChild() would fail to extract the gzip trailer spanned
storage chunks. Introduce a general STV_memcpy() function to copy
bytes from objects.

These bugs are present in both trunk and 3.0

Fixes: #1109
---
 bin/varnishd/cache/cache.h             |    2 +
 bin/varnishd/cache/cache_esi_deliver.c |   33 +++++++++++++-----------
 bin/varnishd/storage/stevedore.c       |   37 ++++++++++++++++++++++++++
 bin/varnishtest/tests/r01109.vtc       |   44 ++++++++++++++++++++++++++++++++
 4 files changed, 101 insertions(+), 15 deletions(-)
 create mode 100644 bin/varnishtest/tests/r01109.vtc

diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index 073eb75..0548d1a 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -1020,6 +1020,8 @@ void STV_free(struct storage *st);
 void STV_open(void);
 void STV_close(void);
 void STV_Freestore(struct object *o);
+void STV_Memcpy(const struct object *obj, unsigned char *dest, ssize_t start,
+		ssize_t n);
 
 /* storage_synth.c */
 struct vsb *SMS_Makesynth(struct object *obj);
diff --git a/bin/varnishd/cache/cache_esi_deliver.c b/bin/varnishd/cache/cache_esi_deliver.c
index dd3d98a..53530c4 100644
--- a/bin/varnishd/cache/cache_esi_deliver.c
+++ b/bin/varnishd/cache/cache_esi_deliver.c
@@ -418,7 +418,7 @@ static uint8_t
 ved_deliver_byterange(const struct sess *sp, ssize_t low, ssize_t high)
 {
 	struct storage *st;
-	ssize_t l, lx;
+	ssize_t l, l2, lx;
 	u_char *p;
 
 //printf("BR %jd %jd\n", low, high);
@@ -440,15 +440,18 @@ ved_deliver_byterange(const struct sess *sp, ssize_t low, ssize_t high)
 			lx = low;
 		}
 //printf("[1-] %jd %jd\n", lx, lx + l);
-		if (lx + l >= high)
-			l = high - lx;
+		l2 = l;
+		if (lx + l2 > high)
+			l2 = 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)
-			return(p[l]);
-		lx += st->len;
+		assert(lx >= low && l2 <= l && lx + l2 <= high);
+		if (l2 != 0)
+			(void)WRW_Write(sp->wrk, p, l2);
+		if (l2 < l) {
+			assert(lx + l2 == high);
+			return(p[l2]);
+		}
+		lx += l;
 	}
 	INCOMPL();
 }
@@ -459,10 +462,11 @@ 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;
+	uint8_t buf[8];
 
 	if (!sp->req->obj->gziped) {
 		VTAILQ_FOREACH(st, &sp->req->obj->store, list)
@@ -542,12 +546,11 @@ ESI_DeliverChild(const struct sess *sp)
 	}
 	if (lpad > 0)
 		(void)WRW_Write(sp->wrk, dbits + 1, lpad);
-	st = VTAILQ_LAST(&sp->req->obj->store, storagehead);
-	assert(st->len > 8);
 
-	p = st->ptr + st->len - 8;
-	icrc = vle32dec(p);
-	ilen = vle32dec(p + 4);
+	assert(sp->req->obj->len > 8);
+	STV_Memcpy(sp->req->obj, buf, sp->req->obj->len - 8, 8);
+	icrc = vle32dec(buf);
+	ilen = vle32dec(buf + 4);
 	sp->req->crc = crc32_combine(sp->req->crc, icrc, ilen);
 	sp->req->l_crc += ilen;
 }
diff --git a/bin/varnishd/storage/stevedore.c b/bin/varnishd/storage/stevedore.c
index f1cbe94..98466b7 100644
--- a/bin/varnishd/storage/stevedore.c
+++ b/bin/varnishd/storage/stevedore.c
@@ -381,6 +381,43 @@ STV_Freestore(struct object *o)
 	}
 }
 
+void
+STV_Memcpy(const struct object *obj, unsigned char *dest, ssize_t start,
+	   ssize_t n)
+{
+	struct storage *st;
+	ssize_t l, lx;
+	u_char *p;
+
+	CHECK_OBJ_NOTNULL(obj, OBJECT_MAGIC);
+	AN(dest);
+	assert(n >= 0 && start + n <= obj->len);
+
+	if (n == 0)
+		return;
+
+	lx = 0;
+	VTAILQ_FOREACH(st, &obj->store, list) {
+		p = st->ptr;
+		l = st->len;
+		if (lx + l < start) {
+			lx += l;
+			continue;
+		}
+		if (lx < start) {
+			p += (start - lx);
+			l -= (start - lx);
+			lx = start;
+		}
+		if (lx + l > start + n)
+			l = start + n - lx;
+		assert(lx >= start && lx + l <= start + n);
+		memcpy(dest, p, l);
+		dest += l;
+		lx += l;
+	}
+}
+
 /*-------------------------------------------------------------------*/
 
 struct storage *
diff --git a/bin/varnishtest/tests/r01109.vtc b/bin/varnishtest/tests/r01109.vtc
new file mode 100644
index 0000000..1a5d61d
--- /dev/null
+++ b/bin/varnishtest/tests/r01109.vtc
@@ -0,0 +1,44 @@
+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
+
-- 
1.7.4.1