From f8873ec0288ffdcc6136b12e8ab7f6d2b390fd63 Mon Sep 17 00:00:00 2001
From: "Federico G. Schwindt" <fgsch@lodoss.net>
Date: Wed, 9 Dec 2015 11:35:57 +0000
Subject: [PATCH] Use a weak comparison function for If-None-Match

As per RFC. Reorganise some of the tests to avoid duplicates.

Fixes #1816.
---
 bin/varnishd/cache/cache_rfc2616.c | 33 +++++++++++++++--------
 bin/varnishtest/tests/c00025.vtc   | 54 +++++++++++++++++++++++++++++---------
 bin/varnishtest/tests/c00026.vtc   | 38 +++++++--------------------
 3 files changed, 74 insertions(+), 51 deletions(-)

diff --git a/bin/varnishd/cache/cache_rfc2616.c b/bin/varnishd/cache/cache_rfc2616.c
index 7608755..d3fa8a2 100644
--- a/bin/varnishd/cache/cache_rfc2616.c
+++ b/bin/varnishd/cache/cache_rfc2616.c
@@ -232,6 +232,16 @@ RFC2616_Req_Gzip(const struct http *hp)
 
 /*--------------------------------------------------------------------*/
 
+static inline int
+rfc2616_weak_compare(const char *p, const char *e)
+{
+	if (p[0] == 'W' && p[1] == '/')
+		p += 2;
+	if (e[0] == 'W' && e[1] == '/')
+		e += 2;
+	return (strcmp(p, e) != 0);
+}
+
 int
 RFC2616_Do_Cond(const struct req *req)
 {
@@ -239,12 +249,20 @@ RFC2616_Do_Cond(const struct req *req)
 	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*/
+	/*
+	 * RFC 2616 13.3.4 states we need to match both ETag and
+	 * If-Modified-Since if present.
+	 */
+	if (http_GetHdr(req->http, H_If_None_Match, &p) &&
+	    http_GetHdr(req->resp, H_ETag, &e)) {
+		if (rfc2616_weak_compare(p, e))
+			return (0);
+		do_cond = 1;
+	}
 
-	if (http_GetHdr(req->http, H_If_Modified_Since, &p) ) {
+	if (http_GetHdr(req->http, H_If_Modified_Since, &p)) {
 		ims = VTIM_parse(p);
-		if (ims > req->t_req)	/* [RFC2616 14.25] */
+		if (!ims || ims > req->t_req)	/* [RFC2616 14.25] */
 			return (0);
 		AZ(ObjGetDouble(req->wrk, req->objcore, OA_LASTMODIFIED, &lm));
 		if (lm > ims)
@@ -252,13 +270,6 @@ RFC2616_Do_Cond(const struct req *req)
 		do_cond = 1;
 	}
 
-	if (http_GetHdr(req->http, H_If_None_Match, &p) &&
-	    http_GetHdr(req->resp, H_ETag, &e)) {
-		if (strcmp(p,e) != 0)
-			return (0);
-		do_cond = 1;
-	}
-
 	return (do_cond);
 }
 
diff --git a/bin/varnishtest/tests/c00025.vtc b/bin/varnishtest/tests/c00025.vtc
index ea0866a..25e3608 100644
--- a/bin/varnishtest/tests/c00025.vtc
+++ b/bin/varnishtest/tests/c00025.vtc
@@ -2,30 +2,60 @@ varnishtest "Test If-None-Match"
 
 server s1 {
 	rxreq
-	expect req.url == "/foo"
-	txresp -hdr {ETag: "123456789"} \
-	    -body "11111\n"
+	expect req.url == /
+	txresp -hdr {ETag: "123456789"} -bodylen 10
+
+	rxreq
+	expect req.url == /other
+	txresp -hdr {ETag: W/"123456789"} -bodylen 10
 } -start
 
 varnish v1 -vcl+backend { } -start
 
 client c1 {
-	txreq -url "/foo"
+	txreq
+	rxresp
+	expect resp.status == 200
+	expect resp.bodylen == 10
+	expect resp.http.etag == {"123456789"}
+
+	txreq -hdr {If-None-Match: "12345678"}
 	rxresp
 	expect resp.status == 200
-	expect resp.bodylen == 6
 
-	txreq -url "/foo" \
-	    -hdr {If-None-Match: "12345678"}
+	txreq -hdr {If-None-Match: W/"12345678"}
 	rxresp
 	expect resp.status == 200
 
-	txreq -url "/foo" \
-	    -hdr {If-None-Match: "123456789"}
+	txreq -hdr {If-None-Match: "123456789"}
 	rxresp -no_obj
 	expect resp.status == 304
-}
 
-client c1 -run
+	txreq -hdr {If-None-Match: W/"123456789"}
+	rxresp -no_obj
+	expect resp.status == 304
+} -run
 
-client c1 -run
+client c2 {
+	txreq -url /other
+	rxresp
+	expect resp.status == 200
+	expect resp.bodylen == 10
+	expect resp.http.etag == {W/"123456789"}
+
+	txreq -url /other -hdr {If-None-Match: "12345678"}
+	rxresp
+	expect resp.status == 200
+
+	txreq -url /other -hdr {If-None-Match: W/"12345678"}
+	rxresp
+	expect resp.status == 200
+
+	txreq -url /other -hdr {If-None-Match: "123456789"}
+	rxresp -no_obj
+	expect resp.status == 304
+
+	txreq -url /other -hdr {If-None-Match: W/"123456789"}
+	rxresp -no_obj
+	expect resp.status == 304
+} -run
diff --git a/bin/varnishtest/tests/c00026.vtc b/bin/varnishtest/tests/c00026.vtc
index d528c0b..2f92867 100644
--- a/bin/varnishtest/tests/c00026.vtc
+++ b/bin/varnishtest/tests/c00026.vtc
@@ -1,51 +1,33 @@
-varnishtest "Client IMS/INM: Test Combination of If-None-Match and If-Modified-Since"
+varnishtest "Test client If-None-Match and If-Modified-Since together"
 
 server s1 {
 	rxreq
-	expect req.url == "/foo"
 	txresp -hdr {ETag: "123456789"} \
-	       -hdr "Last-Modified: Thu, 26 Jun 2008 12:00:01 GMT" \
-	    -body "11111\n"
+	    -hdr "Last-Modified: Thu, 26 Jun 2008 12:00:01 GMT" \
+	    -bodylen 10
 } -start
 
 varnish v1 -vcl+backend { } -start
 
 client c1 {
-	txreq -url "/foo"
+	txreq
 	rxresp
 	expect resp.status == 200
-	expect resp.bodylen == 6
+	expect resp.bodylen == 10
+	expect resp.http.etag == {"123456789"}
 
-	txreq -url "/foo" \
-	    -hdr {If-None-Match: "123456789"}
-	rxresp -no_obj
-	expect resp.status == 304
-
-	txreq -url "/foo" \
-	    -hdr "If-Modified-Since: Thu, 26 Jun 2008 12:00:01 GMT"
-	rxresp -no_obj
-	expect resp.status == 304
-
-	txreq -url "/foo" \
-	    -hdr "If-Modified-Since: Thu, 26 Jun 2008 12:00:00 GMT" \
+	txreq -hdr "If-Modified-Since: Thu, 26 Jun 2008 12:00:00 GMT" \
 	    -hdr {If-None-Match: "123456789"}
 	rxresp
 	expect resp.status == 200
 
-	txreq -url "/foo" \
-	    -hdr "If-Modified-Since: Thu, 26 Jun 2008 12:00:01 GMT" \
+	txreq -hdr "If-Modified-Since: Thu, 26 Jun 2008 12:00:01 GMT" \
 	    -hdr {If-None-Match: "12345678"}
 	rxresp
 	expect resp.status == 200
 
-	txreq -url "/foo" \
-	    -hdr "If-Modified-Since: Thu, 26 Jun 2008 12:00:01 GMT" \
+	txreq -hdr "If-Modified-Since: Thu, 26 Jun 2008 12:00:01 GMT" \
 	    -hdr {If-None-Match: "123456789"}
 	rxresp -no_obj
 	expect resp.status == 304
-
-}
-
-client c1 -run
-
-client c1 -run
+} -run
-- 
2.6.2

