X-Vary-Options patch
|
|
Tim Starling-2
|
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) { |
Robert Collins
|
On Fri, 2008-02-08 at 16:26 +1100, Tim Starling wrote: > > > The added features of the patch are conditional, and are enabled by > the > configure option --enable-vary-options. Unless there is non-trivial process required for regular vary headers with this enabled, I don't think it needs to be optional. -Rob -- GPG key available at: <http://www.robertcollins.net/keys.txt>. |
|||||||||||||||||||
signature.asc (196 bytes)
pokeman
|
In reply to this post
by Tim Starling-2
can you please tell me how to apply this patch im using 2.6.18 we are getting error in my squid.log
2008/02/14 23:19:30| storeLocateVary: Not our vary marker object, F0B4F9361C107C816247C9D0B6B88927 = 'http://l.yimg.com/us.js.yimg.com/lib/bc/bc_2.0.4.js', 'accept-encoding="gzip,%20deflate"'/'-'
|
||||||||||||||||||
|
pokeman
|
In reply to this post
by Robert Collins
can you please send the TAGs what i made change in squid.conf
|
||||||||||||||||||
Adrian Chadd
|
In reply to this post
by Tim Starling-2
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-4
|
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 [hidden email] |
|||||||||||||||||||
|
Adrian Chadd
|
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 [hidden email] > -- - Xenion - http://www.xenion.com.au/ - VPS Hosting - Commercial Squid Support - - $25/pm entry-level VPSes w/ capped bandwidth charges available in WA - |
|||||||||||||||||||
Henrik Nordstrom-5
|
On lör, 2008-06-07 at 10:43 +0800, Adrian Chadd wrote:
> I think some of their stuff was backed out of Squid-2.7 before > release. The Vary invalidation patch was backed out from 2.7 as it's incomplete and broke things. But this X-Vary-Options patch never got committed. Thread stops after your request for him to file a bugzilla entry, and it got lost in the noise until found again by Mark. I have concerns about the completeness about this patch, for example if it handles q values properly. It's not such abig deal on Accept-Encoding, but can get quite messy if applying this to Accept or Accept-Language. For Accept* heaers I think it needs to be extended with an option instructing caches to parse the Accept* header to a limited degree, which means the cache needs to know the list of available choices for the header at the server and their order of priority. Also, for cookie it needs to be a little more elaborate as most often one wants to match on cookie names, not their vaule.. and sometimes a value of a specific cookie. Regards Henrik |
|||||||||||||||||||
|
Mark Nottingham-4
|
+1 to what H says. I'm definitely interested in this area, but want to
think through it a bit more. We can get a certain amount of functionality without any extension; e.g., canonicalising selecting headers to take care of whitespace and case issues, and perhaps even ordering (this doesn't work for generic headers, but where we know the semantics and ordering isn't significant, it isn't a problem). That doesn't take care of the qval problem, but it helps in the accept- encoding case, which is the most common (I don't often see qvals on a- e; anybody?). One of my concerns about X-Vary-Options (can we please drop the 'X-'?) is that there are non-obvious corner cases; it forces the origin server admin to think very carefully about all the different variants that they're going to issue, and the request headers that will match them. If we can get rid of the common cases by canonicalisation, they will have less opportunity to mess things up. Also, it's important to realise that at some point it's more worthwhile to take an approach like TCN and describe the available variants, rather than match against selecting headers. Cheers, On 07/06/2008, at 5:19 PM, Henrik Nordstrom wrote: > On lör, 2008-06-07 at 10:43 +0800, Adrian Chadd wrote: >> I think some of their stuff was backed out of Squid-2.7 before >> release. > > The Vary invalidation patch was backed out from 2.7 as it's incomplete > and broke things. > > But this X-Vary-Options patch never got committed. Thread stops after > your request for him to file a bugzilla entry, and it got lost in the > noise until found again by Mark. > > I have concerns about the completeness about this patch, for example > if > it handles q values properly. It's not such abig deal on > Accept-Encoding, but can get quite messy if applying this to Accept or > Accept-Language. > > For Accept* heaers I think it needs to be extended with an option > instructing caches to parse the Accept* header to a limited degree, > which means the cache needs to know the list of available choices for > the header at the server and their order of priority. > > Also, for cookie it needs to be a little more elaborate as most often > one wants to match on cookie names, not their vaule.. and sometimes a > value of a specific cookie. > > Regards > Henrik > -- Mark Nottingham [hidden email] |
|||||||||||||||||||
|
Adrian Chadd
|
Mark,
Just create a wiki page in wiki.squid-cache.org and kick-start this. Adrian On Sun, Jun 08, 2008, Mark Nottingham wrote: > +1 to what H says. I'm definitely interested in this area, but want to > think through it a bit more. > > We can get a certain amount of functionality without any extension; > e.g., canonicalising selecting headers to take care of whitespace and > case issues, and perhaps even ordering (this doesn't work for generic > headers, but where we know the semantics and ordering isn't > significant, it isn't a problem). > > That doesn't take care of the qval problem, but it helps in the accept- > encoding case, which is the most common (I don't often see qvals on a- > e; anybody?). One of my concerns about X-Vary-Options (can we please > drop the 'X-'?) is that there are non-obvious corner cases; it forces > the origin server admin to think very carefully about all the > different variants that they're going to issue, and the request > headers that will match them. If we can get rid of the common cases by > canonicalisation, they will have less opportunity to mess things up. > > Also, it's important to realise that at some point it's more > worthwhile to take an approach like TCN and describe the available > variants, rather than match against selecting headers. > > Cheers, > > > > On 07/06/2008, at 5:19 PM, Henrik Nordstrom wrote: > > >On l?r, 2008-06-07 at 10:43 +0800, Adrian Chadd wrote: > >>I think some of their stuff was backed out of Squid-2.7 before > >>release. > > > >The Vary invalidation patch was backed out from 2.7 as it's incomplete > >and broke things. > > > >But this X-Vary-Options patch never got committed. Thread stops after > >your request for him to file a bugzilla entry, and it got lost in the > >noise until found again by Mark. > > > >I have concerns about the completeness about this patch, for example > >if > >it handles q values properly. It's not such abig deal on > >Accept-Encoding, but can get quite messy if applying this to Accept or > >Accept-Language. > > > >For Accept* heaers I think it needs to be extended with an option > >instructing caches to parse the Accept* header to a limited degree, > >which means the cache needs to know the list of available choices for > >the header at the server and their order of priority. > > > >Also, for cookie it needs to be a little more elaborate as most often > >one wants to match on cookie names, not their vaule.. and sometimes a > >value of a specific cookie. > > > >Regards > >Henrik > > > > -- > Mark Nottingham [hidden email] > -- - Xenion - http://www.xenion.com.au/ - VPS Hosting - Commercial Squid Support - - $25/pm entry-level VPSes w/ capped bandwidth charges available in WA - |
|||||||||||||||||||
|
Henrik Nordstrom-5
|
In reply to this post
by Mark Nottingham-4
On sön, 2008-06-08 at 11:23 +1000, Mark Nottingham wrote:
> That doesn't take care of the qval problem, but it helps in the accept- > encoding case, which is the most common (I don't often see qvals on a- > e; anybody?). Not on Accept-Encoding, but quite often on Accept-Language. > One of my concerns about X-Vary-Options (can we please > drop the 'X-'?) is that there are non-obvious corner cases; it forces > the origin server admin to think very carefully about all the > different variants that they're going to issue, and the request > headers that will match them. If we can get rid of the common cases by > canonicalisation, they will have less opportunity to mess things up. Agreed. > Also, it's important to realise that at some point it's more > worthwhile to take an approach like TCN and describe the available > variants, rather than match against selecting headers. Yes, what I said.. To make this work "as intended by admins" without excessive roundtrips the cache needs to know the available variants and their priority, allowing the cache to perform the Accept-* logics itself to see if the response is acceptable. Yes, by pathing up and sorting Accept-Encoding can be cut down to a quite acceptable level even without this. But it's harder for Accept-Language or other Accept* headers where there is more diversity. A gentle reminder for those not familiar with the problem: Vary in HTTP is very blunt and only says "I looked at these headers to determine what response to send, so this response is only valid for any request having the exact same content in those headers", but for many situations this is sub-optimal as those headers can be send in a wide variety of different manners which all yield the same result. For a gzip encoding server the following Accept-Encoding variants is all equal: "gzip", "gzip, deflate", "deflate,gzip", "deflate, gzip", "x-myencoding, gzip", "deflate=0, gzip". "gzip=1, deflate=0.5". But the following is very different: "gzip=0, deflate" By sorting and canonicalisation it can be cut down to "gzip" "gzip, deflate" ("gzip, deflate", "deflate,gzip", "deflate, gzip") "x-myencoding, gzip" "gzip, deflate=0" "gzip=1, deflate=0.5" And by special casing it can be further cut down to "gzip" ("gzip", "gzip, deflate", "deflate,gzip", "deflate, gzip", "gzip, deflate=0", "gzip=1, deflate=0.5", "x-myencoding, gzip") based on the special casing that gzip is far mor common than deflate, and that content-encodings are fully interchangeable for implementaitons supporting both. Regards Henrik |
|||||||||||||||||||
| Free Embeddable Forum Powered by Nabble | Help |
