On 10/17/2010 05:57 PM, Graham Leggett wrote:
> On 17 Oct 2010, at 2:29 PM, Ruediger Pluem wrote:
>
>> Are you sure? If the client said Cache-Control: no-cache, we IMHO treat
>> the cached entry as stale and revalidate:
>>
>> From cache_util.c:
>>
>> /* 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");
>>
>>
>> if (ap_cache_liststr(NULL, pragma, "no-cache", NULL)
>> || ap_cache_liststr(NULL, cc_req, "no-cache", NULL)) {
>>
>> if (!conf->ignorecachecontrol) {
>> /* Treat as stale, causing revalidation */
>> return 0;
>> }
>>
>> ap_log_error(APLOG_MARK, APLOG_INFO, 0, r->server,
>> "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);
>> }
>
> 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:
/* Is our cached response fresh enough? */
fresh = cache_check_freshness(h, cache, r);
if (!fresh) {
const char *etag, *lastmod;
/* Cache-Control: only-if-cached and revalidation required, try
* the next provider
*/
if (cache->control_in.only_if_cached) {
/* try again with next cache type */
list = list->next;
continue;
}
/* set aside the stale entry for accessing later */
cache->stale_headers = apr_table_copy(r->pool,
r->headers_in);
cache->stale_handle = h;
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, APR_SUCCESS, r,
"Cached response for %s isn't fresh. Adding/replacing "
"conditional request headers.", r->uri);
/* We can only revalidate with our own conditionals: remove the
* conditions from the original request.
*/
apr_table_unset(r->headers_in, "If-Match");
apr_table_unset(r->headers_in, "If-Modified-Since");
apr_table_unset(r->headers_in, "If-None-Match");
apr_table_unset(r->headers_in, "If-Range");
apr_table_unset(r->headers_in, "If-Unmodified-Since");
etag = apr_table_get(h->resp_hdrs, "ETag");
lastmod = apr_table_get(h->resp_hdrs, "Last-Modified");
>
> What we do differently, is that instead of invalidating the existing
> content with our 5xx response to the no-cache request, we leave it alone
> completely, so that the next request, where the client hasn't asked for
> no-cache, can be served the stale content, instead of an error because
> the stale content had been blown away by the previous no-cache request.
I understand this purpose and I do not question it. It makes a lot of sense.
>
>>> existing file at all, and cache->stale_handle would always be NULL.
>>
>> Even then remains the question: Why doing it differently in both
>> case (ap_die, mod_proxy)?
>
> We do it the same way in both cases, the remove_url filter is removed in
> the ap_die() case on line 1630, and in the mod_proxy case inside
> save_filter on line 791.
Sorry for being a PITA, but IMHO we do not:
URL:
http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/cache/mod_cache.c?rev=1021946&r1=1021945&r2=1021946&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/cache/mod_cache.c (original)
+++ httpd/httpd/trunk/modules/cache/mod_cache.c Tue Oct 12 22:54:06 2010
@@ -779,6 +779,61 @@ static int cache_save_filter(ap_filter_t
dconf = ap_get_module_config(r->per_dir_config, &cache_module);
+ /* RFC2616 13.8 Errors or Incomplete Response Cache Behavior:
+ * If a cache receives a 5xx response while attempting to revalidate an
+ * entry, it MAY either forward this response to the requesting client,
+ * or act as if the server failed to respond. In the latter case, it MAY
+ * return a previously received response unless the cached entry
+ * includes the "must-revalidate" cache-control directive (see section
+ * 14.9).
+ *
+ * This covers the case where an error was generated behind us, for example
+ * by a backend server via mod_proxy.
+ */
+ if (dconf->stale_on_error && r->status >= HTTP_INTERNAL_SERVER_ERROR) {
+
+ ap_remove_output_filter(cache->remove_url_filter);
+
+ if (cache->stale_handle && !ap_cache_liststr(
+ NULL,
+ apr_table_get(cache->stale_handle->resp_hdrs, "Cache-Control"),
+ "must-revalidate", NULL)) {
+ const char *warn_head;
Modified: httpd/httpd/trunk/modules/cache/mod_cache.c
URL:
http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/cache/mod_cache.c?rev=1021546&r1=1021545&r2=1021546&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/cache/mod_cache.c (original)
+++ httpd/httpd/trunk/modules/cache/mod_cache.c Mon Oct 11 23:32:56 2010
+ /* RFC2616 13.8 Errors or Incomplete Response Cache Behavior:
+ * If a cache receives a 5xx response while attempting to revalidate an
+ * entry, it MAY either forward this response to the requesting client,
+ * or act as if the server failed to respond. In the latter case, it MAY
+ * return a previously received response unless the cached entry
+ * includes the "must-revalidate" cache-control directive (see section
+ * 14.9).
+ */
+ apr_pool_userdata_get(&dummy, CACHE_CTX_KEY, r->pool);
+ if (dummy) {
+ cache_request_rec *cache = (cache_request_rec *) dummy;
+
+ if (cache->stale_handle && cache->save_filter && !ap_cache_liststr(
+ NULL, apr_table_get(cache->stale_handle->resp_hdrs,
+ "Cache-Control"), "must-revalidate", NULL)) {
+ const char *warn_head;
+
+ /* morph the current save filter into the out filter, and serve
from
+ * cache.
+ */
+ cache->handle = cache->stale_handle;
+ if (r->main) {
+ cache->save_filter->frec = cache_out_subreq_filter_handle;
+ }
+ else {
+ cache->save_filter->frec = cache_out_filter_handle;
+ }
+
+ r->output_filters = cache->save_filter;
+
+ r->err_headers_out = cache->stale_handle->resp_hdrs;
+
+ /* add a stale warning */
+ warn_head = apr_table_get(r->err_headers_out, "Warning");
+ if ((warn_head == NULL) || ((warn_head != NULL)
+ && (ap_strstr_c(warn_head, "110") == NULL))) {
+ apr_table_mergen(r->err_headers_out, "Warning",
+ "110 Response is stale");
+ }
+
+ ap_remove_output_filter(cache->remove_url_filter);
+
+
Please compare the code. In the mod_proxy case we call
ap_remove_output_filter(cache->remove_url_filter);
if (dconf->stale_on_error && r->status >= HTTP_INTERNAL_SERVER_ERROR)
In the ap_die case we require
+ if (cache->stale_handle && cache->save_filter && !ap_cache_liststr(
+ NULL, apr_table_get(cache->stale_handle->resp_hdrs,
+ "Cache-Control"), "must-revalidate", NULL)) {
+
These additional conditions are required in the second if in the mod_proxy case.
Regards
Rüdiger
