Recent changes to the RFC2616 ttl calculation confused what ttl

value was actually being used.

Fixed by:	DocWilco
Fixes		#984

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);

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)

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);
 }
 
 /*--------------------------------------------------------------------

bin/varnishtest/tests/r00984.vtc

+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