You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Ruediger Pluem <rp...@apache.org> on 2007/01/17 21:23:18 UTC

Re: Mod_cache expires check


On 01/15/2007 01:56 PM, Bart van der Schans wrote:
> In r463496 the following check was added to mod_cache.c :
> 
>     else if (exp != APR_DATE_BAD && exp < r->request_time)
>     {
>         /* if a Expires header is in the past, don't cache it */
>         reason = "Expires header already expired, not cacheable";
>     }
> 
> This check fails to correctly identify the expires header "Thu, 01 Jan
> 1970 00:00:00 GMT". The apr_date_parse_http function(exps) returns
> (apr_time_t)0 which is equal to APR_DATE_BAD, but it should recognize it
> as an already expired header. Is there a way to distinct between
> APR_DATE_BAD and the unix epoch? Or is that considered a bad date?

I would say 0 is not a bad day. But if this is a bug it is an APR(-util) bug.
Thus I forward it to the apr dev list.

Regards

Rüdiger


Re: Mod_cache expires check

Posted by Bart van der Schans <sc...@hippo.nl>.
Roy T. Fielding wrote:
> On Jan 17, 2007, at 12:23 PM, Ruediger Pluem wrote:
>> On 01/15/2007 01:56 PM, Bart van der Schans wrote:
>>> In r463496 the following check was added to mod_cache.c :
>>>
>>>     else if (exp != APR_DATE_BAD && exp < r->request_time)
>>>     {
>>>         /* if a Expires header is in the past, don't cache it */
>>>         reason = "Expires header already expired, not cacheable";
>>>     }
>>>
>>> This check fails to correctly identify the expires header "Thu, 01 Jan
>>> 1970 00:00:00 GMT". The apr_date_parse_http function(exps) returns
>>> (apr_time_t)0 which is equal to APR_DATE_BAD, but it should recognize it
>>> as an already expired header. Is there a way to distinct between
>>> APR_DATE_BAD and the unix epoch? Or is that considered a bad date?
>>
>> I would say 0 is not a bad day. But if this is a bug it is an 
>> APR(-util) bug.
>> Thus I forward it to the apr dev list.
> 
> No, it is a bug in the expression.  A date is an unsigned value and any
> error is specifically forced to 0 so that the comparison
> 
>    if (exp < r->request_time)
> 
> will be true if exp == APR_DATE_BAD (as is desired in this case).
That expression would also return true if the expires header is not set, 
which is probably not desired.

As I understand it, there are four cases:
a - Expires not set
b - Expires set to a bad (unparsable) date
c - Expires set to a date in the past (from and including 0)
d - Expires set to the future (up to +1 year)

According to rfc 2616 14.21, b and c should be treated as the same, but 
mod_cache treats a and b as the same:

exps = apr_table_get(r->err_headers_out, "Expires");
if (exps == NULL) {
     exps = apr_table_get(r->headers_out, "Expires");
}
if (exps != NULL) {
    if (APR_DATE_BAD == (exp = apr_date_parse_http(exps))) {
        exps = NULL;
    }
}
else {
    exp = APR_DATE_BAD;
}


If exp would be set to something like NOT_SET when the expires header is 
not set (in the else block) the check later on could be:

     else if (exp != NOT_SET && exp < r->request_time)
     {
         /* if a Expires header is in the past or bad, don't cache it */
         reason = "Expires header already expired or bad, not cacheable";
     }

It would seem obvious to set exp to NULL instead of NOT_SET, but NULL == 
APR_DATE_BAD evaluates to true (at least on my linux boxes) which would 
make it hard to discern later on between case a. and b.

Regards,

Bart

Re: Mod_cache expires check

Posted by Ruediger Pluem <rp...@apache.org>.

On 01/18/2007 12:49 PM, Henrik Nordstrom wrote:
> tor 2007-01-18 klockan 12:05 +0100 skrev Plüm, Rüdiger, VF EITO:

> 
> Any valid date which can be represented in the textual form is a valid
> Expires header. And any Expires header you can not understand for
> whatever reason is "already expired in the past".
> 
> To solve this there is Cache-Control max-age which is not sensitive to
> the limits of your internal time representation, and which overrides
> Expires if present.

Just trying to clarify:
So if we have
(an Expires header in the past *OR* a broken Expires header) *AND*
(a max-age > 0  *AND* a s-max-age not present ) *OR*
(a s-max-age > 0 *AND* (max-age >= 0 *OR* max-age not present)))

