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.

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1481197 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index 778aa6dd2a05bf10598709e6c5808fe187526956..01527d902013ed11c4cd6ea5922051834349d290 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,10 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.5.0
 
+  *) 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 <coad measurement-factory.com>]
+
   *) mod_proxy: Fix seg-faults when using the global pool on threaded
      MPMs [Thomas Eckert <thomas.r.w.eckert gmail.com>, Jim Jagielski]
 

diff --git a/modules/cache/mod_cache.c b/modules/cache/mod_cache.c
index 5451d8113cf123e6b22fe3eb22e1d3353fb4ce98..2272de173497bcb7575c979c921167a53a499f40 100644 (file)
--- 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) {