Merge r1591328, r1594643, r1594648 from trunk:

mod_cache: Preserve non-cacheable headers forwarded from an origin 304
           response. PR 55547.

When mod_cache asks for a revalidation of a stale entry and the origin responds
with a 304 (not that stale), the module strips the non-cacheable headers from
the origin response and merges the stale headers to update the cache.

The problem is that mod_cache won't forward the non-cacheable headers to the
client, for example if the 304 response contains both Set-Cookie and
'Cache-Control: no-cache="Set-Cookie"' headers, or CacheIgnoreHeaders is used.

mod_cache: follow up to r1591328.

Define the cache_merge_headers_out() function to merge r->err_headers_out into
r->headers_out and add the ones from r->content_type/encoding if available.
Use it in ap_cache_cacheable_headers_out() where the same is done and in
cache_save_filter() where this has to be done before updating the entry.

mod_cache: follow up to r1594643.

Avoid table lookup if not necessary (fast path first).

Submitted by: ylavic
Reviewed/backported by: jim

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

diff --git a/STATUS b/STATUS
index fb8a8b941b1458a8af30c020c90cbfe363003d8f..c33a3a9b87047494b0abc3f2ffb1bd4508e28389 100644 (file)
--- a/STATUS
+++ b/STATUS
@@ -100,14 +100,6 @@ RELEASE SHOWSTOPPERS:
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]
 
-   * mod_cache: Preserve non-cacheable headers forwarded from an origin 304
-                response. PR 55547.
-     trunk patch: http://svn.apache.org/r1591328
-                  http://svn.apache.org/r1594643
-                  http://svn.apache.org/r1594648
-     2.4.x patch: http://people.apache.org/~ylavic/httpd-2.4.x-mod_cache-preserve_304_headers.patch
-     +1: ylavic, minfrin, jim
-
    * minor proxy_util cleanups:
      . get rid of unnecessary block scope
      . Clarify an existing requirement of the server_portstr parameter

diff --git a/modules/cache/cache_util.c b/modules/cache/cache_util.c
index cafbc167816117170a0d9e7811d48b3ff855e1c8..bf45093192747ed1947fc817abc1dcffd005fcf5 100644 (file)
--- a/modules/cache/cache_util.c
+++ b/modules/cache/cache_util.c
@@ -1237,26 +1237,33 @@ CACHE_DECLARE(apr_table_t *)ap_cache_cacheable_headers_out(request_rec *r)
 {
     apr_table_t *headers_out;
 
-    headers_out = apr_table_overlay(r->pool, r->headers_out,
-                                        r->err_headers_out);
-
-    apr_table_clear(r->err_headers_out);
-
-    headers_out = ap_cache_cacheable_headers(r->pool, headers_out,
-                                                  r->server);
+    headers_out = ap_cache_cacheable_headers(r->pool,
+                                             cache_merge_headers_out(r),
+                                             r->server);
 
     cache_control_remove(r,
             cache_table_getm(r->pool, headers_out, "Cache-Control"),
             headers_out);
 
-    if (!apr_table_get(headers_out, "Content-Type")
-        && r->content_type) {
+    return headers_out;
+}
+
+apr_table_t *cache_merge_headers_out(request_rec *r)
+{
+    apr_table_t *headers_out;
+
+    headers_out = apr_table_overlay(r->pool, r->headers_out,
+                                    r->err_headers_out);
+    apr_table_clear(r->err_headers_out);
+
+    if (r->content_type
+            && !apr_table_get(headers_out, "Content-Type")) {
         apr_table_setn(headers_out, "Content-Type",
                        ap_make_content_type(r, r->content_type));
     }
 
-    if (!apr_table_get(headers_out, "Content-Encoding")
-        && r->content_encoding) {
+    if (r->content_encoding
+            && !apr_table_get(headers_out, "Content-Encoding")) {
         apr_table_setn(headers_out, "Content-Encoding",
                        r->content_encoding);
     }

diff --git a/modules/cache/cache_util.h b/modules/cache/cache_util.h
index 3a54fadd067f6604a935f5a24a90e94a66c7b25f..5f296bc7165513348234de307c882b2366036a86 100644 (file)
--- a/modules/cache/cache_util.h
+++ b/modules/cache/cache_util.h
@@ -321,6 +321,12 @@ const char *cache_table_getm(apr_pool_t *p, const apr_table_t *t,
  */
 char *cache_strqtok(char *str, const char *sep, char **last);
 
+/**
+ * Merge err_headers_out into headers_out and add request's Content-Type and
+ * Content-Encoding if available.
+ */
+apr_table_t *cache_merge_headers_out(request_rec *r);
+
 #ifdef __cplusplus
 }
 #endif

diff --git a/modules/cache/mod_cache.c b/modules/cache/mod_cache.c
index 8763078cebde0c334ec2a898ac51d05dad8d5056..245590f807c25d4e4d7a12f2cf053cf55a451083 100644 (file)
--- a/modules/cache/mod_cache.c
+++ b/modules/cache/mod_cache.c
@@ -1442,10 +1442,12 @@ static apr_status_t cache_save_filter(ap_filter_t *f, apr_bucket_brigade *in)
          * 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.
+         * err_headers_out (note that store_headers() below already selects
+         * the cacheable only headers using ap_cache_cacheable_headers_out(),
+         * here we want to keep the original headers in r->headers_out and
+         * forward all of them to the client, including non-cacheable ones).
          */
-        r->headers_out = ap_cache_cacheable_headers_out(r);
+        r->headers_out = cache_merge_headers_out(r);
 
         /* Merge in our cached headers.  However, keep any updated values. */
         /* take output, overlay on top of cached */