> -----Original Message-----
> From: Graham Leggett
> Sent: Montag, 18. Oktober 2010 16:11
> To: [email protected]
> Subject: Re: svn commit: r1021946 -
> /httpd/httpd/trunk/modules/cache/mod_cache.c
>
> On 18 Oct 2010, at 8:36 AM, Ruediger Pluem wrote:
>
> >> If the client specified no-cache, the cache steps down
> completely,
> >> and
> >> the client is guaranteed fresh content from the source
> server (as per
> >> the RFC). The cache will only revalidate if you say max-age=X (or
> >> other
> >> valid caching tokens).
> >
> > I cannot follow that. The above code means that
> > cache_check_freshness returns
> > 0 and thus we replace the client supplied conditionals with our own:
>
> mod_cache used to do this, but not any more as it's an RFC
> violation.
> Now, if the client supplies no-cache, this is caught by a function
> called ap_cache_check_allowed(), which if not allowed, causes no
> attempt to be made to open or touch an existing cached file. Look
> further upwards in cache_select(), you'll see the call to
> ap_cache_check_allowed():
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);
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);
}
>
> So if I'm understanding you correctly, the issue is with the
> different
> handling of the check for "must-revalidate"?
Yes.
>
> Hmmm.
>
> If "must-revalidate" is present in the original cached response, in
> both cases, we don't serve the stale-cached-content, which is
> correct
> according to the definition of must-revalidate.
>
> This makes the invalidation of the stale response academic from the
> client's point of view, as this stale response, which contains the
> "must-revalidate", will never be served to the client while
> the error
> persists.
>
> This is however inconsistent as you've pointed out, and needs to be
> fixed.
>
> From the server's point of view, invalidating the cached
> entry means
> that when the server comes back, it will need to serve a 200
> response
> from scratch, and if our server is returning 5xx errors this is less
> than ideal. I think we should remove the remove_url filter in both
> cases, so that we're consistent, like below.
>
> Does this make sense?
Yeah, the patch below makes the behaviour consistent and makes sense.
Thanks for your patience and for explaining.
>
> Index: modules/cache/mod_cache.c
> ===================================================================
> --- modules/cache/mod_cache.c (revision 1023462)
> +++ modules/cache/mod_cache.c (working copy)
> @@ -1595,6 +1595,8 @@
> if (dummy) {
> cache_request_rec *cache = (cache_request_rec *) dummy;
>
> + ap_remove_output_filter(cache->remove_url_filter);
> +
> if (cache->stale_handle && cache->save_filter
> && !cache->stale_handle->cache_obj-
> >info.control.must_revalidate
> && !cache->stale_handle->cache_obj-
> >info.control.proxy_revalidate) {
> @@ -1627,8 +1629,6 @@
> "110 Response is stale");
> }
>
> - ap_remove_output_filter(cache->remove_url_filter);
> -
> cache_run_cache_status(
> cache->handle,
> r,
>
Regards
Rüdiger
