Re: X-Vary-Options patch

I think some of their stuff was backed out of Squid-2.7 before
release.

What we -should- do is create a wiki page to document all the crazy
stuff about Vary: and coordinate things a little better. I'd really
like to see more sensible vary handling go into Squid and there
certainly seems like there is enough interest in it now to get
work done.

Adrian

On Sat, Jun 07, 2008, Mark Nottingham wrote:
> Squid devs: Did this make its way into 2.7?
>
> Tim: AIUI, the following header:
> X-Vary-Options: Accept-Encoding; list-contains=gzip
> will bucket the cache for this URI into two entries; those whose
> Accept-Encoding contains the list value "gzip", and those that don't.
> Is that correct?
>
> Also, I'm assuming the origin would still need to send
> Vary: Accept-Encoding
> to work properly with downstream caches.
>
> Cheers,
>
>
> On 26/02/2008, at 3:30 PM, Adrian Chadd wrote:
>
> >G'day,
> >
> >I'm happy to commit this to Squid-2.HEAD as-is. Can you throw it in
> >a Bugzilla report and spit me the number?
> >
> >Thanks,
> >
> >
> >
> >Adrian
> >
> >On Fri, Feb 08, 2008, Tim Starling wrote:
> >>There are two major sources of suboptimal hit rate on Wikipedia which
> >>relate to the Vary header:
> >>
> >>* In Accept-Encoding, we only care whether "gzip" is present or
> >>not, but
> >>IE and Firefox use different whitespace conventions and so each get
> >>separate entries in the cache
> >>* We only care whether the user is logged in or not. Other cookies,
> >>such
> >>as pure-JavaScript cookies used by client-side code to store
> >>preferences, unnecessarily degrade our hit rate.
> >>
> >>There have been other patches related to this problem, but as far
> >>as I'm
> >>aware, they're all special-case, site-specific hacks. My patch adds
> >>an
> >>X-Vary-Options response header (hereafter XVO), and thus gives the
> >>origin server fine control over cache variance. In the patch, the XVO
> >>header overrides the Vary header, so the Vary header can still be
> >>sent
> >>as usual for compatibility with caches that don't support this
> >>feature.
> >>
> >>The format of the XVO header is inspired by the format of the Accept
> >>header. As in Vary, XVO is separated by commas into parts which
> >>relate
> >>to different request headers. Then those parts are further
> >>separated by
> >>semicolons. The first semicolon-separated part is the request header
> >>name, and subsequent parts give name/value pairs separated by equals
> >>signs, defining options relating to the variance of that header.
> >>
> >>Two option names are currently defined:
> >>
> >>list-contains: splits the request header into comma-separated parts
> >>and varies depending on whether the resulting list contains the
> >>option value
> >>string-contains: performs a simple search of the request header and
> >>varies depending on whether it matches.
> >>
> >>Multiple such options per header are allowed.
> >>
> >>So for example:
> >>
> >>X-Vary-Options: Cookie; string-contains=UserID;
> >>string-contains=_session, Accept-Encoding; list-contains=gzip
> >>
> >>This would vary the cache on three tests:
> >>* whether the Cookie header contains the string "UserID"
> >>* whether the Cookie header contains the string "_session"
> >>* whether the Accept-Encoding header, interpreted as a comma-
> >>separated
> >>list, contains the item "gzip"
> >>
> >>The patch refactors all references to the Vary and X-Accelerator-Vary
> >>headers into the functions httpHeaderHasVary() and
> >>httpHeaderGetVary()
> >>in HttpHeader.c. It then adds X-Vary-Options to these functions,
> >>interpreting it as a string rather than a list to avoid inappropriate
> >>splitting on whitespace. It puts the resulting combined header into
> >>X-Vary-Options instead of Vary in the base vary marker object,
> >>again to
> >>avoid inappropriate list-style interpretation. httpMakeVaryMark()
> >>then
> >>interprets this combined header in the way described above.
> >>
> >>The added features of the patch are conditional, and are enabled by
> >>the
> >>configure option --enable-vary-options. Autoconf and automake will
> >>need
> >>to be run after applying this patch.
> >>
> >>The interpretation of some non-standards-compliant Vary headers
> >>(those
> >>containing semicolons) is changed slightly by this patch regardless
> >>of
> >>--enable-vary-options.
> >>
> >>The patch is attached and also available at:
> >>http://noc.wikimedia.org/~tstarling/patches/vary_options_upstream.patch
> >>
> >>For your review and consideration.
> >>
> >>-- Tim Starling
> >
> >>diff -Xdiffx -ru squid-2.6.18.orig/configure.in squid-2.6.18/
> >>configure.in
> >>--- squid-2.6.18.orig/configure.in 2008-01-10 23:34:23.000000000
> >>+1100
> >>+++ squid-2.6.18/configure.in 2008-02-07 19:43:23.000000000 +1100
> >>@@ -1507,6 +1507,16 @@
> >> fi
> >>])
> >>
> >>+dnl Enable vary options
> >>+AC_ARG_ENABLE(vary_options,
> >>+[ --enable-vary-options
> >>+ Enable support for the X-Vary-Options
> >>header.],
> >>+[ if test "$enableval" = "yes" ; then
> >>+ echo "Enabling support for vary options"
> >>+ AC_DEFINE(VARY_OPTIONS, 1, [Enable support for the X-Vary-
> >>Options header])
> >>+ fi
> >>+])
> >>+
> >>AC_ARG_ENABLE(follow-x-forwarded-for,
> >>[ --enable-follow-x-forwarded-for
> >> Enable support for following the X-
> >>Forwarded-For
> >>diff -Xdiffx -ru squid-2.6.18.orig/src/client_side.c squid-2.6.18/
> >>src/client_side.c
> >>--- squid-2.6.18.orig/src/client_side.c 2008-02-07
> >>19:28:38.000000000 +1100
> >>+++ squid-2.6.18/src/client_side.c 2008-02-08 14:39:38.000000000
> >>+1100
> >>@@ -735,10 +735,7 @@
> >> request_t *request = http->request;
> >> const char *etag = httpHeaderGetStr(&mem->reply->header,
> >>HDR_ETAG);
> >> const char *vary = request->vary_headers;
> >>- int has_vary = httpHeaderHas(&entry->mem_obj->reply-
> >>>header, HDR_VARY);
> >>-#if X_ACCELERATOR_VARY
> >>- has_vary |= httpHeaderHas(&entry->mem_obj->reply->header,
> >>HDR_X_ACCELERATOR_VARY);
> >>-#endif
> >>+ int has_vary = httpHeaderHasVary(&entry->mem_obj->reply-
> >>>header);
> >> if (has_vary)
> >> vary = httpMakeVaryMark(request, mem->reply);
> >>
> >>@@ -4948,10 +4945,7 @@
> >>varyEvaluateMatch(StoreEntry * entry, request_t * request)
> >>{
> >> const char *vary = request->vary_headers;
> >>- int has_vary = httpHeaderHas(&entry->mem_obj->reply->header,
> >>HDR_VARY);
> >>-#if X_ACCELERATOR_VARY
> >>- has_vary |= httpHeaderHas(&entry->mem_obj->reply->header,
> >>HDR_X_ACCELERATOR_VARY);
> >>-#endif
> >>+ int has_vary = httpHeaderHasVary(&entry->mem_obj->reply-
> >>>header);
> >> if (!has_vary || !entry->mem_obj->vary_headers) {
> >> if (vary) {
> >> /* Oops... something odd is going on here.. */
> >>diff -Xdiffx -ru squid-2.6.18.orig/src/enums.h squid-2.6.18/src/
> >>enums.h
> >>--- squid-2.6.18.orig/src/enums.h 2008-02-07 19:28:38.000000000
> >>+1100
> >>+++ squid-2.6.18/src/enums.h 2008-02-07 21:35:18.000000000 +1100
> >>@@ -256,6 +256,9 @@
> >>#if X_ACCELERATOR_VARY
> >> HDR_X_ACCELERATOR_VARY,
> >>#endif
> >>+#if VARY_OPTIONS
> >>+ HDR_X_VARY_OPTIONS,
> >>+#endif
> >> HDR_X_ERROR_URL, /* errormap, requested URL */
> >> HDR_X_ERROR_STATUS, /* errormap, received HTTP
> >>status line */
> >> HDR_FRONT_END_HTTPS,
> >>diff -Xdiffx -ru squid-2.6.18.orig/src/http.c squid-2.6.18/src/http.c
> >>--- squid-2.6.18.orig/src/http.c 2008-02-07 19:28:38.000000000
> >>+1100
> >>+++ squid-2.6.18/src/http.c 2008-02-08 14:48:44.000000000 +1100
> >>@@ -353,20 +353,29 @@
> >> String vstr = StringNull;
> >>
> >> stringClean(&vstr);
> >>- hdr = httpHeaderGetList(&reply->header, HDR_VARY);
> >>- if (strBuf(hdr))
> >>- strListAdd(&vary, strBuf(hdr), ',');
> >>- stringClean(&hdr);
> >>-#if X_ACCELERATOR_VARY
> >>- hdr = httpHeaderGetList(&reply->header, HDR_X_ACCELERATOR_VARY);
> >>- if (strBuf(hdr))
> >>- strListAdd(&vary, strBuf(hdr), ',');
> >>- stringClean(&hdr);
> >>-#endif
> >>+ vary = httpHeaderGetVary(&reply->header);
> >>+ debug(11,3) ("httpMakeVaryMark: Vary: %s\n", strBuf(vary));
> >>+
> >> while (strListGetItem(&vary, ',', &item, &ilen, &pos)) {
> >>- char *name = xmalloc(ilen + 1);
> >>- xstrncpy(name, item, ilen + 1);
> >>- Tolower(name);
> >>+ const char *sc_item, *sc_pos = NULL;
> >>+ int sc_ilen;
> >>+ String str_item;
> >>+ char *name = NULL;
> >>+ String value_spec = StringNull;
> >>+ int need_value = 1;
> >>+
> >>+ stringLimitInit(&str_item, item, ilen);
> >>+
> >>+ /* Get the header name */
> >>+ if (strListGetItem(&str_item, ';', &sc_item, &sc_ilen,
> >>&sc_pos)) {
> >>+ name = xmalloc(sc_ilen + 1);
> >>+ xstrncpy(name, sc_item, sc_ilen + 1);
> >>+ Tolower(name);
> >>+ } else {
> >>+ name = xmalloc(1);
> >>+ *name = '\0';
> >>+ }
> >>+
> >> if (strcmp(name, "accept-encoding") == 0) {
> >> aclCheck_t checklist;
> >> memset(&checklist, 0, sizeof(checklist));
> >>@@ -381,22 +390,76 @@
> >> if (strcmp(name, "*") == 0) {
> >> /* Can not handle "Vary: *" efficiently, bail out making
> >>the response not cached */
> >> safe_free(name);
> >>+ stringClean(&str_item);
> >> stringClean(&vary);
> >> stringClean(&vstr);
> >> break;
> >> }
> >>- strListAdd(&vstr, name, ',');
> >>+
> >>+ /* Fetch the header string */
> >> hdr = httpHeaderGetByName(&request->header, name);
> >>- safe_free(name);
> >>- value = strBuf(hdr);
> >>- if (value) {
> >>- value = rfc1738_escape_part(value);
> >>- stringAppend(&vstr, "=\"", 2);
> >>- stringAppend(&vstr, value, strlen(value));
> >>- stringAppend(&vstr, "\"", 1);
> >>+
> >>+ /* Process the semicolon-separated options */
> >>+#ifdef VARY_OPTIONS
> >>+ while (strListGetItem(&str_item, ';', &sc_item, &sc_ilen,
> >>&sc_pos)) {
> >>+ char *opt_name = xmalloc(sc_ilen + 1);
> >>+ char *opt_value;
> >>+ char *eqpos;
> >>+ xstrncpy(opt_name, sc_item, sc_ilen + 1);
> >>+ eqpos = strchr(opt_name, '=');
> >>+ if (!eqpos) {
> >>+ opt_value = NULL;
> >>+ } else {
> >>+ *eqpos = '\0';
> >>+ opt_value = eqpos + 1;
> >>+ }
> >>+ Tolower(opt_name);
> >>+
> >>+ if (strcmp(opt_name, "list-contains") == 0 && opt_value) {
> >>+ if (strBuf(hdr) && strListIsMember(&hdr, opt_value,
> >>',')) {
> >>+ opt_value = rfc1738_escape_part(opt_value);
> >>+ strListAdd(&value_spec, "list-contains[\"", ';');
> >>+ stringAppend(&value_spec, opt_value,
> >>strlen(opt_value));
> >>+ stringAppend(&value_spec, "\"]", 2);
> >>+ }
> >>+ need_value = 0;
> >>+ } else if (strcmp(opt_name, "string-contains") == 0 &&
> >>opt_value) {
> >>+ if (strBuf(hdr) && strIsSubstr(&hdr, opt_value)) {
> >>+ opt_value = rfc1738_escape_part(opt_value);
> >>+ strListAdd(&value_spec, "string-contains[\"", ';');
> >>+ stringAppend(&value_spec, opt_value,
> >>strlen(opt_value));
> >>+ stringAppend(&value_spec, "\"]", 2);
> >>+ }
> >>+ need_value = 0;
> >>+ } else {
> >>+ debug(11,3) ("httpMakeVaryMark: unrecognised vary
> >>option: %s\n", opt_name);
> >>+ }
> >>+ safe_free(opt_name);
> >> }
> >>+#endif
> >>+
> >>+ if (need_value) {
> >>+ value = strBuf(hdr);
> >>+ if (value) {
> >>+ value = rfc1738_escape_part(value);
> >>+ strListAdd(&value_spec, "\"", ';');
> >>+ stringAppend(&value_spec, value, strlen(value));
> >>+ stringAppend(&value_spec, "\"", 1);
> >>+ }
> >>+ }
> >>+
> >>+ strListAdd(&vstr, name, ',');
> >>+ stringAppend(&vstr, "=", 1);
> >>+ if (strBuf(value_spec)) {
> >>+ stringAppend(&vstr, strBuf(value_spec),
> >>strLen(value_spec));
> >>+ }
> >>+
> >> stringClean(&hdr);
> >>+ stringClean(&value_spec);
> >>+ stringClean(&str_item);
> >>+ safe_free(name);
> >> }
> >>+
> >> safe_free(request->vary_hdr);
> >> safe_free(request->vary_headers);
> >> if (strBuf(vary) && strBuf(vstr)) {
> >>@@ -514,11 +577,7 @@
> >> /* non-chunked. Handle as one single big chunk (-1 if
> >>terminated by EOF) */
> >> httpState->chunk_size = httpReplyBodySize(httpState-
> >>>orig_request->method, reply);
> >> }
> >>- if (httpHeaderHas(&reply->header, HDR_VARY)
> >>-#if X_ACCELERATOR_VARY
> >>- || httpHeaderHas(&reply->header, HDR_X_ACCELERATOR_VARY)
> >>-#endif
> >>- ) {
> >>+ if (httpHeaderHasVary(&reply->header)) {
> >> const char *vary = NULL;
> >> if (Config.onoff.cache_vary)
> >> vary = httpMakeVaryMark(httpState->orig_request, reply);
> >>diff -Xdiffx -ru squid-2.6.18.orig/src/HttpHeader.c squid-2.6.18/
> >>src/HttpHeader.c
> >>--- squid-2.6.18.orig/src/HttpHeader.c 2007-12-21
> >>20:56:53.000000000 +1100
> >>+++ squid-2.6.18/src/HttpHeader.c 2008-02-08 14:49:24.000000000
> >>+1100
> >>@@ -133,6 +133,9 @@
> >>#if X_ACCELERATOR_VARY
> >> {"X-Accelerator-Vary", HDR_X_ACCELERATOR_VARY, ftStr},
> >>#endif
> >>+#if VARY_OPTIONS
> >>+ {"X-Vary-Options", HDR_X_VARY_OPTIONS, ftStr},
> >>+#endif
> >> {"X-Error-URL", HDR_X_ERROR_URL, ftStr},
> >> {"X-Error-Status", HDR_X_ERROR_STATUS, ftInt},
> >> {"Front-End-Https", HDR_FRONT_END_HTTPS, ftStr},
> >>@@ -210,6 +213,9 @@
> >>#if X_ACCELERATOR_VARY
> >> HDR_X_ACCELERATOR_VARY,
> >>#endif
> >>+#if VARY_OPTIONS
> >>+ HDR_X_VARY_OPTIONS,
> >>+#endif
> >> HDR_X_SQUID_ERROR
> >>};
> >>
> >>@@ -1185,6 +1191,54 @@
> >> return tot;
> >>}
> >>
> >>+/* Get the combined Vary headers as a String
> >>+ * Returns StringNull if there are no vary headers
> >>+ */
> >>+String httpHeaderGetVary(const HttpHeader * hdr)
> >>+{
> >>+ String hdrString = StringNull;
> >>+#if VARY_OPTIONS
> >>+ HttpHeaderEntry *e;
> >>+ if ((e = httpHeaderFindEntry(hdr, HDR_X_VARY_OPTIONS))) {
> >>+ stringInit(&hdrString, strBuf(e->value));
> >>+ return hdrString;
> >>+ }
> >>+#endif
> >>+
> >>+ hdrString = httpHeaderGetList(hdr, HDR_VARY);
> >>+#if X_ACCELERATOR_VARY
> >>+ {
> >>+ String xavString = StringNull;
> >>+ xavString = httpHeaderGetList(hdr, HDR_X_ACCELERATOR_VARY);
> >>+ if (strBuf(xavString))
> >>+ strListAdd(&hdrString, strBuf(xavString), ',');
> >>+ stringClean(&xavString);
> >>+ }
> >>+#endif
> >>+ return hdrString;
> >>+}
> >>+
> >>+/*
> >>+ * Returns TRUE if at least one of the vary headers are present
> >>+ */
> >>+int httpHeaderHasVary(const HttpHeader * hdr)
> >>+{
> >>+#if VARY_OPTIONS
> >>+ if (httpHeaderHas(hdr, HDR_X_VARY_OPTIONS)) {
> >>+ return TRUE;
> >>+ }
> >>+#endif
> >>+#if X_ACCELERATOR_VARY
> >>+ if (httpHeaderHas(hdr, HDR_X_ACCELERATOR_VARY)) {
> >>+ return TRUE;
> >>+ }
> >>+#endif
> >>+ if (httpHeaderHas(hdr, HDR_VARY)) {
> >>+ return TRUE;
> >>+ }
> >>+ return FALSE;
> >>+}
> >>+
> >>/*
> >> * HttpHeaderEntry
> >> */
> >>@@ -1438,3 +1492,5 @@
> >> assert(id >= 0 && id < HDR_ENUM_END);
> >> return strBuf(Headers[id].name);
> >>}
> >>+
> >>+
> >>diff -Xdiffx -ru squid-2.6.18.orig/src/HttpReply.c squid-2.6.18/src/
> >>HttpReply.c
> >>--- squid-2.6.18.orig/src/HttpReply.c 2006-06-11 10:28:19.000000000
> >>+1000
> >>+++ squid-2.6.18/src/HttpReply.c 2008-02-08 14:42:04.000000000
> >>+1100
> >>@@ -325,8 +325,7 @@
> >> return squid_curtime;
> >> }
> >> }
> >>- if (Config.onoff.vary_ignore_expire &&
> >>- httpHeaderHas(&rep->header, HDR_VARY)) {
> >>+ if (Config.onoff.vary_ignore_expire && httpHeaderHasVary(&rep-
> >>>header)) {
> >> const time_t d = httpHeaderGetTime(&rep->header, HDR_DATE);
> >> const time_t e = httpHeaderGetTime(&rep->header, HDR_EXPIRES);
> >> if (d == e)
> >>diff -Xdiffx -ru squid-2.6.18.orig/src/protos.h squid-2.6.18/src/
> >>protos.h
> >>--- squid-2.6.18.orig/src/protos.h 2008-02-07 19:28:38.000000000
> >>+1100
> >>+++ squid-2.6.18/src/protos.h 2008-02-08 14:46:21.000000000 +1100
> >>@@ -444,6 +444,8 @@
> >>extern squid_off_t httpHeaderGetSize(const HttpHeader * hdr,
> >>http_hdr_type id);
> >>extern time_t httpHeaderGetTime(const HttpHeader * hdr,
> >>http_hdr_type id);
> >>extern TimeOrTag httpHeaderGetTimeOrTag(const HttpHeader * hdr,
> >>http_hdr_type id);
> >>+extern String httpHeaderGetVary(const HttpHeader * hdr);
> >>+extern int httpHeaderHasVary(const HttpHeader * hdr);
> >>extern HttpHdrCc *httpHeaderGetCc(const HttpHeader * hdr);
> >>extern HttpHdrRange *httpHeaderGetRange(const HttpHeader * hdr);
> >>extern HttpHdrContRange *httpHeaderGetContRange(const HttpHeader *
> >>hdr);
> >>diff -Xdiffx -ru squid-2.6.18.orig/src/store.c squid-2.6.18/src/
> >>store.c
> >>--- squid-2.6.18.orig/src/store.c 2008-02-07 19:28:38.000000000
> >>+1100
> >>+++ squid-2.6.18/src/store.c 2008-02-08 14:55:06.000000000 +1100
> >>@@ -721,7 +721,12 @@
> >> state->e = storeCreateEntry(url, log_url, flags, method);
> >> httpBuildVersion(&version, 1, 0);
> >> httpReplySetHeaders(state->e->mem_obj->reply, version, HTTP_OK,
> >>"Internal marker object", "x-squid-internal/vary", -1, -1,
> >>squid_curtime + 100000);
> >>+#if VARY_OPTIONS
> >>+ /* Can't put a string into a list header */
> >>+ httpHeaderPutStr(&state->e->mem_obj->reply->header,
> >>HDR_X_VARY_OPTIONS, vary);
> >>+#else
> >> httpHeaderPutStr(&state->e->mem_obj->reply->header, HDR_VARY,
> >>vary);
> >>+#endif
> >> storeSetPublicKey(state->e);
> >> if (!state->oe) {
> >> /* New entry, create new unique ID */
> >>@@ -1039,20 +1044,8 @@
> >> }
> >> newkey = storeKeyPublicByRequest(mem->request);
> >> if (mem->vary_headers && !EBIT_TEST(e->flags,
> >>KEY_EARLY_PUBLIC)) {
> >>- String vary = StringNull;
> >> vary_id_t vary_id;
> >>- String varyhdr;
> >>- varyhdr = httpHeaderGetList(&mem->reply->header, HDR_VARY);
> >>- if (strBuf(varyhdr))
> >>- strListAdd(&vary, strBuf(varyhdr), ',');
> >>- stringClean(&varyhdr);
> >>-#if X_ACCELERATOR_VARY
> >>- /* This needs to match the order in
> >>http.c:httpMakeVaryMark */
> >>- varyhdr = httpHeaderGetList(&mem->reply->header,
> >>HDR_X_ACCELERATOR_VARY);
> >>- if (strBuf(varyhdr))
> >>- strListAdd(&vary, strBuf(varyhdr), ',');
> >>- stringClean(&varyhdr);
> >>-#endif
> >>+ String vary = httpHeaderGetVary(&mem->reply->header);
> >> /* Create or update the vary object */
> >> vary_id = storeAddVary(mem->url, mem->log_url, mem-
> >>>method, newkey, httpHeaderGetStr(&mem->reply->header, HDR_ETAG),
> >>strBuf(vary), mem->vary_headers, mem->vary_encoding);
> >> if (vary_id.create_time) {
> >>
> >
> >
> >--
> >- Xenion - http://www.xenion.com.au/ - VPS Hosting - Commercial
> >Squid Support -
> >- $25/pm entry-level VPSes w/ capped bandwidth charges available in
> >WA -
>
> --
> Mark Nottingham mnot_at_yahoo-inc.com
>

-- 
- Xenion - http://www.xenion.com.au/ - VPS Hosting - Commercial Squid Support -
- $25/pm entry-level VPSes w/ capped bandwidth charges available in WA -