Re: X-Vary-Options patch

Fri, 06 Jun 2008 18:12:11 -0700 

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