On 18 Oct 2010, at 4:41 PM, Plüm, Rüdiger, VF-Group wrote:

Ok, I missed that. Thanks for the pointer.
But shouldn't we remove the following code from cache_check_freshness then:

   /* This value comes from the client's initial request. */
   cc_req = apr_table_get(r->headers_in, "Cache-Control");
   pragma = apr_table_get(r->headers_in, "Pragma");

ap_cache_control(r, &cache->control_in, cc_req, pragma, r- >headers_in);

The above still needs to be there, as we use other information from the request (like max-age) in the freshness check below.

ap_cache_control() has a check to ensure that it only parses the header once, so calling it a second time isn't a problem.

   if (cache->control_in.no_cache) {

       if (!conf->ignorecachecontrol) {
           /* Treat as stale, causing revalidation */
           return 0;
       }

       ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r,
               "Incoming request is asking for a uncached version of "
               "%s, but we have been configured to ignore it and "
               "serve a cached response anyway",
               r->unparsed_uri);
   }

In theory, the above is caught by ap_cache_check_allowed(), so doesn't do anything, on condition we always call ap_cache_check_allowed() before running cache_check_freshness().

Yeah, the patch below makes the behaviour consistent and makes sense.

Will commit it.

I've found some more stuff that can be optimised in cache_check_freshness(), will look at this in more detail tonight.

Regards,
Graham
--

Reply via email to