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 22:34:12 UTC

svn commit: r1487103 - in /httpd/httpd/branches/2.4.x: ./ CHANGES STATUS modules/cache/cache_util.c modules/cache/cache_util.h modules/cache/mod_cache.c

Author: minfrin
Date: Tue May 28 20:34:12 2013
New Revision: 1487103

URL: http://svn.apache.org/r1487103
Log:
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
headers are present in the response. PR 54706

trunk patch: http://svn.apache.org/r1478382
2.4.x patch: http://people.apache.org/~minfrin/httpd-mod_cache-nocacheheader2.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/cache_util.c
    httpd/httpd/branches/2.4.x/modules/cache/cache_util.h
    httpd/httpd/branches/2.4.x/modules/cache/mod_cache.c

Propchange: httpd/httpd/branches/2.4.x/
------------------------------------------------------------------------------
  Merged /httpd/httpd/trunk:r1478382

Modified: httpd/httpd/branches/2.4.x/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/CHANGES?rev=1487103&r1=1487102&r2=1487103&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:34:12 2013
@@ -2,6 +2,12 @@
 
 Changes with Apache 2.4.5
 
+  *) 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
+     headers are present in the response. PR 54706 [Graham Leggett,
+     Yann Ylavic <ylavic.dev gmail.com>]
+
   *) mod_cache: Invalidate cached entities in response to RFC2616 Section
      13.10 Invalidation After Updates or Deletions. PR 15868 [Graham
      Leggett]

Modified: httpd/httpd/branches/2.4.x/STATUS
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/STATUS?rev=1487103&r1=1487102&r2=1487103&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/STATUS (original)
+++ httpd/httpd/branches/2.4.x/STATUS Tue May 28 20:34:12 2013
@@ -90,14 +90,6 @@ RELEASE SHOWSTOPPERS:
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]
  
-    * 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
-      headers are present in the response. PR 54706
-      trunk patch: http://svn.apache.org/r1478382
-      2.4.x patch: http://people.apache.org/~minfrin/httpd-mod_cache-nocacheheader2.4.patch
-      +1: minfrin, jim, wrowe
-
     * 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.