we should cache it (provided nothing else
in Cache-Control does restrict that)?



Regards

Rüdiger

Re: Mod_cache expires check

Posted by Henrik Nordstrom <he...@henriknordstrom.net>.
tor 2007-01-18 klockan 12:05 +0100 skrev Plüm, Rüdiger, VF EITO:

> Just curious: Is the Unix epoch an invalid date in the Expires header
> (as this in the past it does not really matter for the question whether
> this document is expired or not as it would be in both cases)?

The RFC does not care for the UNIX epoch.

Any valid date which can be represented in the textual form is a valid
Expires header. And any Expires header you can not understand for
whatever reason is "already expired in the past".

To solve this there is Cache-Control max-age which is not sensitive to
the limits of your internal time representation, and which overrides
Expires if present.

Regards
Henrik

Re: Mod_cache expires check

Posted by Plüm, Rüdiger, VF EITO <ru...@vodafone.com>.

> -----Ursprüngliche Nachricht-----
> Von: Roy T. Fielding 
> Gesendet: Mittwoch, 17. Januar 2007 23:23
> An: Ruediger Pluem
> Cc: dev@httpd.apache.org; dev@apr.apache.org
> Betreff: Re: Mod_cache expires check
> 
> 
> On Jan 17, 2007, at 12:23 PM, Ruediger Pluem wrote:

> >
> > I would say 0 is not a bad day. But if this is a bug it is an APR(- 
> > util) bug.
> > Thus I forward it to the apr dev list.
> 
> No, it is a bug in the expression.  A date is an unsigned 
> value and any

But apr_time_t is a signed 64 bit integer. So it would be possible to
define -1 as the flag for an invalid date. I agree that a date itself
is an unsigned value, but this does not make 0 an invalid date per se or
is there a definition somewhere that the first valid date is 1 second
after the Unix epoch?

> error is specifically forced to 0 so that the comparison

Just curious: Is the Unix epoch an invalid date in the Expires header
(as this in the past it does not really matter for the question whether
this document is expired or not as it would be in both cases)?


Regards

Rüdiger


Re: Mod_cache expires check

Posted by "Roy T. Fielding" <fi...@gbiv.com>.
On Jan 17, 2007, at 12:23 PM, Ruediger Pluem wrote:
> On 01/15/2007 01:56 PM, Bart van der Schans wrote:
>> In r463496 the following check was added to mod_cache.c :
>>
>>     else if (exp != APR_DATE_BAD && exp < r->request_time)
>>     {
>>         /* if a Expires header is in the past, don't cache it */
>>         reason = "Expires header already expired, not cacheable";
>>     }
>>
>> This check fails to correctly identify the expires header "Thu, 01  
>> Jan
>> 1970 00:00:00 GMT". The apr_date_parse_http function(exps) returns
>> (apr_time_t)0 which is equal to APR_DATE_BAD, but it should  
>> recognize it
>> as an already expired header. Is there a way to distinct between
>> APR_DATE_BAD and the unix epoch? Or is that considered a bad date?
>
> I would say 0 is not a bad day. But if this is a bug it is an APR(- 
> util) bug.
> Thus I forward it to the apr dev list.

No, it is a bug in the expression.  A date is an unsigned value and any
error is specifically forced to 0 so that the comparison

    if (exp < r->request_time)

will be true if exp == APR_DATE_BAD (as is desired in this case).

....Roy


Re: Mod_cache expires check

Posted by Bart van der Schans <sc...@hippo.nl>.
Ruediger Pluem wrote:
> 
> 
> Providing a better reference to the patch you are talking about would be a start :-).

Of course, and now when I'm trying to find Davi's mail from the 18th of 
January in the archive it seems to be missing, so maybe it didn't even 
make it to the list :( So here is his patch again.

The original problem was described in:
http://mail-archives.apache.org/mod_mbox/httpd-dev/200701.mbox/%3c45AB7A0E.5050409@hippo.nl%3e

Regards,

Bart




Re: Mod_cache expires check

Posted by Ruediger Pluem <rp...@apache.org>.

On 03/03/2007 06:08 PM, Bart van der Schans wrote:
> Bart van der Schans wrote:
> 
>> Davi Arnaut wrote:
>>  >
>>  > Looking at it more, the previous check it's also useless. Attempted
>> patch...
>>
>> I finally had some time to test the patch and it seems to work
>> correctly. It still recognizes Unix epoch as a bad date, but mod_cache
>> won't cache it.
>>
> 
> Is there any change the patch from Davi will make it in the trunk (and
> hopefully backported to the branch)? What is the correct way to move
> forward with this? Can I help in some way?

Providing a better reference to the patch you are talking about would be a start :-).
Seriously, in general you are correct in bugging us here and the help you can
offer varies from case to case (testing, providing documentation, rewriting the patch
according to review remarks, etc.)

