[master] 12b4809 Do not use an objcore flagged OC_F_PASS as a stale object

[Martin Blix Grydeland]

commit 12b480972e4da66ef734d8d0237c75141ec35d7d
Author: Martin Blix Grydeland <martin at varnish-software.com>
Date:   Fri May 27 14:30:18 2016 +0200

    Do not use an objcore flagged OC_F_PASS as a stale object
    
    If an object marked OF_IMSCAND is used as the template during backend
    IMS revalidation, and the new object also is marked OC_F_PASS
    (hit-for-pass) in vcl_backend_response, the resulting object would
    (though ObjCopyAttr) have both OF_IMSCAND and OC_F_PASS. The object
    could then be used again for backend IMS revalidation (since it has
    the OF_IMSCAND flag), which would cause troubles as the body would by
    this time be deleted.
    
    Fix by not considering objects marked OC_F_PASS as a stale object
    candidate during lookup.
    
    Fixes: #1956

diff --git a/bin/varnishd/cache/cache_fetch.c b/bin/varnishd/cache/cache_fetch.c
index 62c79ad..23d8d58 100644
--- a/bin/varnishd/cache/cache_fetch.c
+++ b/bin/varnishd/cache/cache_fetch.c
@@ -189,6 +189,7 @@ vbf_stp_mkbereq(struct worker *wrk, struct busyobj *bo)
 	if (bo->stale_oc != NULL &&
 	    ObjCheckFlag(bo->wrk, bo->stale_oc, OF_IMSCAND) &&
 	    (bo->stale_oc->boc != NULL || ObjGetLen(wrk, bo->stale_oc) != 0)) {
+		AZ(bo->stale_oc->flags & OC_F_PASS);
 		q = HTTP_GetHdrPack(bo->wrk, bo->stale_oc, H_Last_Modified);
 		if (q != NULL)
 			http_PrintfHeader(bo->bereq0,
diff --git a/bin/varnishd/cache/cache_req_fsm.c b/bin/varnishd/cache/cache_req_fsm.c
index f057e26..ef73ccd 100644
--- a/bin/varnishd/cache/cache_req_fsm.c
+++ b/bin/varnishd/cache/cache_req_fsm.c
@@ -404,8 +404,9 @@ cnt_lookup(struct worker *wrk, struct req *req)
 
 	if ((oc->flags & OC_F_PASS) && busy != NULL) {
 		/* Treat a graced Hit-For-Pass as a miss */
+		(void)HSH_DerefObjCore(wrk, &req->objcore);
+		AZ(req->objcore);
 		req->objcore = busy;
-		req->stale_oc = oc;
 		req->req_step = R_STP_MISS;
 		return (REQ_FSM_MORE);
 	} else if (oc->flags & OC_F_PASS) {
diff --git a/bin/varnishtest/tests/r01956.vtc b/bin/varnishtest/tests/r01956.vtc
new file mode 100644
index 0000000..9d2d505
--- /dev/null
+++ b/bin/varnishtest/tests/r01956.vtc
@@ -0,0 +1,53 @@
+varnishtest "#1956: graced hit-for-pass, backend ims and obj flags inheritance"
+
+server s1 {
+	rxreq
+	txresp -hdr {ETag: "foo"} -body "asdf"
+
+	rxreq
+	expect req.http.if-none-match == {"foo"}
+	txresp -status 304 -hdr {ETag: "bar"}
+
+	rxreq
+	expect req.http.if-none-match == "<undef>"
+	txresp -status 200 -hdr {ETag: "baz"} -body "asdf"
+} -start
+
+varnish v1 -vcl+backend {
+	sub vcl_backend_response {
+		set beresp.ttl = 0.1s;
+		set beresp.keep = 60s;
+		if (beresp.http.etag == {""bar""}) {
+			set beresp.grace = 60s;
+			set beresp.uncacheable = true;
+		} else {
+			set beresp.grace = 0s;
+		}
+		return (deliver);
+	}
+} -start
+
+client c1 {
+	timeout 5
+
+	txreq -hdr "cnt: 1"
+	rxresp
+	expect resp.http.etag == {"foo"}
+	expect resp.body == "asdf"
+
+	delay 0.2
+
+	txreq -hdr "cnt: 2"
+	rxresp
+	expect resp.status == 200
+	expect resp.http.etag == {"bar"}
+	expect resp.body == "asdf"
+
+	delay 0.2
+
+	txreq -hdr "cnt: 3"
+	rxresp
+	expect resp.status == 200
+	expect resp.http.etag == {"baz"}
+	expect resp.body == "asdf"
+} -run