martin at varnish-software
Apr 4, 2012, 2:31 AM
Post #1 of 1
(103 views)
Permalink
|
|
[PATCH] Fix ved_deliver_byterange() and ESI_DeliverChild() when ESI-included objects span storage chunks
|
|
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 when it
spans storage chunks.
These bugs are present in both trunk and 3.0.
Introduces a general STV_Memcpy() function to copy bytes from objects.
Fixes: #1109
---
bin/varnishd/cache/cache.h | 2 +
bin/varnishd/cache/cache_esi_deliver.c | 33 +++++++++++++-----------
bin/varnishd/storage/stevedore.c | 41 +++++++++++++++++++++++++++++
bin/varnishtest/tests/r01109.vtc | 44 ++++++++++++++++++++++++++++++++
4 files changed, 105 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..c8d83ff 100644
--- a/bin/varnishd/storage/stevedore.c
+++ b/bin/varnishd/storage/stevedore.c
@@ -381,6 +381,47 @@ 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 (l > n)
+ l = n;
+ assert(lx == start && l <= n);
+ memcpy(dest, p, l);
+ dest += l;
+ lx += l;
+ start += l;
+ n -= l;
+ if (n == 0)
+ break;
+ }
+}
+
/*-------------------------------------------------------------------*/
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
_______________________________________________
varnish-dev mailing list
varnish-dev [at] varnish-cache
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev
|
