[master] 9210c4685 http2: Don't send resp connection-specific headers

Dridi Boukelmoune dridi.boukelmoune at gmail.com
Mon Oct 5 13:47:07 UTC 2020


commit 9210c4685f7b0ca9bb3ae5598f014843c8c43bd1
Author: Dridi Boukelmoune <dridi.boukelmoune at gmail.com>
Date:   Tue Sep 29 12:44:00 2020 +0200

    http2: Don't send resp connection-specific headers
    
    Some browsers are strict about this and simply drop responses containing
    such headers. Since this is not filtering a context switch between a
    client and a backend transaction (or cache hit) a new filtering flag is
    added to the HTTP headers table for connection-specific headers. This
    new flag cannot be compounded as HTTPH_R_FETCH|HTTPH_A_INS because the
    TE header is an exception and left alone, even though trailers aren't
    supported.
    
    Better diff with the --ignore-all-space option.
    
    We could go further and consider any client request containing one as
    malformed as mandated by RFC 7540.
    
    Closes #3416

diff --git a/bin/varnishd/cache/cache.h b/bin/varnishd/cache/cache.h
index 40fafc766..215ed8759 100644
--- a/bin/varnishd/cache/cache.h
+++ b/bin/varnishd/cache/cache.h
@@ -651,11 +651,13 @@ int HTTP_IterHdrPack(struct worker *, struct objcore *, const char **);
 	 for ((ptr) = NULL; HTTP_IterHdrPack(wrk, oc, &(ptr));)
 const char *HTTP_GetHdrPack(struct worker *, struct objcore *, const char *hdr);
 enum sess_close http_DoConnection(struct http *hp);
+int http_IsFiltered(const struct http *hp, unsigned u, unsigned how);
 
-#define HTTPH_R_PASS	(1 << 0)	/* Request (c->b) in pass mode */
-#define HTTPH_R_FETCH	(1 << 1)	/* Request (c->b) for fetch */
-#define HTTPH_A_INS	(1 << 2)	/* Response (b->o) for insert */
-#define HTTPH_A_PASS	(1 << 3)	/* Response (b->o) for pass */
+#define HTTPH_R_PASS		(1 << 0)	/* Request (c->b) in pass mode */
+#define HTTPH_R_FETCH		(1 << 1)	/* Request (c->b) for fetch */
+#define HTTPH_A_INS		(1 << 2)	/* Response (b->o) for insert */
+#define HTTPH_A_PASS		(1 << 3)	/* Response (b->o) for pass */
+#define HTTPH_C_SPECIFIC	(1 << 4)	/* Connection-specific */
 
 #define HTTPH(a, b, c) extern char b[];
 #include "tbl/http_headers.h"
diff --git a/bin/varnishd/cache/cache_http.c b/bin/varnishd/cache/cache_http.c
index 9fa4c1f64..6986860ce 100644
--- a/bin/varnishd/cache/cache_http.c
+++ b/bin/varnishd/cache/cache_http.c
@@ -904,6 +904,13 @@ http_isfiltered(const struct http *fm, unsigned u, unsigned how)
 	return (0);
 }
 
+int
+http_IsFiltered(const struct http *fm, unsigned u, unsigned how)
+{
+
+	return (http_isfiltered(fm, u, how));
+}
+
 /*--------------------------------------------------------------------
  * Estimate how much workspace we need to Filter this header according
  * to 'how'.
diff --git a/bin/varnishd/http2/cache_http2_deliver.c b/bin/varnishd/http2/cache_http2_deliver.c
index 367fd9bf9..c93145322 100644
--- a/bin/varnishd/http2/cache_http2_deliver.c
+++ b/bin/varnishd/http2/cache_http2_deliver.c
@@ -241,6 +241,9 @@ h2_build_headers(struct vsb *resp, struct req *req)
 		r = strchr(hp->hd[u].b, ':');
 		AN(r);
 
+		if (http_IsFiltered(hp, u, HTTPH_C_SPECIFIC))
+			continue; //rfc7540,l,2999,3006
+
 		hps = hp_idx[tolower(*hp->hd[u].b)];
 		sz = 1 + r - hp->hd[u].b;
 		assert(sz > 0);
@@ -295,6 +298,8 @@ h2_deliver(struct req *req, struct boc *boc, int sendbody)
 
 	VSLb(req->vsl, SLT_RespProtocol, "HTTP/2.0");
 
+	(void)http_DoConnection(req->resp);
+
 	ss = WS_Snapshot(req->ws);
 
 	WS_VSB_new(resp, req->ws);
diff --git a/bin/varnishtest/tests/r03416.vtc b/bin/varnishtest/tests/r03416.vtc
new file mode 100644
index 000000000..3d7431353
--- /dev/null
+++ b/bin/varnishtest/tests/r03416.vtc
@@ -0,0 +1,25 @@
+varnishtest "Filter hop-by-hop headers out of h2 responses"
+
+server s1 {
+	rxreq
+	txresp
+} -start
+
+varnish v1 -cliok "param.set feature +http2"
+varnish v1 -vcl+backend {
+	sub vcl_deliver {
+		set resp.http.Keep-Alive = "timeout=5, max=1000";
+		set resp.http.Connection = "other";
+		set resp.http.Other = "foo";
+	}
+} -start
+
+client c1 {
+	stream 1 {
+		txreq
+		rxresp
+		expect resp.http.keep-alive == <undef>
+		expect resp.http.connection == <undef>
+		expect resp.http.other == <undef>
+	} -run
+} -run
diff --git a/include/tbl/http_headers.h b/include/tbl/http_headers.h
index 324adcfc1..416d6a8f6 100644
--- a/include/tbl/http_headers.h
+++ b/include/tbl/http_headers.h
@@ -42,7 +42,8 @@
 
 /* Shorthand for this file only, to keep table narrow */
 
