You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by mi...@apache.org on 2013/05/28 23:32:02 UTC
svn commit: r1487131 - in /httpd/httpd/branches/2.4.x: ./ CHANGES STATUS
modules/cache/mod_cache.c
Author: minfrin
Date: Tue May 28 21:32:01 2013
New Revision: 1487131
URL: http://svn.apache.org/r1487131
Log:
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
Modified:
httpd/httpd/branches/2.4.x/ (props changed)
httpd/httpd/branches/2.4.x/CHANGES
httpd/httpd/branches/2.4.x/STATUS
httpd/httpd/branches/2.4.x/modules/cache/mod_cache.c
Propchange: httpd/httpd/branches/2.4.x/
------------------------------------------------------------------------------
Merged /httpd/httpd/trunk:r1481197
Modified: httpd/httpd/branches/2.4.x/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/CHANGES?rev=1487131&r1=1487130&r2=1487131&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/CHANGES [utf-8] (original)
+++ httpd/httpd/branches/2.4.x/CHANGES [utf-8] Tue May 28 21:32:01 2013
@@ -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 <coad measurement-factory.com>]
+
*) 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
<coad measurement-factory.com>]
Modified: httpd/httpd/branches/2.4.x/STATUS
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1487131&r1=1487130&r2=1487131&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/STATUS (original)
+++ httpd/httpd/branches/2.4.x/STATUS Tue May 28 21:32:01 2013
@@ -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 ]
Modified: httpd/httpd/branches/2.4.x/modules/cache/mod_cache.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/cache/mod_cache.c?rev=1487131&r1=1487130&r2=1487131&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/modules/cache/mod_cache.c (original)
+++ httpd/httpd/branches/2.4.x/modules/cache/mod_cache.c Tue May 28 21:32:01 2013
@@ -1160,69 +1160,46 @@ static apr_status_t cache_save_filter(ap
}
/* 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;
+ if (reason && r->status == HTTP_NOT_MODIFIED && cache->stale_handle) {
- /* 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);
+ 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);
- r->headers_in = cache->stale_headers;
- r->headers_out = ap_cache_cacheable_headers_out(r);
-
- /* 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);
-
- 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) {