mod_cache: Ensure that updated responses to HEAD requests don't get

mistakenly paired with a previously cached body. Ensure that any existing
body is removed when a HEAD request is cached.

trunk patch: http://svn.apache.org/r1479411

Submitted by: minfrin
Reviewed by: jim, wrowe

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1487122 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index f6d5ffd8ac2171695b801388c7d313b31bcc6782..ffca0d791df8e9ae7402b4c6c794920753d950c5 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -2,6 +2,11 @@
 
 Changes with Apache 2.4.5
 
+  *) mod_cache: Ensure that updated responses to HEAD requests don't get
+     mistakenly paired with a previously cached body. Ensure that any existing
+     body is removed when a HEAD request is cached. [Graham Leggett,
+     Co-Advisor <coad measurement-factory.com>]
+
   *) mod_cache: Honour Cache-Control: no-store in a request. [Graham Leggett]
 
   *) mod_cache: Make sure that contradictory entity headers present in a 304

diff --git a/STATUS b/STATUS
index cc920ea9cc8da124958ba4a4e87ab25d03dc426a..999826c716f70cb718744e06fbee06f93c47e492 100644 (file)
--- 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: Ensure that updated responses to HEAD requests don't get
-      mistakenly paired with a previously cached body. Ensure that any existing
-      body is removed when a HEAD request is cached.
-      trunk patch: http://svn.apache.org/r1479411
-      2.4.x patch: trunk patch works (minus CHANGES)
-      +1: minfrin, jim, wrowe
-
     * core: Add the ability to do explicit matching on weak and strong ETags
       as per RFC2616 Section 13.3.3.
       trunk patch: http://svn.apache.org/r1479528

diff --git a/modules/cache/mod_cache.c b/modules/cache/mod_cache.c
index 9258ebd56466dcf7fee3068868f49d4f94639500..f47eeae5872127246019c72b0aa7d11e1f930ebd 100644 (file)
--- a/modules/cache/mod_cache.c
+++ b/modules/cache/mod_cache.c
@@ -1435,9 +1435,6 @@ static apr_status_t cache_save_filter(ap_filter_t *f, apr_bucket_brigade *in)
 
     /* We found a stale entry which wasn't really stale. */
     if (cache->stale_handle) {
-        /* Load in the saved status and clear the status line. */
-        r->status = info->status;
-        r->status_line = NULL;
 
         /* RFC 2616 10.3.5 states that entity headers are not supposed
          * to be in the 304 response.  Therefore, we need to combine the
@@ -1476,6 +1473,10 @@ static apr_status_t cache_save_filter(ap_filter_t *f, apr_bucket_brigade *in)
         apr_bucket *bkt;
         int status;
 
+        /* Load in the saved status and clear the status line. */
+        r->status = info->status;
+        r->status_line = NULL;
+
         /* We're just saving response headers, so we are done. Commit
          * the response at this point, unless there was a previous error.
          */

diff --git a/modules/cache/mod_cache_disk.c b/modules/cache/mod_cache_disk.c
index b0d86fe1ffd2efde74fb6392f82cc5ed9662d8c3..a5699482ae8fd6386856656b1d0e783cf424f641 100644 (file)
--- a/modules/cache/mod_cache_disk.c
+++ b/modules/cache/mod_cache_disk.c
@@ -940,6 +940,10 @@ static apr_status_t store_headers(cache_handle_t *h, request_rec *r, cache_info
         dobj->headers_in = ap_cache_cacheable_headers_in(r);
     }
 
+    if (r->header_only && r->status != HTTP_NOT_MODIFIED) {
+        dobj->disk_info.header_only = 1;
+    }
+
     return APR_SUCCESS;
 }
 
@@ -1188,49 +1192,51 @@ static apr_status_t store_body(cache_handle_t *h, request_rec *r,
             continue;
         }
 
