[4.0] 34d0fe4 Proper handling of duplicate headers on IMS headers merge

[Martin Blix Grydeland]

commit 34d0fe4a3761573d5eeef73592257fb87df14d36
Author: Pål Hermunn Johansen <hermunn at varnish-software.com>
Date:   Tue Mar 15 10:52:19 2016 +0100

    Proper handling of duplicate headers on IMS headers merge
    
    With this patch we properly handle duplicated headers when using http
    conditionals (dubbed backend IMS in Varnish). If the backend first
    sends the following:
    
    foo: a
    foo: b
    
    the backend forwards these as is. If then the objects grace period
    ends, but the object is not evicted from the cached, varnish will send
    a if-none-match header to the backend. The backend can then reply with
    a status 304 Not Modified. In this case the backend can send new 'foo'
    headers, or not. Varnish then has to react accordingly by either
    sending the original set of 'foo' headers, or the new set of 'foo'
    headers. The attached test checks both cases of backend behavior.
    
    Ref: #1879

diff --git a/bin/varnishd/cache/cache_http.c b/bin/varnishd/cache/cache_http.c
index 982af71..9276082 100644
--- a/bin/varnishd/cache/cache_http.c
+++ b/bin/varnishd/cache/cache_http.c
@@ -246,11 +246,12 @@ http_IsHdr(const txt *hh, const char *hdr)
 /*--------------------------------------------------------------------*/
 
 static unsigned
-http_findhdr(const struct http *hp, unsigned l, const char *hdr)
+http_findhdr_from(const struct http *hp, unsigned l, const char *hdr,
+    unsigned from)
 {
 	unsigned u;
 
-	for (u = HTTP_HDR_FIRST; u < hp->nhd; u++) {
+	for (u = from; u < hp->nhd; u++) {
 		Tcheck(hp->hd[u]);
 		if (hp->hd[u].e < hp->hd[u].b + l + 1)
 			continue;
@@ -263,6 +264,11 @@ http_findhdr(const struct http *hp, unsigned l, const char *hdr)
 	return (0);
 }
 
+static unsigned
+http_findhdr(const struct http *hp, unsigned l, const char *hdr) {
+	return (http_findhdr_from(hp, l, hdr, HTTP_HDR_FIRST));
+}
+
 /*--------------------------------------------------------------------
  * Count how many instances we have of this header
  */
@@ -808,8 +814,12 @@ http_Merge(const struct http *fm, struct http *to, int not_ce)
 		if (!p)
 			continue;
 		u = http_findhdr(fm, p - to->hd[v].b, to->hd[v].b);
-		if (u)
+		while (u) {
 			fm->hdf[u] &= ~HDF_MARKER;
+			u = http_findhdr_from(fm, p - to->hd[v].b,
+				to->hd[v].b, u + 1);
+		}
+
 	}
 	for (u = HTTP_HDR_FIRST; u < fm->nhd; u++)
 		if (fm->hdf[u] & HDF_MARKER)
diff --git a/bin/varnishtest/tests/r01879.vtc b/bin/varnishtest/tests/r01879.vtc
new file mode 100644
index 0000000..4c9a53d
--- /dev/null
+++ b/bin/varnishtest/tests/r01879.vtc
@@ -0,0 +1,47 @@
+varnishtest "r01879: Check duplicate headers handling on IMS header merge"
+
+server s1 {
+	rxreq
+	txresp -hdr {etag: "foo"} -hdr "foo: a" -hdr "foo: b"
+	rxreq
+	expect req.http.if-none-match == {"foo"}
+	txresp -status 304 -hdr {etag: "foo"}  -hdr "foo: c" -hdr "foo: d"
+	rxreq
+	txresp -hdr {etag: "bar"} -hdr "foo: a" -hdr "foo: b"
+	rxreq
+	expect req.http.if-none-match == {"bar"}
+	txresp -status 304 -hdr {etag: "bar"}
+} -start
+
+varnish v1 -vcl+backend {
+	import ${vmod_std};
+
+	sub vcl_backend_response {
+		set beresp.ttl = 0.00001s;
+		set beresp.grace = 0s;
+		set beresp.keep = 9999s;
+	}
+	sub vcl_deliver {
+		std.collect(resp.http.foo);
+	}
+} -start
+
+client c1 {
+	txreq
+	rxresp
+	expect resp.http.foo == "a, b"
+	delay .1
+	txreq
+	rxresp
+	expect resp.http.foo == "c, d"
+} -run
+
+client c2 {
+	txreq
+	rxresp
+	expect resp.http.foo == "a, b"
+	delay .1
+	txreq
+	rxresp
+	expect resp.http.foo == "a, b"
+} -run