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       [EMAIL PROTECTED]


Reply via email to