-        /* Attempt to create the data file at the last possible moment, if
-         * the body is empty, we don't write a file at all, and save an inode.
-         */
-        if (!dobj->data.tempfd) {
-            apr_finfo_t finfo;
-            rv = apr_file_mktemp(&dobj->data.tempfd, dobj->data.tempfile,
-                                 APR_CREATE | APR_WRITE | APR_BINARY |
-                                 APR_BUFFERED | APR_EXCL, dobj->data.pool);
+        if (!dobj->disk_info.header_only) {
+
+            /* Attempt to create the data file at the last possible moment, if
+             * the body is empty, we don't write a file at all, and save an inode.
+             */
+            if (!dobj->data.tempfd) {
+                apr_finfo_t finfo;
+                rv = apr_file_mktemp(&dobj->data.tempfd, dobj->data.tempfile,
+                        APR_CREATE | APR_WRITE | APR_BINARY | APR_BUFFERED
+                                | APR_EXCL, dobj->data.pool);
+                if (rv != APR_SUCCESS) {
+                    apr_pool_destroy(dobj->data.pool);
+                    return rv;
+                }
+                dobj->file_size = 0;
+                rv = apr_file_info_get(&finfo, APR_FINFO_IDENT,
+                        dobj->data.tempfd);
+                if (rv != APR_SUCCESS) {
+                    apr_pool_destroy(dobj->data.pool);
+                    return rv;
+                }
+                dobj->disk_info.device = finfo.device;
+                dobj->disk_info.inode = finfo.inode;
+                dobj->disk_info.has_body = 1;
+            }
+
+            /* write to the cache, leave if we fail */
+            rv = apr_file_write_full(dobj->data.tempfd, str, length, &written);
             if (rv != APR_SUCCESS) {
+                ap_log_rerror(
+                        APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(00731) "Error when writing cache file for URL %s", h->cache_obj->key);
+                /* Remove the intermediate cache file and return non-APR_SUCCESS */
                 apr_pool_destroy(dobj->data.pool);
                 return rv;
             }
-            dobj->file_size = 0;
-            rv = apr_file_info_get(&finfo, APR_FINFO_IDENT,
-                    dobj->data.tempfd);
-            if (rv != APR_SUCCESS) {
+            dobj->file_size += written;
+            if (dobj->file_size > dconf->maxfs) {
+                ap_log_rerror(
+                        APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(00732) "URL %s failed the size check "
+                        "(%" APR_OFF_T_FMT ">%" APR_OFF_T_FMT ")", h->cache_obj->key, dobj->file_size, dconf->maxfs);
+                /* Remove the intermediate cache file and return non-APR_SUCCESS */
                 apr_pool_destroy(dobj->data.pool);
-                return rv;
+                return APR_EGENERAL;
             }
-            dobj->disk_info.device = finfo.device;
-            dobj->disk_info.inode = finfo.inode;
-            dobj->disk_info.has_body = 1;
-        }
 
-        /* write to the cache, leave if we fail */
-        rv = apr_file_write_full(dobj->data.tempfd, str, length, &written);
-        if (rv != APR_SUCCESS) {
-            ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, APLOGNO(00731)
-                    "Error when writing cache file for URL %s",
-                    h->cache_obj->key);
-            /* Remove the intermediate cache file and return non-APR_SUCCESS */
-            apr_pool_destroy(dobj->data.pool);
-            return rv;
-        }
-        dobj->file_size += written;
-        if (dobj->file_size > dconf->maxfs) {
-            ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(00732)
-                    "URL %s failed the size check "
-                    "(%" APR_OFF_T_FMT ">%" APR_OFF_T_FMT ")",
-                    h->cache_obj->key, dobj->file_size, dconf->maxfs);
-            /* Remove the intermediate cache file and return non-APR_SUCCESS */
-            apr_pool_destroy(dobj->data.pool);
-            return APR_EGENERAL;
         }
 
         /* have we reached the limit of how much we're prepared to write in one
@@ -1256,43 +1262,44 @@ static apr_status_t store_body(cache_handle_t *h, request_rec *r,
     if (seen_eos) {
         const char *cl_header = apr_table_get(r->headers_out, "Content-Length");
 
-        if (dobj->data.tempfd) {
-            rv = apr_file_close(dobj->data.tempfd);
-            if (rv != APR_SUCCESS) {
-                /* Buffered write failed, abandon attempt to write */
-                apr_pool_destroy(dobj->data.pool);
-                return rv;
+        if (!dobj->disk_info.header_only) {
+
+            if (dobj->data.tempfd) {
+                rv = apr_file_close(dobj->data.tempfd);
+                if (rv != APR_SUCCESS) {
+                    /* Buffered write failed, abandon attempt to write */
+                    apr_pool_destroy(dobj->data.pool);
+                    return rv;
+                }
             }
-        }
 
