[experimental-ims] b229950 Recent changes to the RFC2616 ttl calculation confused what ttl value was actually being used.
Geoff Simmons
geoff at varnish-cache.org
Wed Aug 31 16:04:38 CEST 2011
commit b22995084401716d63fac0882ef5b56725af0c49
Author: Poul-Henning Kamp <phk at FreeBSD.org>
Date: Fri Aug 26 07:20:41 2011 +0000
Recent changes to the RFC2616 ttl calculation confused what ttl
value was actually being used.
Fixed by: DocWilco
Fixes #984
diff --git a/bin/varnishd/cache.h b/bin/varnishd/cache.h
index 44e60ad..974ea3f 100644
--- a/bin/varnishd/cache.h
+++ b/bin/varnishd/cache.h
@@ -934,7 +934,7 @@ char *WS_Snapshot(struct ws *ws);
unsigned WS_Free(const struct ws *ws);
/* rfc2616.c */
-double RFC2616_Ttl(const struct sess *sp);
+void RFC2616_Ttl(const struct sess *sp);
enum body_status RFC2616_Body(const struct sess *sp);
unsigned RFC2616_Req_Gzip(const struct sess *sp);
int RFC2616_Do_Cond(const struct sess *sp);
diff --git a/bin/varnishd/cache_center.c b/bin/varnishd/cache_center.c
index 9c7f822..21731db 100644
--- a/bin/varnishd/cache_center.c
+++ b/bin/varnishd/cache_center.c
@@ -580,7 +580,7 @@ cnt_fetch(struct sess *sp)
*/
EXP_Clr(&sp->wrk->exp);
sp->wrk->exp.entered = TIM_real();
- sp->wrk->exp.ttl = RFC2616_Ttl(sp);
+ RFC2616_Ttl(sp);
/* pass from vclrecv{} has negative TTL */
if (sp->objcore == NULL)
diff --git a/bin/varnishd/rfc2616.c b/bin/varnishd/rfc2616.c
index eb01850..84e95f4 100644
--- a/bin/varnishd/rfc2616.c
+++ b/bin/varnishd/rfc2616.c
@@ -65,10 +65,9 @@
*
*/
-double
+void
RFC2616_Ttl(const struct sess *sp)
{
- double ttl;
unsigned max_age, age;
double h_date, h_expires;
char *p;
@@ -78,7 +77,7 @@ RFC2616_Ttl(const struct sess *sp)
assert(sp->wrk->exp.entered != 0.0 && !isnan(sp->wrk->exp.entered));
/* If all else fails, cache using default ttl */
- ttl = params->default_ttl;
+ sp->wrk->exp.ttl = params->default_ttl;
max_age = age = 0;
h_expires = 0;
@@ -126,9 +125,9 @@ RFC2616_Ttl(const struct sess *sp)
max_age = strtoul(p, NULL, 0);
if (age > max_age)
- ttl = 0;
+ sp->wrk->exp.ttl = 0;
else
- ttl = max_age - age;
+ sp->wrk->exp.ttl = max_age - age;
break;
}
@@ -139,7 +138,7 @@ RFC2616_Ttl(const struct sess *sp)
/* If backend told us it is expired already, don't cache. */
if (h_expires < h_date) {
- ttl = 0;
+ sp->wrk->exp.ttl = 0;
break;
}
@@ -151,9 +150,10 @@ RFC2616_Ttl(const struct sess *sp)
* trust Expires: relative to our own clock.
*/
if (h_expires < sp->wrk->exp.entered)
- ttl = 0;
+ sp->wrk->exp.ttl = 0;
else
- ttl = h_expires - sp->wrk->exp.entered;
+ sp->wrk->exp.ttl = h_expires -
+ sp->wrk->exp.entered;
break;
} else {
/*
@@ -161,7 +161,7 @@ RFC2616_Ttl(const struct sess *sp)
* derive a relative time from the two headers.
* (the negative ttl case is caught above)
*/
- ttl = (int)(h_expires - h_date);
+ sp->wrk->exp.ttl = (int)(h_expires - h_date);
}
}
@@ -169,10 +169,8 @@ RFC2616_Ttl(const struct sess *sp)
/* calculated TTL, Our time, Date, Expires, max-age, age */
WSP(sp, SLT_TTL,
"%u RFC %.0f %.0f %.0f %.0f %.0f %.0f %.0f %u",
- sp->xid, ttl, -1., -1., sp->wrk->exp.entered, sp->wrk->exp.age,
- h_date, h_expires, max_age);
-
- return (ttl);
+ sp->xid, sp->wrk->exp.ttl, -1., -1., sp->wrk->exp.entered,
+ sp->wrk->exp.age, h_date, h_expires, max_age);
}
/*--------------------------------------------------------------------
diff --git a/bin/varnishtest/tests/r00984.vtc b/bin/varnishtest/tests/r00984.vtc
new file mode 100644
index 0000000..d20334d
--- /dev/null
+++ b/bin/varnishtest/tests/r00984.vtc
@@ -0,0 +1,97 @@
+varnishtest "Status other than 200,203,300,301,302,307,410 and 404 should not be cached"
+
+server s1 {
+ rxreq
+ txresp -status 500
+ rxreq
+ txresp -status 200 -body "11"
+ rxreq
+ txresp -status 200 -body "ban"
+ rxreq
+ txresp -status 503
+ rxreq
+ txresp -status 200 -body "11"
+ rxreq
+ txresp -status 200 -body "ban"
+ rxreq
+ txresp -status 502
+ rxreq
+ txresp -status 200 -body "11"
+ rxreq
+ txresp -status 200 -body "ban"
+ rxreq
+ txresp -status 405
+ rxreq
+ txresp -status 200 -body "11"
+ rxreq
+ txresp -status 200 -body "ban"
+ rxreq
+ txresp -status 200 -body "11"
+} -start
+
+varnish v1 -arg "-t 300" -vcl+backend {
+ sub vcl_recv {
+ if (req.url == "/ban") {
+ ban("req.url ~ /");
+ }
+ }
+} -start
+
+client c1 {
+ txreq -url "/"
+ rxresp
+ expect resp.status == 500
+ txreq -url "/"
+ rxresp
+ expect resp.status == 200
+ expect resp.bodylen == 2
+ txreq -url "/ban"
+ rxresp
+ expect resp.status == 200
+ expect resp.bodylen == 3
+
+ txreq -url "/"
+ rxresp
+ expect resp.status == 503
+ txreq -url "/"
+ rxresp
+ expect resp.status == 200
+ expect resp.bodylen == 2
+ txreq -url "/ban"
+ rxresp
+ expect resp.status == 200
+ expect resp.bodylen == 3
+
+ txreq -url "/"
+ rxresp
+ expect resp.status == 502
+ txreq -url "/"
+ rxresp
+ expect resp.status == 200
+ expect resp.bodylen == 2
+ txreq -url "/ban"
+ rxresp
+ expect resp.status == 200
+ expect resp.bodylen == 3
+
+ txreq -url "/"
+ rxresp
+ expect resp.status == 405
+ txreq -url "/"
+ rxresp
+ expect resp.status == 200
+ expect resp.bodylen == 2
+ txreq -url "/ban"
+ rxresp
+ expect resp.status == 200
+ expect resp.bodylen == 3
+
+ txreq -url "/"
+ rxresp
+ expect resp.status == 200
+ expect resp.bodylen == 2
+ txreq -url "/"
+ rxresp
+ expect resp.status == 200
+ expect resp.bodylen == 2
+} -run
More information about the varnish-commit
mailing list