Regards

Rüdiger


Re: Mod_cache expires check

Posted by Bart van der Schans <sc...@hippo.nl>.
Bart van der Schans wrote:

> Is there any change the patch from Davi will make it in the trunk (and 

That should have read chance of course, sorry about the typo.

Bart

Re: Mod_cache expires check

Posted by Bart van der Schans <sc...@hippo.nl>.
Bart van der Schans wrote:
> Davi Arnaut wrote:
>  >
>  > Looking at it more, the previous check it's also useless. Attempted 
> patch...
> 
> I finally had some time to test the patch and it seems to work 
> correctly. It still recognizes Unix epoch as a bad date, but mod_cache 
> won't cache it.
> 

Is there any change the patch from Davi will make it in the trunk (and 
hopefully backported to the branch)? What is the correct way to move 
forward with this? Can I help in some way?

Regards,

Bart

Re: Mod_cache expires check

Posted by Bart van der Schans <sc...@hippo.nl>.
Davi Arnaut wrote:
 >
 > Looking at it more, the previous check it's also useless. Attempted 
patch...

I finally had some time to test the patch and it seems to work 
correctly. It still recognizes Unix epoch as a bad date, but mod_cache 
won't cache it.

Regards,

Bart

Re: Mod_cache expires check

Posted by Davi Arnaut <da...@haxent.com.br>.
Ruediger Pluem wrote:
> 
> On 01/15/2007 01:56 PM, Bart van der Schans wrote:
>> In r463496 the following check was added to mod_cache.c :
>>
>>     else if (exp != APR_DATE_BAD && exp < r->request_time)
>>     {
>>         /* if a Expires header is in the past, don't cache it */
>>         reason = "Expires header already expired, not cacheable";
>>     }
>>
>> This check fails to correctly identify the expires header "Thu, 01 Jan
>> 1970 00:00:00 GMT". The apr_date_parse_http function(exps) returns
>> (apr_time_t)0 which is equal to APR_DATE_BAD, but it should recognize it
>> as an already expired header. Is there a way to distinct between
>> APR_DATE_BAD and the unix epoch? Or is that considered a bad date?
> 
> I would say 0 is not a bad day. But if this is a bug it is an APR(-util) bug.
> Thus I forward it to the apr dev list.
> 
> Regards
> 
> Rüdiger
> 

Looking at it more, the previous check it's also useless. Attempted patch...

--
Davi Arnaut

Re: Mod_cache expires check

Posted by "Roy T. Fielding" <fi...@gbiv.com>.
On Jan 17, 2007, at 12:23 PM, Ruediger Pluem wrote:
> On 01/15/2007 01:56 PM, Bart van der Schans wrote:
>> In r463496 the following check was added to mod_cache.c :
>>
>>     else if (exp != APR_DATE_BAD && exp < r->request_time)
>>     {
>>         /* if a Expires header is in the past, don't cache it */
>>         reason = "Expires header already expired, not cacheable";
>>     }
>>
>> This check fails to correctly identify the expires header "Thu, 01  
>> Jan
>> 1970 00:00:00 GMT". The apr_date_parse_http function(exps) returns
>> (apr_time_t)0 which is equal to APR_DATE_BAD, but it should  
>> recognize it
>> as an already expired header. Is there a way to distinct between
>> APR_DATE_BAD and the unix epoch? Or is that considered a bad date?
>
> I would say 0 is not a bad day. But if this is a bug it is an APR(- 
> util) bug.
> Thus I forward it to the apr dev list.

No, it is a bug in the expression.  A date is an unsigned value and any
error is specifically forced to 0 so that the comparison

    if (exp < r->request_time)

will be true if exp == APR_DATE_BAD (as is desired in this case).

....Roy