-        if (r->connection->aborted || r->no_cache) {
-            ap_log_rerror(APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00733)
-                    "Discarding body for URL %s "
-                    "because connection has been aborted.",
-                    h->cache_obj->key);
-            /* Remove the intermediate cache file and return non-APR_SUCCESS */
-            apr_pool_destroy(dobj->data.pool);
-            return APR_EGENERAL;
-        }
-        if (dobj->file_size < dconf->minfs) {
-            ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(00734)
-                    "URL %s failed the size check "
-                    "(%" APR_OFF_T_FMT "<%" APR_OFF_T_FMT ")",
-                    h->cache_obj->key, dobj->file_size, dconf->minfs);
-            /* Remove the intermediate cache file and return non-APR_SUCCESS */
-            apr_pool_destroy(dobj->data.pool);
-            return APR_EGENERAL;
-        }
-        if (cl_header) {
-            apr_int64_t cl = apr_atoi64(cl_header);
-            if ((errno == 0) && (dobj->file_size != cl)) {
-                ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(00735)
-                        "URL %s didn't receive complete response, not caching",
-                        h->cache_obj->key);
+            if (r->connection->aborted || r->no_cache) {
+                ap_log_rerror(
+                        APLOG_MARK, APLOG_INFO, 0, r, APLOGNO(00733) "Discarding body for URL %s "
+                        "because connection has been aborted.", h->cache_obj->key);
                 /* Remove the intermediate cache file and return non-APR_SUCCESS */
                 apr_pool_destroy(dobj->data.pool);
                 return APR_EGENERAL;
             }
+            if (dobj->file_size < dconf->minfs) {
+                ap_log_rerror(
+                        APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(00734) "URL %s failed the size check "
+                        "(%" APR_OFF_T_FMT "<%" APR_OFF_T_FMT ")", h->cache_obj->key, dobj->file_size, dconf->minfs);
+                /* Remove the intermediate cache file and return non-APR_SUCCESS */
+                apr_pool_destroy(dobj->data.pool);
+                return APR_EGENERAL;
+            }
+            if (cl_header) {
+                apr_int64_t cl = apr_atoi64(cl_header);
+                if ((errno == 0) && (dobj->file_size != cl)) {
+                    ap_log_rerror(
+                            APLOG_MARK, APLOG_DEBUG, 0, r, APLOGNO(00735) "URL %s didn't receive complete response, not caching", h->cache_obj->key);
+                    /* Remove the intermediate cache file and return non-APR_SUCCESS */
+                    apr_pool_destroy(dobj->data.pool);
+                    return APR_EGENERAL;
+                }
+            }
+
         }
 
         /* All checks were fine, we're good to go when the commit comes */
@@ -1319,7 +1326,12 @@ static apr_status_t commit_entity(cache_handle_t *h, request_rec *r)
         rv = file_cache_el_final(conf, &dobj->vary, r);
     }
     if (APR_SUCCESS == rv) {
-        rv = file_cache_el_final(conf, &dobj->data, r);
+        if (!dobj->disk_info.header_only) {
+            rv = file_cache_el_final(conf, &dobj->data, r);
+        }
+        else if (dobj->data.file){
+            rv = apr_file_remove(dobj->data.file, dobj->data.pool);
+        }
     }
 
     /* remove the cached items completely on any failure */

diff --git a/modules/cache/mod_cache_socache.c b/modules/cache/mod_cache_socache.c
index fb3fea948aad073224ea52130d6e3a0ebfb9aad5..e6a4c4d992a9a868e73e564582dafd2980225a99 100644 (file)
--- a/modules/cache/mod_cache_socache.c
+++ b/modules/cache/mod_cache_socache.c
@@ -879,7 +879,13 @@ static apr_status_t store_headers(cache_handle_t *h, request_rec *r,
     socache_info->request_time = obj->info.request_time;
     socache_info->response_time = obj->info.response_time;
     socache_info->status = obj->info.status;
-    socache_info->header_only = r->header_only;
+
+    if (r->header_only && r->status != HTTP_NOT_MODIFIED) {
+        socache_info->header_only = 1;
+    }
+    else {
+        socache_info->header_only = sobj->socache_info.header_only;
+    }
 
     socache_info->name_len = strlen(sobj->name);