Ignore If-Modified-Since if we have If-None-Match

The latter takes precedence as per RFC 7232.  Also if we have an I-N-M
but no entity don't check I-M-S, just do a non-conditional request.

bin/varnishd/cache/cache_rfc2616.c

@@ -256,33 +256,31 @@ RFC2616_Do_Cond(const struct req *req)
 {
 	const char *p, *e;
 	double ims, lm;
-	int do_cond = 0;
 
 	/*
-	 * RFC 2616 13.3.4 states we need to match both ETag and
-	 * If-Modified-Since if present.
+	 * We MUST ignore If-Modified-Since if we have an If-None-Match
+	 * header [RFC7232 3.3 p16].
 	 */
-	if (http_GetHdr(req->http, H_If_None_Match, &p) &&
-	    http_GetHdr(req->resp, H_ETag, &e)) {
+	if (http_GetHdr(req->http, H_If_None_Match, &p)) {
+		if (!http_GetHdr(req->resp, H_ETag, &e))
+			return (0);
 		if (http_GetHdr(req->http, H_Range, NULL))
-			do_cond = rfc2616_strong_compare(p, e);
+			return (rfc2616_strong_compare(p, e));
 		else
-			do_cond = rfc2616_weak_compare(p, e);
-		if (!do_cond)
-			return (0);
+			return (rfc2616_weak_compare(p, e));
 	}
 
 	if (http_GetHdr(req->http, H_If_Modified_Since, &p)) {
 		ims = VTIM_parse(p);
-		if (!ims || ims > req->t_req)	/* [RFC2616 14.25] */
+		if (!ims || ims > req->t_req)	/* [RFC7232 3.3 p16] */
 			return (0);
 		AZ(ObjGetDouble(req->wrk, req->objcore, OA_LASTMODIFIED, &lm));
 		if (lm > ims)
 			return (0);
-		do_cond = 1;
+		return (1);
 	}
 
-	return (do_cond);
+	return (0);
 }
 
 /*--------------------------------------------------------------------*/

bin/varnishtest/tests/c00026.vtc

@@ -6,6 +6,11 @@ server s1 {
 	txresp -hdr {ETag: "123456789"} \
 	    -hdr "Last-Modified: Thu, 26 Jun 2008 12:00:01 GMT" \
 	    -bodylen 10
+
+	rxreq
+	expect req.url == /other
+	txresp -hdr "Last-Modified: Thu, 26 Jun 2008 12:00:01 GMT" \
+	    -bodylen 10
 } -start
 
 varnish v1 -vcl+backend { } -start
@@ -19,8 +24,8 @@ client c1 {
 
 	txreq -hdr "If-Modified-Since: Thu, 26 Jun 2008 12:00:00 GMT" \
 	    -hdr {If-None-Match: "123456789"}
-	rxresp
-	expect resp.status == 200
+	rxresp -no_obj
+	expect resp.status == 304
 
 	txreq -hdr "If-Modified-Since: Thu, 26 Jun 2008 12:00:01 GMT" \
 	    -hdr {If-None-Match: "12345678"}
@@ -31,4 +36,10 @@ client c1 {
 	    -hdr {If-None-Match: "123456789"}
 	rxresp -no_obj
 	expect resp.status == 304
+
+	txreq -url /other \
+	    -hdr "If-Modified-Since: Thu, 26 Jun 2008 12:00:01 GMT" \
+	    -hdr {If-None-Match: "123456789"}
+	rxresp
+	expect resp.status == 200
 } -run

bin/varnishtest/tests/r00907.vtc

-varnishtest "Ticket #907 200/304 handling with Etags + Last-Modified"
-
-server s1 {
-	rxreq
-	txresp \
-		-hdr {ETag: "saengei1Ohshicich4iteesu"} \
-		-hdr "Last-Modified: Tue, 20 Sep 2011 18:55:00 GMT"
-} -start
-
-varnish v1 -vcl+backend {
-	sub vcl_deliver {
-		set resp.http.x-timestamp = now;
-	}
-} -start
-
-client c1 {
-	txreq -hdr {If-None-Match: "saengei1Ohshicich4iteesu"}
-	rxresp -no_obj
-	expect resp.status == 304
-
-	txreq -hdr {If-None-Match: "saengei1Ohshicich4iteesu"} \
-	      -hdr "If-Modified-Since: Tue, 20 Sep 2011 18:54:59 GMT"
-	rxresp -no_obj
-	expect resp.status == 200
-
-	txreq -hdr {If-None-Match: "saengei1Ohshicich4iteesu"} \
-	      -hdr "If-Modified-Since: Tue, 20 Sep 2011 18:55:00 GMT"
-	rxresp -no_obj
-	expect resp.status == 304
-
-} -run