[master] c0fc574 VRY_Len() returns only the length of a single entry in the VRY spec, for conditional fetch-copying, we need the length of the full VRY-spec.

Poul-Henning Kamp phk at varnish-cache.org
Wed Oct 2 09:52:34 CEST 2013


commit c0fc574a530b7a66060e80b22bd91de8e2eb5b32
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date:   Wed Oct 2 07:51:52 2013 +0000

    VRY_Len() returns only the length of a single entry in the VRY spec,
    for conditional fetch-copying, we need the length of the full VRY-spec.
    
    Fixes:	#1350

diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index 7647e9c..a7ac6aa 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -1125,11 +1125,10 @@ void VSL_Flush(struct vsl_log *, int overflow);
 /* cache_vary.c */
 int VRY_Create(struct busyobj *bo, struct vsb **psb);
 int VRY_Match(struct req *, const uint8_t *vary);
-void VRY_Validate(const uint8_t *vary);
+unsigned VRY_Validate(const uint8_t *vary);
 void VRY_Prep(struct req *);
 enum vry_finish_flag { KEEP, DISCARD };
 void VRY_Finish(struct req *req, enum vry_finish_flag);
-unsigned VRY_Len(const uint8_t *);
 
 /* cache_vcl.c */
 void VCL_Init(void);
diff --git a/bin/varnishd/cache/cache_fetch.c b/bin/varnishd/cache/cache_fetch.c
index 097a04b..24c6a6a 100644
--- a/bin/varnishd/cache/cache_fetch.c
+++ b/bin/varnishd/cache/cache_fetch.c
@@ -393,7 +393,7 @@ vbf_stp_fetch(struct worker *wrk, struct busyobj *bo)
 		obj->vary = (void *)WS_Copy(obj->http->ws,
 		    VSB_data(vary), varyl);
 		AN(obj->vary);
-		VRY_Validate(obj->vary);
+		(void)VRY_Validate(obj->vary);
 		VSB_delete(vary);
 	}
 
@@ -469,15 +469,18 @@ vbf_stp_condfetch(struct worker *wrk, struct busyobj *bo)
 	struct object *obj;
 	struct objiter *oi;
 	void *sp;
-	ssize_t sl, al, tl;
+	ssize_t sl, al, tl, vl;
 	struct storage *st;
 
 	CHECK_OBJ_NOTNULL(wrk, WORKER_MAGIC);
 	CHECK_OBJ_NOTNULL(bo, BUSYOBJ_MAGIC);
 
 	l = 0;
-	if (bo->ims_obj->vary != NULL)
-		l += VRY_Len(bo->ims_obj->vary);
+	if (bo->ims_obj->vary != NULL) {
+		vl = VRY_Validate(bo->ims_obj->vary);
+		l += vl;
+	} else
+		vl = 0;
 	l += http_EstimateWS(bo->ims_obj->http, 0, &nhttp);
 
 	bo->stats = &wrk->stats;
@@ -499,9 +502,11 @@ vbf_stp_condfetch(struct worker *wrk, struct busyobj *bo)
 
 	/* XXX: ESI */
 
-	if (bo->ims_obj->vary != NULL)
+	if (bo->ims_obj->vary != NULL) {
 		obj->vary = (void *)WS_Copy(obj->http->ws,
-		    bo->ims_obj->vary, VRY_Len(bo->ims_obj->vary));
+		    bo->ims_obj->vary, vl);
+		assert(vl == VRY_Validate(obj->vary));
+	}
 
 	obj->vxid = bo->vsl->wid;
 
diff --git a/bin/varnishd/cache/cache_vary.c b/bin/varnishd/cache/cache_vary.c
index 363e357..bfc8686 100644
--- a/bin/varnishd/cache/cache_vary.c
+++ b/bin/varnishd/cache/cache_vary.c
@@ -173,7 +173,7 @@ VRY_Create(struct busyobj *bo, struct vsb **psb)
 /*
  * Find length of a vary entry
  */
-unsigned
+static unsigned
 VRY_Len(const uint8_t *p)
 {
 	unsigned l = vbe16dec(p);
@@ -247,12 +247,12 @@ VRY_Finish(struct req *req, enum vry_finish_flag flg)
 {
 	uint8_t *p = NULL;
 
-	VRY_Validate(req->vary_b);
+	(void)VRY_Validate(req->vary_b);
 	if (flg == KEEP && req->vary_l != NULL) {
 		p = malloc(req->vary_l - req->vary_b);
 		if (p != NULL) {
 			memcpy(p, req->vary_b, req->vary_l - req->vary_b);
-			VRY_Validate(p);
+			(void)VRY_Validate(p);
 		}
 	}
 	WS_Release(req->ws, 0);
@@ -323,7 +323,7 @@ VRY_Match(struct req *req, const uint8_t *vary)
 			vsp[ln + 0] = 0xff;
 			vsp[ln + 1] = 0xff;
 			vsp[ln + 2] = 0;
-			VRY_Validate(vsp);
+			(void)VRY_Validate(vsp);
 			req->vary_l = vsp + ln + 3;
 
 			i = vry_cmp(vary, vsp);
@@ -352,12 +352,19 @@ VRY_Match(struct req *req, const uint8_t *vary)
 	}
 }
 
-void
+/*
+ * Check the validity of a Vary string and return its total length
+ */
+
+unsigned
 VRY_Validate(const uint8_t *vary)
 {
+	unsigned retval = 0;
 
 	while (vary[2] != 0) {
 		assert(strlen((const char*)vary+3) == vary[2]);
+		retval += VRY_Len(vary);
 		vary += VRY_Len(vary);
 	}
+	return (retval + 3);
 }
diff --git a/bin/varnishtest/tests/r01350.vtc b/bin/varnishtest/tests/r01350.vtc
new file mode 100644
index 0000000..ae3cfbe
--- /dev/null
+++ b/bin/varnishtest/tests/r01350.vtc
@@ -0,0 +1,31 @@
+varnishtest "IMS + Vary panic"
+
+server s1 {
+	rxreq
+	txresp -hdr "Vary: Accept-Encoding" -hdr "Last-Modified: Wed, 10 May 2006 07:22:58 GMT" -body "IMS"
+	rxreq
+	txresp -status 304 -body ""
+} -start
+
+varnish v1 -vcl+backend {
+	sub vcl_backend_response {
+		set beresp.ttl = 2s;
+		set beresp.grace = 1m;
+		set beresp.keep = 1m;
+		return(deliver);
+	}
+} -start
+
+client c1 {
+	txreq
+	rxresp
+	expect resp.status == 200
+delay 3
+	txreq
+	rxresp
+	expect resp.status == 200
+delay 1
+	txreq
+	rxresp
+	expect resp.status == 200
+} -run



More information about the varnish-commit mailing list