Modified: httpd/httpd/branches/2.4.x/modules/cache/cache_util.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/cache/cache_util.c?rev=1487103&r1=1487102&r2=1487103&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/modules/cache/cache_util.c (original)
+++ httpd/httpd/branches/2.4.x/modules/cache/cache_util.c Tue May 28 20:34:12 2013
@@ -543,8 +543,6 @@ int cache_check_freshness(cache_handle_t
 
     /* These come from the cached entity. */
     if (h->cache_obj->info.control.no_cache
-            || h->cache_obj->info.control.no_cache_header
-            || h->cache_obj->info.control.private_header
             || h->cache_obj->info.control.invalidated) {
         /*
          * The cached entity contained Cache-Control: no-cache, or a
@@ -860,95 +858,6 @@ CACHE_DECLARE(char *)ap_cache_generate_n
     return apr_pstrdup(p, hashfile);
 }
 
-/*
- * Create a new table consisting of those elements from an
- * headers table that are allowed to be stored in a cache.
- */
-CACHE_DECLARE(apr_table_t *)ap_cache_cacheable_headers(apr_pool_t *pool,
-                                                        apr_table_t *t,
-                                                        server_rec *s)
-{
-    cache_server_conf *conf;
-    char **header;
-    int i;
-    apr_table_t *headers_out;
-
-    /* Short circuit the common case that there are not
-     * (yet) any headers populated.
-     */
-    if (t == NULL) {
-        return apr_table_make(pool, 10);
-    };
-
-    /* Make a copy of the headers, and remove from
-     * the copy any hop-by-hop headers, as defined in Section
-     * 13.5.1 of RFC 2616
-     */
-    headers_out = apr_table_copy(pool, t);
-
-    apr_table_unset(headers_out, "Connection");
-    apr_table_unset(headers_out, "Keep-Alive");
-    apr_table_unset(headers_out, "Proxy-Authenticate");
-    apr_table_unset(headers_out, "Proxy-Authorization");
-    apr_table_unset(headers_out, "TE");
-    apr_table_unset(headers_out, "Trailers");
-    apr_table_unset(headers_out, "Transfer-Encoding");
-    apr_table_unset(headers_out, "Upgrade");
-
-    conf = (cache_server_conf *)ap_get_module_config(s->module_config,
-                                                     &cache_module);
-
-    /* Remove the user defined headers set with CacheIgnoreHeaders.
-     * This may break RFC 2616 compliance on behalf of the administrator.
-     */
-    header = (char **)conf->ignore_headers->elts;
-    for (i = 0; i < conf->ignore_headers->nelts; i++) {
-        apr_table_unset(headers_out, header[i]);
-    }
-    return headers_out;
-}
-
-/*
- * Create a new table consisting of those elements from an input
- * headers table that are allowed to be stored in a cache.
- */
-CACHE_DECLARE(apr_table_t *)ap_cache_cacheable_headers_in(request_rec *r)
-{
-    return ap_cache_cacheable_headers(r->pool, r->headers_in, r->server);
-}
-
-/*
- * Create a new table consisting of those elements from an output
- * headers table that are allowed to be stored in a cache;
- * ensure there is a content type and capture any errors.
- */
-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);
-
-    if (!apr_table_get(headers_out, "Content-Type")
-        && r->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) {
-        apr_table_setn(headers_out, "Content-Encoding",
-                       r->content_encoding);
-    }
-
-    return headers_out;
-}
-
 /**
  * String tokenizer that ignores separator characters within quoted strings
  * and escaped characters, as per RFC2616 section 2.2.
@@ -981,7 +890,7 @@ static char *cache_strqtok(char *str, co
     *last = token;
     while (**last) {
         if (!quoted) {
-            if (**last == '\"') {
+            if (**last == '\"' && !ap_strchr_c(sep, '\"')) {
                 quoted = 1;
                 ++*last;
             }
@@ -1071,9 +980,7 @@ int ap_cache_control(request_rec *r, cac
                 /* ...then try slowest cases */
                 else if (!strncasecmp(token, "no-cache", 8)) {
                     if (token[8] == '=') {
-                        if (apr_table_get(headers, token + 9)) {
-                            cc->no_cache_header = 1;
-                        }
+                        cc->no_cache_header = 1;
                     }
                     else if (!token[8]) {
                         cc->no_cache = 1;
@@ -1148,9 +1055,7 @@ int ap_cache_control(request_rec *r, cac
                 }
                 else if (!strncasecmp(token, "private", 7)) {
                     if (token[7] == '=') {
-                        if (apr_table_get(headers, token + 8)) {
-                            cc->private_header = 1;
-                        }
+                        cc->private_header = 1;
                     }
                     else if (!token[7]) {
                         cc->private = 1;
@@ -1181,3 +1086,209 @@ int ap_cache_control(request_rec *r, cac
 
     return (cc_header != NULL || pragma_header != NULL);
 }
+
+/**
+ * Parse the Cache-Control, identifying and removing headers that
+ * exist as tokens after the no-cache and private tokens.
+ */
+static int cache_control_remove(request_rec *r, const char *cc_header,
+        apr_table_t *headers)
+{
+    char *last, *slast;
+    int found = 0;
+
+    if (cc_header) {
+        char *header = apr_pstrdup(r->pool, cc_header);
+        char *token = cache_strqtok(header, CACHE_SEPARATOR, &last);
+        while (token) {
+            switch (token[0]) {
+            case 'n':
+            case 'N': {
+                if (!strncmp(token, "no-cache", 8)
+                        || !strncasecmp(token, "no-cache", 8)) {
+                    if (token[8] == '=') {
+                        const char *header = cache_strqtok(token + 9,
+                                CACHE_SEPARATOR "\"", &slast);
+                        while (header) {
+                            apr_table_unset(headers, header);
+                            header = cache_strqtok(NULL, CACHE_SEPARATOR "\"",
+                                    &slast);
+                        }
+                        found = 1;
+                    }
+                    break;
+                }
+                break;
+            }
+            case 'p':
+            case 'P': {
+                if (!strncmp(token, "private", 7)
+                        || !strncasecmp(token, "private", 7)) {
+                    if (token[7] == '=') {
+                        const char *header = cache_strqtok(token + 8,
+                                CACHE_SEPARATOR "\"", &slast);
+                        while (header) {
+                            apr_table_unset(headers, header);
+                            header = cache_strqtok(NULL, CACHE_SEPARATOR "\"",
+                                    &slast);
+                        }
+                        found = 1;
+                    }
+                }
+                break;
+            }
+            }
+            token = cache_strqtok(NULL, CACHE_SEPARATOR, &last);
+        }
+    }
+
+    return found;
+}
+
+/*
+ * Create a new table consisting of those elements from an
+ * headers table that are allowed to be stored in a cache.
+ */
+CACHE_DECLARE(apr_table_t *)ap_cache_cacheable_headers(apr_pool_t *pool,
+                                                        apr_table_t *t,
+                                                        server_rec *s)
+{
+    cache_server_conf *conf;
+    char **header;
+    int i;
+    apr_table_t *headers_out;
+
+    /* Short circuit the common case that there are not
+     * (yet) any headers populated.
+     */
+    if (t == NULL) {
+        return apr_table_make(pool, 10);
+    };
+
+    /* Make a copy of the headers, and remove from
+     * the copy any hop-by-hop headers, as defined in Section
+     * 13.5.1 of RFC 2616
+     */
+    headers_out = apr_table_copy(pool, t);
+
+    apr_table_unset(headers_out, "Connection");
+    apr_table_unset(headers_out, "Keep-Alive");
+    apr_table_unset(headers_out, "Proxy-Authenticate");
+    apr_table_unset(headers_out, "Proxy-Authorization");
+    apr_table_unset(headers_out, "TE");
+    apr_table_unset(headers_out, "Trailers");
+    apr_table_unset(headers_out, "Transfer-Encoding");
+    apr_table_unset(headers_out, "Upgrade");
+
+    conf = (cache_server_conf *)ap_get_module_config(s->module_config,
+                                                     &cache_module);
+
+    /* Remove the user defined headers set with CacheIgnoreHeaders.
+     * This may break RFC 2616 compliance on behalf of the administrator.
+     */
+    header = (char **)conf->ignore_headers->elts;
+    for (i = 0; i < conf->ignore_headers->nelts; i++) {
+        apr_table_unset(headers_out, header[i]);
+    }
+    return headers_out;
+}
+
+/*
+ * Create a new table consisting of those elements from an input
+ * headers table that are allowed to be stored in a cache.
+ */
+CACHE_DECLARE(apr_table_t *)ap_cache_cacheable_headers_in(request_rec *r)
+{
+    return ap_cache_cacheable_headers(r->pool, r->headers_in, r->server);
+}
+
+/*
+ * Create a new table consisting of those elements from an output
+ * headers table that are allowed to be stored in a cache;
+ * ensure there is a content type and capture any errors.
+ */
+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);
+
+    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) {
+        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) {
+        apr_table_setn(headers_out, "Content-Encoding",
+                       r->content_encoding);
+    }
+
+    return headers_out;
+}
+
+typedef struct
+{
+    apr_pool_t *p;
+    const char *first;
+    apr_array_header_t *merged;
+} cache_table_getm_t;
+
+static int cache_table_getm_do(void *v, const char *key, const char *val)
+{
+    cache_table_getm_t *state = (cache_table_getm_t *) v;
+
+    if (!state->first) {
+        /**
+         * The most common case is a single header, and this is covered by
+         * a fast path that doesn't allocate any memory. On the second and
+         * subsequent header, an array is created and the array concatenated
+         * together to form the final value.
+         */
+        state->first = val;
+    }
+    else {
+        const char **elt;
+        if (!state->merged) {
+            state->merged = apr_array_make(state->p, 10, sizeof(const char *));
+            elt = apr_array_push(state->merged);
+            *elt = state->first;
+        }
+        elt = apr_array_push(state->merged);
+        *elt = val;
+    }
+    return 1;
+}
+
+const char *cache_table_getm(apr_pool_t *p, const apr_table_t *t,
+        const char *key)
+{
+    cache_table_getm_t state;
+
+    state.p = p;
+    state.first = NULL;
+    state.merged = NULL;
+
+    apr_table_do(cache_table_getm_do, &state, t, key, NULL);
+
+    if (!state.first) {
+        return NULL;
+    }
+    else if (!state.merged) {
+        return state.first;
+    }
+    else {
+        return apr_array_pstrcat(p, state.merged, ',');
+    }
+}

Modified: httpd/httpd/branches/2.4.x/modules/cache/cache_util.h
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/modules/cache/cache_util.h?rev=1487103&r1=1487102&r2=1487103&view=diff
==============================================================================
--- httpd/httpd/branches/2.4.x/modules/cache/cache_util.h (original)
+++ httpd/httpd/branches/2.4.x/modules/cache/cache_util.h Tue May 28 20:34:12 2013
@@ -292,6 +292,19 @@ apr_status_t cache_remove_lock(cache_ser
 cache_provider_list *cache_get_providers(request_rec *r,
         cache_server_conf *conf, apr_uri_t uri);
 
+/**
+ * Get a value from a table, where the table may contain multiple
+ * values for a given key.
+ *
+ * When the table contains a single value, that value is returned
+ * unchanged.
+ *
+ * When the table contains two or more values for a key, all values
+ * for the key are returned, separated by commas.
+ */
+const char *cache_table_getm(apr_pool_t *p, const apr_table_t *t,
+        const char *key);
+
 #ifdef __cplusplus
 }
 #endif

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=1487103&r1=1487102&r2=1487103&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:34:12 2013
@@ -918,12 +918,12 @@ static apr_status_t cache_save_filter(ap
     if (etag == NULL) {
         etag = apr_table_get(r->headers_out, "Etag");
     }
-    cc_out = apr_table_get(r->err_headers_out, "Cache-Control");
-    pragma = apr_table_get(r->err_headers_out, "Pragma");
+    cc_out = cache_table_getm(r->pool, r->err_headers_out, "Cache-Control");
+    pragma = cache_table_getm(r->pool, r->err_headers_out, "Pragma");
     headers = r->err_headers_out;
     if (!cc_out && !pragma) {
-        cc_out = apr_table_get(r->headers_out, "Cache-Control");
-        pragma = apr_table_get(r->headers_out, "Pragma");
+        cc_out = cache_table_getm(r->pool, r->headers_out, "Cache-Control");
+        pragma = cache_table_getm(r->pool, r->headers_out, "Pragma");
         headers = r->headers_out;
     }
 
@@ -932,8 +932,10 @@ static apr_status_t cache_save_filter(ap
      */
     if (r->status == HTTP_NOT_MODIFIED && cache->stale_handle && !cc_out
             && !pragma) {
-        cc_out = apr_table_get(cache->stale_handle->resp_hdrs, "Cache-Control");
-        pragma = apr_table_get(cache->stale_handle->resp_hdrs, "Pragma");
+        cc_out = cache_table_getm(r->pool, cache->stale_handle->resp_hdrs,
+                "Cache-Control");
+        pragma = cache_table_getm(r->pool, cache->stale_handle->resp_hdrs,
+                "Pragma");
     }
 
     /* Parse the cache control header */