Hello!

On Fri, Nov 15, 2013 at 04:35:09PM +0100, Florent Le Coz wrote:

> Hi,
>
> On 11/15/2013 01:44 PM, Maxim Dounin wrote:
> […]
> >
> > Perfectly correct solution would be to store a bit (likely in
> > u->headers_in) to indicate that valid_sec was set based on
> > X-Accel-Expires and shouldn't be overwritten.
> >
>
> Since there are, in nginx, three headers that can modify the value
> of valid_sec (Expires, Cache-Control and Expires), I think it would
> be cleaner to define a priority for each of these headers and to use
> that priority to decide if we modify or not the valid_sec value.
>
> That’s what I’ve done in the attached patch.
> When setting the value of valid_sec, each header writes its own
> priority in valid_sec_prio.
> When processing an other header, instead of checking if the
> valid_sec is already set, we check if the headers’ priority is
> higher than the one set, before setting (or not) the value found in
> the header being processed.
>
> I’ve set the priorities as: X-Accel-Expires > Cache-Control > Expires.
> I’m not sure about what the priority of X-Accel-Expires should be
> (but the last two are well defined in the RFC as you correctly
> pointed in a previous message).

That's certainly looks like an overkill. At most, we need just
one bit to disambiguate the Cache-Control header processing, as it
needs to know whether valid_sec was set by X-Accel-Expires (and
then it shouldn't do anything) or by Expires (and then it's
expected to override the value set).

(A side note: we might also want to do something with u->cacheable
set to 0 by Expires. The Expires header is expected to be
overriden by Cache-Control, but it doesn't happen if an Expires
header contained a date in the past and u->cacheable was set to 0
due to it.)

[...]

> @@ -3674,6 +3675,10 @@ ngx_http_upstream_process_expires(ngx_ht
> return NGX_OK;
> }
>
> + if (u->headers_in.valid_sec_prio >= NGX_HTTP_UPSTREAM_EXPIRES_H_P) {
> + return NGX_OK;
> + }
> +

Just a side note: this change is a nop as the check isn't reached
if u->cache->valid_sec is set.

--
Maxim Dounin
http://nginx.org/en/donation.html

_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel