Mailing-List: contact cvs-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r1487108 - in /httpd/httpd/branches/2.4.x: CHANGES
 modules/cache/cache_storage.c modules/cache/cache_storage.h
 modules/cache/mod_cache.c
Date: Tue, 28 May 2013 20:49:52 -0000
To: cvs@httpd.apache.org
From: minfrin@apache.org
Message-Id: <20130528204952.BA42223889F7@eris.apache.org>

Author: minfrin
Date: Tue May 28 20:49:52 2013
New Revision: 1487108

URL: http://svn.apache.org/r1487108
Log:
mod_cache: When serving from cache, only the last header of a multivalued
header was taken into account. Fixed. Ensure that Warning headers are correctly
handled as per RFC2616.

trunk patch: http://svn.apache.org/r1478441
             http://svn.apache.org/r1480283
2.4.x patch: http://people.apache.org/~minfrin/httpd-mod_cache-multipleheaders2.4.patch

Submitted by: minfrin
Reviewed by: jim, wrowe

Modified:
    httpd/httpd/branches/2.4.x/CHANGES
    httpd/httpd/branches/2.4.x/modules/cache/cache_storage.c
    httpd/httpd/branches/2.4.x/modules/cache/cache_storage.h
    httpd/httpd/branches/2.4.x/modules/cache/mod_cache.c

Modified: httpd/httpd/branches/2.4.x/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/CHANGES?rev=1487108&r1=1487107&r2=1487108&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/CHANGES [utf-8] (original)
+++ httpd/httpd/branches/2.4.x/CHANGES [utf-8] Tue May 28 20:49:52 2013
@@ -2,6 +2,10 @@
 
 Changes with Apache 2.4.5
 
+  *) mod_cache: When serving from cache, only the last header of a multivalued
+     header was taken into account. Fixed. Ensure that Warning headers are
+     correctly handled as per RFC2616. [Graham Leggett]
+
   *) mod_cache: Ignore response headers specified by no-cache=header and
      private=header as specified by RFC2616 14.9.1 What is Cacheable. Ensure
      that these headers are still processed when multiple Cache-Control

Modified: httpd/httpd/branches/2.4.x/modules/cache/cache_storage.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/cache/cache_storage.c?rev=1487108&r1=1487107&r2=1487108&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/modules/cache/cache_storage.c (original)
+++ httpd/httpd/branches/2.4.x/modules/cache/cache_storage.c Tue May 28 20:49:52 2013
@@ -113,26 +113,69 @@ int cache_create_entity(cache_request_re
     return DECLINED;
 }
 
-static int set_cookie_doo_doo(void *v, const char *key, const char *val)
+static int filter_header_do(void *v, const char *key, const char *val)
+{
+    if ((*key == 'W' || *key == 'w') && !strcasecmp(key, "Warning")
+            && *val == '1') {
+        /* any stored Warning headers with warn-code 1xx (see section
+         * 14.46) MUST be deleted from the cache entry and the forwarded
+         * response.
+         */
+    }
+    else {
+        apr_table_addn(v, key, val);
+    }
+    return 1;
+}
+static int remove_header_do(void *v, const char *key, const char *val)
+{
+    if ((*key == 'W' || *key == 'w') && !strcasecmp(key, "Warning")) {
+        /* any stored Warning headers with warn-code 2xx MUST be retained
+         * in the cache entry and the forwarded response.
+         */
+    }
+    else {
+        apr_table_unset(v, key);
+    }
+    return 1;
+}
+static int add_header_do(void *v, const char *key, const char *val)
 {
     apr_table_addn(v, key, val);
     return 1;
 }
 
 /**
- * Take headers from the cache, and overlap them over the existing response
- * headers.
+ * Take two sets of headers, sandwich them together, and apply the result to
+ * r->headers_out.
+ *
+ * To complicate this, a header may be duplicated in either table. Should a
+ * header exist in the top table, all matching headers will be removed from
+ * the bottom table before the headers are combined. The Warning headers are
+ * handled specially. Warnings are added rather than being replaced, while
+ * in the case of revalidation 1xx Warnings are stripped.
+ *
+ * The Content-Type and Last-Modified headers are then re-parsed and inserted
+ * into the request.
  */
