From: Graham Leggett Date: Tue, 28 May 2013 21:32:01 +0000 (+0000) Subject: mod_cache: If a 304 response indicates an entity not currently cached, then X-Git-Tag: 2.4.5~194 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=923bc0ef5fdc9adac5338dafd522d113eeb98dc1;p=apache mod_cache: If a 304 response indicates an entity not currently cached, then the cache MUST disregard the response and repeat the request without the conditional. trunk patch: http://svn.apache.org/r1481197 2.4.x patch: http://people.apache.org/~minfrin/httpd-mod_cache-uncacheable304-2.4.patch Submitted by: minfrin Reviewed by: jim, wrowe git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1487131 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/CHANGES b/CHANGES index ca2b33a862..3b6e851714 100644 --- a/CHANGES +++ b/CHANGES @@ -2,6 +2,10 @@ Changes with Apache 2.4.5 + *) mod_cache: If a 304 response indicates an entity not currently cached, then + the cache MUST disregard the response and repeat the request without the + conditional. [Graham Leggett, Co-Advisor ] + *) mod_cache: Ensure that we don't attempt to replace a cached response with an older response as per RFC2616 13.12. [Graham Leggett, Co-Advisor ] diff --git a/STATUS b/STATUS index 9eba1de70b..54ebb9d301 100644 --- a/STATUS +++ b/STATUS @@ -90,13 +90,6 @@ RELEASE SHOWSTOPPERS: PATCHES ACCEPTED TO BACKPORT FROM TRUNK: [ start all new proposals below, under PATCHES PROPOSED. ] - * mod_cache: If a 304 response indicates an entity not currently cached, then - the cache MUST disregard the response and repeat the request without the - conditional. - trunk patch: http://svn.apache.org/r1481197 - 2.4.x patch: http://people.apache.org/~minfrin/httpd-mod_cache-uncacheable304-2.4.patch - +1: minfrin, jim, wrowe - PATCHES PROPOSED TO BACKPORT FROM TRUNK: [ New proposals should be added at the end of the list ] diff --git a/modules/cache/mod_cache.c b/modules/cache/mod_cache.c index 15fc7e86e0..513448a583 100644 --- a/modules/cache/mod_cache.c +++ b/modules/cache/mod_cache.c @@ -1160,69 +1160,46 @@ static apr_status_t cache_save_filter(ap_filter_t *f, apr_bucket_brigade *in) } /* Hold the phone. Some servers might allow us to cache a 2xx, but - * then make their 304 responses non cacheable. This leaves us in a - * sticky position. If the 304 is in answer to our own conditional - * request, we cannot send this 304 back to the client because the - * client isn't expecting it. Instead, our only option is to respect - * the answer to the question we asked (has it changed, answer was - * no) and return the cached item to the client, and then respect - * the uncacheable nature of this 304 by allowing the remove_url - * filter to kick in and remove the cached entity. + * then make their 304 responses non cacheable. RFC2616 says this: + * + * If a 304 response indicates an entity not currently cached, then + * the cache MUST disregard the response and repeat the request + * without the conditional. + * + * A 304 response with contradictory headers is technically a + * different entity, to be safe, we remove the entity from the cache. */ - if (reason && r->status == HTTP_NOT_MODIFIED && - cache->stale_handle) { - apr_bucket_brigade *bb; - apr_bucket *bkt; - int status; - - cache->handle = cache->stale_handle; - info = &cache->handle->cache_obj->info; - - /* Load in the saved status and clear the status line. */ - r->status = info->status; - r->status_line = NULL; - - bb = apr_brigade_create(r->pool, r->connection->bucket_alloc); - - r->headers_in = cache->stale_headers; - r->headers_out = ap_cache_cacheable_headers_out(r); + if (reason && r->status == HTTP_NOT_MODIFIED && cache->stale_handle) { - /* Merge in our cached headers. However, keep any updated values. */ - /* take output, overlay on top of cached */ - cache_accept_headers(cache->handle, r, r->headers_out, - cache->handle->resp_hdrs, 1); + ap_log_rerror( + APLOG_MARK, APLOG_INFO, 0, r, APLOGNO() "cache: %s responded with an uncacheable 304, retrying the request. Reason: %s", r->unparsed_uri, reason); - status = ap_meets_conditions(r); - if (status != OK) { - r->status = status; + /* we've got a cache conditional miss! tell anyone who cares */ + cache_run_cache_status(cache->handle, r, r->headers_out, AP_CACHE_MISS, + apr_psprintf(r->pool, + "conditional cache miss: 304 was uncacheable, entity removed: %s", + reason)); - bkt = apr_bucket_flush_create(bb->bucket_alloc); - APR_BRIGADE_INSERT_TAIL(bb, bkt); - } - else { - cache->provider->recall_body(cache->handle, r->pool, bb); + /* remove the cached entity immediately, we might cache it again */ + ap_remove_output_filter(cache->remove_url_filter); + cache_remove_url(cache, r); - bkt = apr_bucket_eos_create(bb->bucket_alloc); - APR_BRIGADE_INSERT_TAIL(bb, bkt); - } + /* let someone else attempt to cache */ + cache_remove_lock(conf, cache, r, NULL); - cache->block_response = 1; + /* remove this filter from the chain */ + ap_remove_output_filter(f); - /* we've got a cache conditional hit! tell anyone who cares */ - cache_run_cache_status( - cache->handle, - r, - r->headers_out, - AP_CACHE_REVALIDATE, - apr_psprintf( - r->pool, - "conditional cache hit: 304 was uncacheable though (%s); entity removed", - reason)); + /* retry without the conditionals */ + 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"); - /* let someone else attempt to cache */ - cache_remove_lock(conf, cache, r, NULL); + ap_internal_redirect(r->uri, r); - return ap_pass_brigade(f->next, bb); + return APR_SUCCESS; } if (reason) {