Re: X-Vary-Options patch

[Henrik Nordstrom]

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

Re: X-Vary-Options patch

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

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

Re: X-Vary-Options patch

[Mark Nottingham]

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

Re: X-Vary-Options patch

[Henrik Nordstrom]

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

Re: X-Vary-Options patch

[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.in2008-01-10 23:34:23.0  
> >>+1100
> >>+++ squid-2.6.18/configure.in 2008-02-07 19:43:23.0 +1100
> >>@@ -1507,6 +1507,16 @@
> >>  fi
> >>])
> >>
> >>+dnl Enable vary options
> >>+AC_ARG_ENABLE(vary_options,
> >>+[  --enable-vary-options
> >>+ Ena

Re: X-Vary-Options patch

[Mark Nottingham]

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.in2008-01-10 23:34:23.0  
+1100

+++ squid-2.6.18/configure.in 2008-02-07 19:43:23.0 +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.0 +1100
+++ squid-2.6.18/src/client_side.c2008-02-08 14:39:38.0  
+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 = htt

Re: X-Vary-Options patch

[Adrian Chadd]

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.in2008-01-10 23:34:23.0 +1100
> +++ squid-2.6.18/configure.in 2008-02-07 19:43:23.0 +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.0 
> +1100
> +++ squid-2.6.18/src/client_side.c2008-02-08 14:39:38.0 +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)
> 

Re: X-Vary-Options patch

[pokeman]

can you please send the TAGs what i made change in squid.conf

Robert Collins wrote:
> 
> 
> 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>.
> 
>  
> 

-- 
View this message in context: 
http://www.nabble.com/X-Vary-Options-patch-tp15349937p15495250.html
Sent from the Squid - Development mailing list archive at Nabble.com.

Re: X-Vary-Options patch

[pokeman]

 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.0 +1000
> +++ squid-2.6.18/src/HttpReply.c  2008-02-08 14:42:04.0 +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.h2008-02-07 19:28:38.0 +1100
> +++ squid-2.6.18/src/protos.h 2008-02-08 14:46:21.0 +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.0 +1100
> +++ squid-2.6.18/src/store.c  2008-02-08 14:55:06.0 +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 +
> 10);
> +#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)  {
> 
> 
> 

-- 
View this message in context: 
http://www.nabble.com/X-Vary-Options-patch-tp15349937p15489646.html
Sent from the Squid - Development mailing list archive at Nabble.com.

Re: X-Vary-Options patch

[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: .


signature.asc
Description: This is a digitally signed message part

X-Vary-Options patch

[Tim Starling]

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.0 +1100
+++ squid-2.6.18/configure.in   2008-02-07 19:43:23.0 +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.0 +1100
+++ squid-2.6.18/src/client_side.c  2008-02-08 14:39:38.0 +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_ACCELER