-void cache_accept_headers(cache_handle_t *h, request_rec *r,
-        int preserve_orig)
+void cache_accept_headers(cache_handle_t *h, request_rec *r, apr_table_t *top,
+        apr_table_t *bottom, int revalidation)
 {
-    apr_table_t *cookie_table, *hdr_copy;
     const char *v;
 
-    v = apr_table_get(h->resp_hdrs, "Content-Type");
+    if (revalidation) {
+        r->headers_out = apr_table_make(r->pool, 10);
+        apr_table_do(filter_header_do, r->headers_out, bottom, NULL);
+    }
+    else if (r->headers_out != bottom) {
+        r->headers_out = apr_table_copy(r->pool, bottom);
+    }
+    apr_table_do(remove_header_do, r->headers_out, top, NULL);
+    apr_table_do(add_header_do, r->headers_out, top, NULL);
+
+    v = apr_table_get(r->headers_out, "Content-Type");
     if (v) {
         ap_set_content_type(r, v);
-        apr_table_unset(h->resp_hdrs, "Content-Type");
         /*
          * Also unset possible Content-Type headers in r->headers_out and
          * r->err_headers_out as they may be different to what we have received
@@ -149,39 +192,12 @@ void cache_accept_headers(cache_handle_t
     /* If the cache gave us a Last-Modified header, we can't just
      * pass it on blindly because of restrictions on future values.
      */
-    v = apr_table_get(h->resp_hdrs, "Last-Modified");
+    v = apr_table_get(r->headers_out, "Last-Modified");
     if (v) {
         ap_update_mtime(r, apr_date_parse_http(v));
         ap_set_last_modified(r);
-        apr_table_unset(h->resp_hdrs, "Last-Modified");
     }
 
-    /* The HTTP specification says that it is legal to merge duplicate
-     * headers into one.  Some browsers that support Cookies don't like
-     * merged headers and prefer that each Set-Cookie header is sent
-     * separately.  Lets humour those browsers by not merging.
-     * Oh what a pain it is.
-     */
-    cookie_table = apr_table_make(r->pool, 2);
-    apr_table_do(set_cookie_doo_doo, cookie_table, r->err_headers_out,
-                 "Set-Cookie", NULL);
-    apr_table_do(set_cookie_doo_doo, cookie_table, h->resp_hdrs,
-                 "Set-Cookie", NULL);
-    apr_table_unset(r->err_headers_out, "Set-Cookie");
-    apr_table_unset(h->resp_hdrs, "Set-Cookie");
-
-    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);
-    }
 }
 
 /*
@@ -362,7 +378,7 @@ int cache_select(cache_request_rec *cach
             }
 
             /* Okay, this response looks okay.  Merge in our stuff and go. */
-            cache_accept_headers(h, r, 0);
+            cache_accept_headers(h, r, h->resp_hdrs, r->headers_out, 0);
 
             cache->handle = h;
             return OK;

Modified: httpd/httpd/branches/2.4.x/modules/cache/cache_storage.h
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/cache/cache_storage.h?rev=1487108&r1=1487107&r2=1487108&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/modules/cache/cache_storage.h (original)
+++ httpd/httpd/branches/2.4.x/modules/cache/cache_storage.h Tue May 28 20:49:52 2013
@@ -61,11 +61,12 @@ apr_status_t cache_generate_key_default(
  * 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.
+ * @param top headers to be applied
+ * @param bottom headers to be overwritten
+ * @param revalidation true if revalidation is taking place
  */
-void cache_accept_headers(cache_handle_t *h, request_rec *r,
-        int preserve_orig);
+void cache_accept_headers(cache_handle_t *h, request_rec *r, apr_table_t *top,
+        apr_table_t *bottom, int revalidation);
 
 #ifdef __cplusplus
 }

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=1487108&r1=1487107&r2=1487108&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 20:49:52 2013
@@ -1090,6 +1090,13 @@ static apr_status_t cache_save_filter(ap
         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);
+
+        /* 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;
@@ -1098,20 +1105,6 @@ static apr_status_t cache_save_filter(ap
             APR_BRIGADE_INSERT_TAIL(bb, bkt);
         }
         else {
-            /* RFC 2616 10.3.5 states that entity headers are not supposed
-             * to be in the 304 response.  Therefore, we need to combine the
-             * response headers with 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 = ap_cache_cacheable_headers_out(r);
-
-            /* Merge in our cached headers.  However, keep any updated values. */
-            cache_accept_headers(cache->handle, r, 1);
-
             cache->provider->recall_body(cache->handle, r->pool, bb);
 
             bkt = apr_bucket_eos_create(bb->bucket_alloc);
@@ -1382,7 +1375,9 @@ static apr_status_t cache_save_filter(ap
         r->headers_out = ap_cache_cacheable_headers_out(r);
 
         /* Merge in our cached headers.  However, keep any updated values. */
-        cache_accept_headers(cache->handle, r, 1);
+        /* take output, overlay on top of cached */
+        cache_accept_headers(cache->handle, r, r->headers_out,
+                cache->handle->resp_hdrs, 1);
     }
 
     /* Write away header information to cache. It is possible that we are