Re: X-Vary-Options patch

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