-#if defined(P) || defined(F) || defined(I) || defined(H) || defined(S)
+#if defined(P) || defined(F) || defined(I) || defined(H) || defined(S) || \
+    defined(K)
 #error "Macro overloading"  // Trust but verify
 #endif
 
@@ -50,65 +51,67 @@
 #define F HTTPH_R_FETCH
 #define I HTTPH_A_INS
 #define S HTTPH_A_PASS
+#define K HTTPH_C_SPECIFIC
 #define H(s,e,f) HTTPH(s, e, f)
 
-H("Accept",		H_Accept,		0      )	// 2616 14.1
-H("Accept-Charset",	H_Accept_Charset,	0      )	// 2616 14.2
-H("Accept-Encoding",	H_Accept_Encoding,	0      )	// 2616 14.3
-H("Accept-Language",	H_Accept_Language,	0      )	// 2616 14.4
-H("Accept-Ranges",	H_Accept_Ranges,	P|F|I  )	// 2616 14.5
-H("Age",		H_Age,			    I|S)	// 2616 14.6
-H("Allow",		H_Allow,		0      )	// 2616 14.7
-H("Authorization",	H_Authorization,	0      )	// 2616 14.8
-H("Cache-Control",	H_Cache_Control,	  F    )	// 2616 14.9
-H("Connection",		H_Connection,		P|F|I|S)	// 2616 14.10
-H("Content-Encoding",	H_Content_Encoding,	0      )	// 2616 14.11
-H("Content-Language",	H_Content_Language,	0      )	// 2616 14.12
-H("Content-Length",	H_Content_Length,	0      )	// 2616 14.13
-H("Content-Location",	H_Content_Location,	0      )	// 2616 14.14
-H("Content-MD5",	H_Content_MD5,		0      )	// 2616 14.15
-H("Content-Range",	H_Content_Range,	  F|I  )	// 2616 14.16
-H("Content-Type",	H_Content_Type,		0      )	// 2616 14.17
-H("Cookie",		H_Cookie,		0      )	// 6265 4.2
-H("Date",		H_Date,			0      )	// 2616 14.18
-H("ETag",		H_ETag,			0      )	// 2616 14.19
-H("Expect",		H_Expect,		0      )	// 2616 14.20
-H("Expires",		H_Expires,		0      )	// 2616 14.21
-H("From",		H_From,			0      )	// 2616 14.22
-H("Host",		H_Host,			0      )	// 2616 14.23
-H("HTTP2-Settings",	H_HTTP2_Settings,	P|F|I|S)	// 7540 3.2.1
-H("If-Match",		H_If_Match,		  F    )	// 2616 14.24
-H("If-Modified-Since",	H_If_Modified_Since,	  F    )	// 2616 14.25
-H("If-None-Match",	H_If_None_Match,	  F    )	// 2616 14.26
-H("If-Range",		H_If_Range,		  F    )	// 2616 14.27
-H("If-Unmodified-Since",H_If_Unmodified_Since,	  F    )	// 2616 14.28
-H("Keep-Alive",		H_Keep_Alive,		P|F|I|S)	// 2616 13.5.1
-H("Last-Modified",	H_Last_Modified,	0      )	// 2616 14.29
-H("Location",		H_Location,		0      )	// 2616 14.30
-H("Max-Forwards",	H_Max_Forwards,		0      )	// 2616 14.31
-H("Pragma",		H_Pragma,		0      )	// 2616 14.32
-H("Proxy-Authenticate",	H_Proxy_Authenticate,	  F|I  )	// 2616 14.33
-H("Proxy-Authorization",H_Proxy_Authorization,	  F|I  )	// 2616 14.34
-H("Range",		H_Range,		  F|I  )	// 2616 14.35
-H("Referer",		H_Referer,		0      )	// 2616 14.36
-H("Retry-After",	H_Retry_After,		0      )	// 2616 14.37
-H("Server",		H_Server,		0      )	// 2616 14.38
-H("Set-Cookie",		H_Set_Cookie,		0      )	// 6265 4.1
-H("TE",			H_TE,			P|F|I|S)	// 2616 14.39
-H("Trailer",		H_Trailer,		P|F|I|S)	// 2616 14.40
-H("Transfer-Encoding",	H_Transfer_Encoding,	P|F|I|S)	// 2616 14.41
-H("Upgrade",		H_Upgrade,		P|F|I|S)	// 2616 14.42
-H("User-Agent",		H_User_Agent,		0      )	// 2616 14.43
-H("Vary",		H_Vary,			0      )	// 2616 14.44
-H("Via",		H_Via,			0      )	// 2616 14.45
-H("Warning",		H_Warning,		0      )	// 2616 14.46
-H("WWW-Authenticate",	H_WWW_Authenticate,	0      )	// 2616 14.47
-H("X-Forwarded-For",	H_X_Forwarded_For,	0      )	// No RFC
+H("Accept",		H_Accept,		0        )	// 2616 14.1
+H("Accept-Charset",	H_Accept_Charset,	0        )	// 2616 14.2
+H("Accept-Encoding",	H_Accept_Encoding,	0        )	// 2616 14.3
+H("Accept-Language",	H_Accept_Language,	0        )	// 2616 14.4
+H("Accept-Ranges",	H_Accept_Ranges,	P|F|I    )	// 2616 14.5
+H("Age",		H_Age,			    I|S  )	// 2616 14.6
+H("Allow",		H_Allow,		0        )	// 2616 14.7
+H("Authorization",	H_Authorization,	0        )	// 2616 14.8
+H("Cache-Control",	H_Cache_Control,	  F      )	// 2616 14.9
+H("Connection",		H_Connection,		P|F|I|S|K)	// 2616 14.10
+H("Content-Encoding",	H_Content_Encoding,	0        )	// 2616 14.11
+H("Content-Language",	H_Content_Language,	0        )	// 2616 14.12
+H("Content-Length",	H_Content_Length,	0        )	// 2616 14.13
+H("Content-Location",	H_Content_Location,	0        )	// 2616 14.14
+H("Content-MD5",	H_Content_MD5,		0        )	// 2616 14.15
+H("Content-Range",	H_Content_Range,	  F|I    )	// 2616 14.16
+H("Content-Type",	H_Content_Type,		0        )	// 2616 14.17
+H("Cookie",		H_Cookie,		0        )	// 6265 4.2
+H("Date",		H_Date,			0        )	// 2616 14.18
+H("ETag",		H_ETag,			0        )	// 2616 14.19
+H("Expect",		H_Expect,		0        )	// 2616 14.20
+H("Expires",		H_Expires,		0        )	// 2616 14.21
+H("From",		H_From,			0        )	// 2616 14.22
+H("Host",		H_Host,			0        )	// 2616 14.23
+H("HTTP2-Settings",	H_HTTP2_Settings,	P|F|I|S|K)	// 7540 3.2.1
+H("If-Match",		H_If_Match,		  F      )	// 2616 14.24
+H("If-Modified-Since",	H_If_Modified_Since,	  F      )	// 2616 14.25
+H("If-None-Match",	H_If_None_Match,	  F      )	// 2616 14.26
+H("If-Range",		H_If_Range,		  F      )	// 2616 14.27
+H("If-Unmodified-Since",H_If_Unmodified_Since,	  F      )	// 2616 14.28
+H("Keep-Alive",		H_Keep_Alive,		P|F|I|S|K)	// 2616 13.5.1
+H("Last-Modified",	H_Last_Modified,	0        )	// 2616 14.29
+H("Location",		H_Location,		0        )	// 2616 14.30
+H("Max-Forwards",	H_Max_Forwards,		0        )	// 2616 14.31
+H("Pragma",		H_Pragma,		0        )	// 2616 14.32
+H("Proxy-Authenticate",	H_Proxy_Authenticate,	  F|I    )	// 2616 14.33
+H("Proxy-Authorization",H_Proxy_Authorization,	  F|I    )	// 2616 14.34
+H("Range",		H_Range,		  F|I    )	// 2616 14.35
+H("Referer",		H_Referer,		0        )	// 2616 14.36
+H("Retry-After",	H_Retry_After,		0        )	// 2616 14.37
+H("Server",		H_Server,		0        )	// 2616 14.38
+H("Set-Cookie",		H_Set_Cookie,		0        )	// 6265 4.1
+H("TE",			H_TE,			P|F|I|S  )	// 2616 14.39
+H("Trailer",		H_Trailer,		P|F|I|S  )	// 2616 14.40
+H("Transfer-Encoding",	H_Transfer_Encoding,	P|F|I|S|K)	// 2616 14.41
+H("Upgrade",		H_Upgrade,		P|F|I|S|K)	// 2616 14.42
+H("User-Agent",		H_User_Agent,		0        )	// 2616 14.43
+H("Vary",		H_Vary,			0        )	// 2616 14.44
+H("Via",		H_Via,			0        )	// 2616 14.45
+H("Warning",		H_Warning,		0        )	// 2616 14.46
+H("WWW-Authenticate",	H_WWW_Authenticate,	0        )	// 2616 14.47
+H("X-Forwarded-For",	H_X_Forwarded_For,	0        )	// No RFC
 
 #undef P
 #undef F
 #undef I
 #undef S
+#undef K
 #undef H
 #undef HTTPH
 


More information about the varnish-commit mailing list