r5531 - trunk/varnish-cache/bin/varnishd

phk at varnish-cache.org phk at varnish-cache.org
Tue Nov 9 13:16:30 CET 2010


Author: phk
Date: 2010-11-09 13:16:30 +0100 (Tue, 09 Nov 2010)
New Revision: 5531

Modified:
   trunk/varnish-cache/bin/varnishd/cache.h
   trunk/varnish-cache/bin/varnishd/cache_center.c
   trunk/varnish-cache/bin/varnishd/cache_http.c
Log:
One of the silly overgeneralizations in RFC2616, is that headers which
contain comma-separated lists, can be spread over multiple header
lines.

There is no way of knowing if this rule applies to any header not
in RFC2616, short of chasing down the relevant standards document,
if any, for the particular header.
 
Considering the fact that HTTP header lines have no natural
limitation on length AND that RFC2616 already specifies a mechanism
for header-continuation, this doesn't add any value, at all.
 
It is hardly a surprise that nobody used this either, so until now,
we have ignored this silly stuff and just used the first header we
found.
 
But now Chromium, of all things, seems to find it necessary to
spread its Cache-Control across two lines, and we get to deal
with this crap.
 
Add a function for stitching multiple header lines into one, and
call it on Cache-Control in requests to deal with Chromiums issues.

Since we have it, call it preemptively on Cache-Control and Vary
in backend responses, since the C-code examines these fields.

XXX: At some point, add VCL support for collecting specific headers
this way.

Fixes: #686



Modified: trunk/varnish-cache/bin/varnishd/cache.h
===================================================================
--- trunk/varnish-cache/bin/varnishd/cache.h	2010-11-09 11:25:24 UTC (rev 5530)
+++ trunk/varnish-cache/bin/varnishd/cache.h	2010-11-09 12:16:30 UTC (rev 5531)
@@ -588,6 +588,7 @@
 const char *http_DoConnection(const struct http *hp);
 void http_CopyHome(struct worker *w, int fd, const struct http *hp);
 void http_Unset(struct http *hp, const char *hdr);
+void http_CollectHdr(struct http *hp, const char *hdr);
 
 /* cache_httpconn.c */
 void HTC_Init(struct http_conn *htc, struct ws *ws, int fd);

Modified: trunk/varnish-cache/bin/varnishd/cache_center.c
===================================================================
--- trunk/varnish-cache/bin/varnishd/cache_center.c	2010-11-09 11:25:24 UTC (rev 5530)
+++ trunk/varnish-cache/bin/varnishd/cache_center.c	2010-11-09 12:16:30 UTC (rev 5531)
@@ -453,6 +453,14 @@
 	}
 
 	/*
+	 * These two headers can be spread over multiple actual headers
+	 * and we rely on their content outside of VCL, so collect them
+	 * into one line here.
+	 */
+	http_CollectHdr(sp->wrk->beresp, H_Cache_Control);
+	http_CollectHdr(sp->wrk->beresp, H_Vary);
+
+	/*
 	 * Save a copy before it might get mangled in VCL.  When it comes to
 	 * dealing with the body, we want to see the unadultered headers.
 	 */
@@ -1058,6 +1066,8 @@
 	sp->hash_ignore_busy = 0;
 	sp->client_identity = NULL;
 
+	http_CollectHdr(sp->http, H_Cache_Control);
+
 	VCL_recv_method(sp);
 	recv_handling = sp->handling;
 

Modified: trunk/varnish-cache/bin/varnishd/cache_http.c
===================================================================
--- trunk/varnish-cache/bin/varnishd/cache_http.c	2010-11-09 11:25:24 UTC (rev 5530)
+++ trunk/varnish-cache/bin/varnishd/cache_http.c	2010-11-09 12:16:30 UTC (rev 5531)
@@ -163,7 +163,6 @@
 
 /*--------------------------------------------------------------------*/
 
-
 static int
 http_IsHdr(const txt *hh, const char *hdr)
 {
@@ -178,6 +177,72 @@
 	return (!strncasecmp(hdr, hh->b, l));
 }
 
+/*--------------------------------------------------------------------
+ * This function collapses multiple headerlines of the same name.
+ * The lines are joined with a comma, according to [rfc2616, 4.2bot, p32]
+ */
+
+void
+http_CollectHdr(struct http *hp, const char *hdr)
+{
+	unsigned u, v, ml, f = 0, x;
+	char *b = NULL, *e = NULL;
+
+	for (u = HTTP_HDR_FIRST; u < hp->nhd; u++) {
+		Tcheck(hp->hd[u]);
+		if (!http_IsHdr(&hp->hd[u], hdr))
+			continue;
+		if (f == 0) {
+			/* Found first header, just record the fact */
+			f = u;
+			continue;
+		}
+		if (b == NULL) {
+			/* Found second header */
+			ml = WS_Reserve(hp->ws, 0);
+			b = hp->ws->f;
+			e = b + ml;
+			x = Tlen(hp->hd[f]);
+			if (b + x < e) {
+				memcpy(b, hp->hd[f].b, x);
+				b += x;
+			} else 
+				b = e;
+		}
+
+		AN(b);
+		AN(e);
+
+		/* Append the Nth header we found */
+		if (b < e)
+			*b++ = ',';
+		x = Tlen(hp->hd[u]) - *hdr;
+		if (b + x < e) {
+			memcpy(b, hp->hd[u].b + *hdr, x);
+			b += x;
+		} else 
+			b = e;
+
+		/* Shift remaining headers up one slot */
+		for (v = u; v < hp->nhd + 1; v++)
+			hp->hd[v] = hp->hd[v + 1];
+		hp->nhd--;
+
+	}
+	if (b == NULL)
+		return;
+	AN(e);
+	if (b >= e) {
+		WS_Release(hp->ws, 0);
+		return;
+	}
+	*b = '\0';
+	hp->hd[f].b = hp->ws->f;
+	hp->hd[f].e = b;
+	WS_ReleaseP(hp->ws, b + 1);
+}
+
+
 /*--------------------------------------------------------------------*/
 
 static unsigned
@@ -186,11 +251,6 @@
 	unsigned u;
 
 	for (u = HTTP_HDR_FIRST; u < hp->nhd; u++) {
-		/* XXX We have to check for empty header entries
-		   because a header could have been lost in
-		   http_copyHome */
-		if (hp->hd[u].b == NULL)
-			continue;
 		Tcheck(hp->hd[u]);
 		if (hp->hd[u].e < hp->hd[u].b + l + 1)
 			continue;
@@ -228,6 +288,7 @@
 	return (1);
 }
 
+
 /*--------------------------------------------------------------------
  * Find a given headerfield, and if present and wanted, the beginning
  * of its value.




More information about the varnish-commit mailing list