More work to properly handle revalidated responses correctly.

* modules/cache/mod_cache.c: If we add a new Expires value, tell our client;
  merge in headers properly (or better than before) so that we can update the
  header fields on a revalidated but with updated header fields.
* modules/cache/mod_cache.h, modules/cache/cache_storage.c:
  Add preserve_orig flag to ap_cache_accept_headers to allow updating of fields.
* modules/cache/mod_disk_cache.c: Load status value from disk.

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

diff --git a/modules/cache/cache_storage.c b/modules/cache/cache_storage.c
index a6bd26b82f3b2cf25142c8f26a25fe40e16318a7..de091bca1e000dbe53a222cfdfaf300dbfc4107a 100644 (file)
--- a/modules/cache/cache_storage.c
+++ b/modules/cache/cache_storage.c
@@ -105,9 +105,10 @@ static int set_cookie_doo_doo(void *v, const char *key, const char *val)
     return 1;
 }
 
-CACHE_DECLARE(void) ap_cache_accept_headers(cache_handle_t *h, request_rec *r)
+CACHE_DECLARE(void) ap_cache_accept_headers(cache_handle_t *h, request_rec *r,
+                                            int preserve_orig)
 {
-    apr_table_t *cookie_table;
+    apr_table_t *cookie_table, *hdr_copy;
     const char *v;
 
     v = apr_table_get(h->resp_hdrs, "Content-Type");
@@ -140,7 +141,14 @@ CACHE_DECLARE(void) ap_cache_accept_headers(cache_handle_t *h, request_rec *r)
     apr_table_unset(r->err_headers_out, "Set-Cookie");
     apr_table_unset(h->resp_hdrs, "Set-Cookie");
 
-    apr_table_overlap(r->headers_out, h->resp_hdrs, APR_OVERLAP_TABLES_SET);
+    if (preserve_orig) {
+        hdr_copy = apr_table_copy(r->pool, h->resp_hdrs);
+        apr_table_overlap(hdr_copy, r->headers_out, APR_OVERLAP_TABLES_SET);
+        r->headers_out = hdr_copy;
+    }
+    else {
+        apr_table_overlap(r->headers_out, h->resp_hdrs, APR_OVERLAP_TABLES_SET);
+    }
     if (!apr_is_empty_table(cookie_table)) {
         r->err_headers_out = apr_table_overlay(r->pool, r->err_headers_out,
                                                cookie_table);
@@ -274,7 +282,7 @@ int cache_select_url(request_rec *r, char *url)
             }
 
             /* Okay, this response looks okay.  Merge in our stuff and go. */
-            ap_cache_accept_headers(h, r);
+            ap_cache_accept_headers(h, r, 0);
 
             cache->handle = h;
             return OK;

diff --git a/modules/cache/mod_cache.c b/modules/cache/mod_cache.c
index 09c1d5c15353710538879bc4cb03f98053479585..494388d554cce6001b22406093e339c7c419318d 100644 (file)
--- a/modules/cache/mod_cache.c
+++ b/modules/cache/mod_cache.c
@@ -532,12 +532,6 @@ static int cache_save_filter(ap_filter_t *f, apr_bucket_brigade *in)
             /* Oh, hey.  It isn't that stale!  Yay! */
             cache->handle = cache->stale_handle;
             info = &cache->handle->cache_obj->info;
-            /* Load in the saved status. */
-            r->status = info->status;
-            /* The cached response will override our err_headers_out. */
-            apr_table_clear(r->err_headers_out);
-            /* Merge in our headers. */
-            ap_cache_accept_headers(cache->handle, r);
             rv = OK;
         }
         else {
@@ -636,6 +630,8 @@ static int cache_save_filter(ap_filter_t *f, apr_bucket_brigade *in)
      *      expire date = date + defaultexpire
      */
     if (exp == APR_DATE_BAD) {
+        char expire_hdr[APR_RFC822_DATE_LEN];
+
         /* if lastmod == date then you get 0*conf->factor which results in
          *   an expiration time of now. This causes some problems with
          *   freshness calculations, so we choose the else path...
@@ -647,19 +643,50 @@ static int cache_save_filter(ap_filter_t *f, apr_bucket_brigade *in)
                 x = conf->maxex;
             }
             exp = date + x;
+            apr_rfc822_date(expire_hdr, exp);
+            apr_table_set(r->headers_out, "Expires", expire_hdr);
         }
         else {
             exp = date + conf->defex;
+            apr_rfc822_date(expire_hdr, exp);
+            apr_table_set(r->headers_out, "Expires", expire_hdr);
         }
     }
     info->expire = exp;
 
-    /*
-     * Write away header information to cache.
-     */
+    /* 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 load in the
+         * cached headers before we update the cached headers.
+         *
+         * However, before doing that, we need to first merge in
+         * err_headers_out and we also need to strip any hop-by-hop
+         * headers that might have snuck in.
+         */
+        r->headers_out = apr_table_overlay(r->pool, r->headers_out,
+                                           r->err_headers_out);
+        r->headers_out = ap_cache_cacheable_hdrs_out(r->pool, r->headers_out,
+                                                     r->server);
+        apr_table_clear(r->err_headers_out);
+
+        /* Merge in our cached headers.  However, keep any updated values. */
+        ap_cache_accept_headers(cache->handle, r, 1);
+    }
+
+    /* Write away header information to cache. */
     rv = cache->provider->store_headers(cache->handle, r, info);
 
-    /* Did we actually find an entity before, but it wasn't really stale? */
+    /* Did we just update the cached headers on a revalidated response?
+     *
+     * If so, we can now decide what to serve to the client:
+     * - If the original request was conditional and is satisified, send 304.
+     * - Otherwise, send the cached body.
+    */
     if (rv == APR_SUCCESS && cache->stale_handle) {
         apr_bucket_brigade *bb;
         apr_bucket *bkt;
@@ -668,14 +695,15 @@ static int cache_save_filter(ap_filter_t *f, apr_bucket_brigade *in)
 
         /* Were we initially a conditional request? */
         if (ap_cache_request_is_conditional(cache->stale_headers)) {
-            /* FIXME: Should we now go and make sure it's really not
-             * modified since what the user thought?
-             */
+            /* FIXME: We must ensure that the request meets conditions. */
+
+            /* Set the status to be a 304. */
+            r->status = HTTP_NOT_MODIFIED;
+
             bkt = apr_bucket_flush_create(bb->bucket_alloc);
             APR_BRIGADE_INSERT_TAIL(bb, bkt);
         }
         else {
-            r->status = info->status;
             cache->provider->recall_body(cache->handle, r->pool, bb);
         }
 

diff --git a/modules/cache/mod_cache.h b/modules/cache/mod_cache.h
index 076c6e342d8620a866b0b6938b1e8ba6bb330fcd..554d0a7ebf38db8cfc0915326e024215a9bd2e0d 100644 (file)
--- a/modules/cache/mod_cache.h
+++ b/modules/cache/mod_cache.h
@@ -241,7 +241,15 @@ CACHE_DECLARE(apr_time_t) ap_cache_current_age(cache_info *info, const apr_time_
  */
 CACHE_DECLARE(int) ap_cache_check_freshness(cache_handle_t *h, request_rec *r);
 
-CACHE_DECLARE(void) ap_cache_accept_headers(cache_handle_t *h, request_rec *r);
+/**
+ * Merge in cached headers into the response
+ * @param h cache_handle_t
+ * @param r request_rec
+ * @param preserve_orig If 1, the values in r->headers_out are preserved.
+ *        Otherwise, they are overwritten by the cached value.
+ */
+CACHE_DECLARE(void) ap_cache_accept_headers(cache_handle_t *h, request_rec *r,
+                                            int preserve_orig);
 
 CACHE_DECLARE(apr_time_t) ap_cache_hex2usec(const char *x);
 CACHE_DECLARE(void) ap_cache_usec2hex(apr_time_t j, char *y);

diff --git a/modules/cache/mod_disk_cache.c b/modules/cache/mod_disk_cache.c
index e6a36716b855a520de276f2aa98c1ab0ffbe4500..efdb4a6d374c6bc92a04b5f827bb9d3edb40cf74 100644 (file)
--- a/modules/cache/mod_disk_cache.c
+++ b/modules/cache/mod_disk_cache.c
@@ -221,6 +221,7 @@ static int file_cache_recall_mydata(apr_file_t *fd, cache_info *info,
     /* Store it away so we can get it later. */
     dobj->disk_info = disk_info;
 
+    info->status = disk_info.status;
     info->date = disk_info.date;
     info->expire = disk_info.expire;
     info->request_time = disk_info.request_time;