httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From CASTELLE Thomas <tcaste...@generali.fr>
Subject RE: [PATCH] mod_cache RFC compliance
Date Thu, 07 Aug 2003 14:04:20 GMT
Hello Paul (or whoever can answer me... ;-)),

I just wanted to know why the patch you committed concerning mod_cache
hasn't been introduced in Apache 2.0.47 ? Will it be in 2.0.48 ? And
concerning the over mod_cache RFC violations, is there any news ? I can't
see anything on the cvs.apache.org/http2.0 website, but maybe I'm not
looking in the right place...

Best regards,

Thomas.

-----Message d'origine-----
De : Paul J. Reder [mailto:rederpj@remulak.net]
Envoyé : mardi 24 juin 2003 14:32
À : dev@httpd.apache.org
Objet : Re: [PATCH] mod_cache RFC compliance


Thomas,

I'm working on getting a patch in to CVS. The answer to question 1 is
that, depending on how it got added, the various values could be in
either err_headers_out or headers_out. I'm adjusting your patch (plus
the other header references) accordingly. Should be done, tested, and
committed soon.

Thanks for the kickstart on this.

Paul J. Reder

CASTELLE Thomas wrote:
> Thanks for looking into this Paul !
> 
> Concerning the second question, I totally agree with you. I tested it 
> and it works. It is obviously more logical...
> 
> I hope you will be able to integrate this patch in the next apache 
> release...
> 
> Thanks again,
> 
> Thomas.
> 
>  
> 
> -----Message d'origine-----
> De : Paul J. Reder [mailto:rederpj@remulak.net]
> Envoyé : mardi 17 juin 2003 05:21
> À : dev@httpd.apache.org
> Objet : Re: [PATCH] mod_cache RFC compliance
> 
> 
> I am reviewing this patch and have a few questions for Thomas or someone
> in the know.
> 
> The first has to do with Thomas' observation that Cache-Control is to be
> found in r->err_headers_out rather than in r->headers_out... I looked into
> this and ran into the following piece of code in mod_expires.c 
> (expires_filter):
> 
>      /*
>       * Check to see which output header table we should use;
>       * mod_cgi loads script fields into r->err_headers_out,
>       * for instance.
>       */
>      expiry = apr_table_get(r->err_headers_out, "Expires");
>      if (expiry != NULL) {
>          t = r->err_headers_out;
>      }
>      else {
>          expiry = apr_table_get(r->headers_out, "Expires");
>          t = r->headers_out;
>      }
> 
> This code later calls set_expiration_fields which adds Cache-Control
> to whichever headers t points to.
> 
> Question 1:
> Does this imply that the cache code needs to look for ETags,
Cache-Control,
> etc in both places too? Right now the code all looks in r->headers_out.
> Thomas is changing some of them to look in r->err_headers_out instead.
> Is there a rule of thumb for determining when to check headers_out vs.
> err_headers_out?
> 
> 
> 
> The second observation is related to the freshness computation referenced
> below. I agree with Thomas that the code as it stands isn't quite right,
> but I disagree that negating the logic fixes it. If I understand the
> RFC correctly (section 13.2.4), max_age and s-maxage take precedence over
> an expires header. This would mean that, as Thomas points out, if an
> expires header is more liberal than the max_age value then it currently
> passes the freshness test even though it should fail.
> 
> Question 2:
> Am I correct in my belief that the fix requires seperation of the checks
> so that the maxage and s-maxage checks/computations happen first and you
> only evaluate freshness using the expires value if there are no max-age or
> s-maxage values present? (as in the following code)
> 
>      if (((smaxage != -1) && (age < (smaxage - minfresh))) ||
>          ((maxage  != -1) && (age < (maxage + maxstale - minfresh))) ||
>          ((smaxage == -1) && (maxage == -1) &&
>           (info->expire != APR_DATE_BAD) &&
>           (age < (apr_time_sec(info->expire - info->date) + maxstale - 
> minfresh)))) {
>          /* it's fresh darlings... */
>          ...
>      }
> 
> In other words, if a value is specified for smaxage or maxage, the 
> freshness
> is determined solely based on those values (without regard to any expires
> header). If there are no values specified for either maxage or smaxage
then
> the freshness is determined based on the expires header (if it exists). Is
> that correct?
> 
> By the way, mod_disk_cache has a lot of issues. If it isn't saving and
> retrieving the headers correctly, its a bug in the disk_cache code that
> needs to be fixed. I wouldn't look to mod_disk_cache as an example of
> how the caching code should be working. :)
> 
> 
> Thanks in advance for any insight that Thomas or anyone else has to offer
> on these questions. Given a couple of answers I can have something
> committed Tuesday. Thanks for the research and patch Thomas.
> 
> 
> CASTELLE Thomas wrote:
>  > Hello,
>  >
>  > In order to accelerate the RFC compliance of mod_cache, I propose these
>  > two patches which fix two problems :
>  > - It doesn't handle the Cache-Control directives (max-age, max-stale,
>  > min-fresh...) properly.
>  > - It doesn't send a "If-Modified-Since" to avoid that the backend
server
>  > sends every time a 200 response where a 304 would be enough.
>  >
>  > Actually, we are waiting for these features to be implemented since
>  > http-2.0.43 so that we could put Apache in our production environment.
I
>  > am not an Apache developper, so this is just a proposition, but I
tested
>  > it and it seemed to work.
>  >
>  >
>  > The cache_util.c patch deals with the first issue. First, the
>  > Cache-Control directive seems to be in the r->err_headers_out and not
in
>  > the r->headers_out. Second, the following test seems useless :
>  >
>  >         if ((-1 < smaxage && age < (smaxage - minfresh)) ||
>  >           (-1 < maxage && age < (maxage + maxstale - minfresh)) ||
>  >           (info->expire != APR_DATE_BAD &&
>  >            age < (apr_time_sec(info->expire - info->date) + maxstale -
>  > minfresh))) {
>  >
>  > because it is always true, no matter if max-age is set or not.
>  > Let's take an example (I suppose here s-maxage, max-stale and min-fresh
>  > not set,
>  > so smaxage = - 1, maxstale = 0 and minfresh = 0) :
>  > - with age = 20, maxage = -1 (not set) and expire - date = 30, the
>  > second test
>  > is FALSE. The third is TRUE. So the whole test is TRUE, the page is
>  > considered
>  > to be fresh => no problem.
>  > - with age = 20, maxage = 10 and expire - date = 30, the second test is
>  > FALSE,
>  > but the third is still TRUE. So the whole test is TRUE, the page is
>  > considered
>  > to be fresh => problem.
>  >
>  >
>  > The mod_cache.c patch deals with the second issue. The info structure
is
>  > never initialized, and even if it was, the info->lastmods and
info->etag
>  > don't seem to be saved in the file when using mod_disk_cache. So I used
>  > the Etag and Last-Modified informations we can find in the
>  > r->headers_out and r->err_headers_out. I don't know if it's correct,
but
>  > it seems to work now...
>  >
>  > Thanks for looking to these patch and eventually integrate it in the
>  > next Apache release !
>  >
>  > Thanks a lot for this really great product !
>  >
>  >
>  > Thomas.
> 
> -- 
> Paul J. Reder
> -----------------------------------------------------------
> "The strength of the Constitution lies entirely in the determination of 
> each
> citizen to defend it.  Only if every single citizen feels duty bound to do
> his share in this defense are the constitutional rights secure."
> -- Albert Einstein
> 

-- 
Paul J. Reder
-----------------------------------------------------------
"The strength of the Constitution lies entirely in the determination of each
citizen to defend it.  Only if every single citizen feels duty bound to do
his share in this defense are the constitutional rights secure."
-- Albert Einstein


Mime
View raw message