mod_cache: Use the actual URI path and query-string for identifying the

cached entity (key), such that rewrites are taken into account when
running afterwards (CacheQuickHandler off).  PR 21935.

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

diff --git a/CHANGES b/CHANGES
index 3b0b85eed8b04fd88b86336a17971176ada2921b..0a681c45fb34e10ce41db4ea405a7853ddc99b19 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,10 @@
                                                          -*- coding: utf-8 -*-
 Changes with Apache 2.5.0
 
+  *) mod_cache: Use the actual URI path and query-string for identifying the
+     cached entity (key), such that rewrites are taken into account when
+     running afterwards (CacheQuickHandler off).  PR 21935.  [Yann Ylavic]
+
   *) mod_ssl: Fix quick renegotiation (OptRenegotiaton) with no intermediate
      in the client certificate chain.  PR 55786.  [Yann Ylavic]
 

diff --git a/modules/cache/cache_storage.c b/modules/cache/cache_storage.c
index 217c1d74b44ad4981132f1cd3e0fb44ef1e64fc7..1a75cc1d814e3b895f27e1dda09ed9f8e9c3c6e9 100644 (file)
--- a/modules/cache/cache_storage.c
+++ b/modules/cache/cache_storage.c
@@ -427,7 +427,9 @@ int cache_select(cache_request_rec *cache, request_rec *r)
 }
 
 static apr_status_t cache_canonicalise_key(request_rec *r, apr_pool_t* p,
-        const char *uri, apr_uri_t *parsed_uri, const char **key)
+                                           const char *uri, const char *query,
+                                           apr_uri_t *parsed_uri,
+                                           const char **key)
 {
     cache_server_conf *conf;
     char *port_str, *hn, *lcs;
@@ -563,7 +565,7 @@ static apr_status_t cache_canonicalise_key(request_rec *r, apr_pool_t* p,
      * if needed.
      */
     path = uri;
-    querystring = parsed_uri->query;
+    querystring = apr_pstrdup(p, query ? query : parsed_uri->query);
     if (conf->ignore_session_id->nelts) {
         int i;
         char **identifier;
@@ -588,7 +590,7 @@ static apr_status_t cache_canonicalise_key(request_rec *r, apr_pool_t* p,
             /*
              * Check if the identifier is in the querystring and cut it out.
              */
-            if (querystring) {
+            if (querystring && *querystring) {
                 /*
                  * First check if the identifier is at the beginning of the
                  * querystring and followed by a '='
@@ -605,7 +607,7 @@ static apr_status_t cache_canonicalise_key(request_rec *r, apr_pool_t* p,
                      * identifier with a '&' and append a '='
                      */
                     complete = apr_pstrcat(p, "&", *identifier, "=", NULL);
-                    param = strstr(querystring, complete);
+                    param = ap_strstr_c(querystring, complete);
                     /* If we found something we are sitting on the '&' */
                     if (param) {
                         param++;
@@ -669,7 +671,11 @@ static apr_status_t cache_canonicalise_key(request_rec *r, apr_pool_t* p,
 apr_status_t cache_generate_key_default(request_rec *r, apr_pool_t* p,
         const char **key)
 {
-    return cache_canonicalise_key(r, p, r->uri, &r->parsed_uri, key);
+    /* We want the actual query-string, which may differ from
+     * r->parsed_uri.query (immutable), so use "" (not NULL).
+     */
+    const char *args = r->args ? r->args : "";
+    return cache_canonicalise_key(r, p, r->uri, args, &r->parsed_uri, key);
 }
 
 /*
@@ -709,12 +715,13 @@ int cache_invalidate(cache_request_rec *cache, request_rec *r)
 
     location = apr_table_get(r->headers_out, "Location");
     if (location) {
-        if (APR_SUCCESS != apr_uri_parse(r->pool, location, &location_uri)
-                || APR_SUCCESS
-                        != cache_canonicalise_key(r, r->pool, location,
-                                &location_uri, &location_key)
-                || !(r->parsed_uri.hostname && location_uri.hostname
-                        && !strcmp(r->parsed_uri.hostname,
+        if (apr_uri_parse(r->pool, location, &location_uri)
+                || cache_canonicalise_key(r, r->pool,
+                                          location, NULL,
+                                          &location_uri, &location_key)
+                || !(r->parsed_uri.hostname
+                     && location_uri.hostname
+                     && !strcmp(r->parsed_uri.hostname,
                                 location_uri.hostname))) {
             location_key = NULL;
         }
@@ -722,14 +729,15 @@ int cache_invalidate(cache_request_rec *cache, request_rec *r)
 
     content_location = apr_table_get(r->headers_out, "Content-Location");
     if (content_location) {
-        if (APR_SUCCESS
-                != apr_uri_parse(r->pool, content_location,
-                        &content_location_uri)
-                || APR_SUCCESS
-                        != cache_canonicalise_key(r, r->pool, content_location,
-                                &content_location_uri, &content_location_key)
-                || !(r->parsed_uri.hostname && content_location_uri.hostname
-                        && !strcmp(r->parsed_uri.hostname,
+        if (apr_uri_parse(r->pool, content_location,
+                          &content_location_uri)
+                || cache_canonicalise_key(r, r->pool,
+                                          content_location, NULL,
+                                          &content_location_uri,
+                                          &content_location_key)
+                || !(r->parsed_uri.hostname
+                     && content_location_uri.hostname
+                     && !strcmp(r->parsed_uri.hostname,
                                 content_location_uri.hostname))) {
             content_location_key = NULL;
         }

diff --git a/modules/cache/cache_util.c b/modules/cache/cache_util.c
index c349d10e645cdd4b295db880f71114e725ad9c75..096058308e037fa8f92be1ed31acdfa971383ae7 100644 (file)
--- a/modules/cache/cache_util.c
+++ b/modules/cache/cache_util.c
@@ -31,8 +31,9 @@ extern module AP_MODULE_DECLARE_DATA cache_module;
  * in "filter". All but the path comparisons are case-insensitive.
  */
 static int uri_meets_conditions(const apr_uri_t *filter, const int pathlen,
-                                const apr_uri_t *url)
+                                request_rec *r)
 {
+    const apr_uri_t *url = &r->parsed_uri;
 
     /* Scheme, hostname port and local part. The filter URI and the
      * URI we test may have the following shapes:
@@ -113,7 +114,7 @@ static int uri_meets_conditions(const apr_uri_t *filter, const int pathlen,
     /* For HTTP caching purposes, an empty (NULL) path is equivalent to
      * a single "/" path. RFCs 3986/2396
      */
-    if (!url->path) {
+    if (!r->uri) {
         if (*filter->path == '/' && pathlen == 1) {
             return 1;
         }
@@ -125,7 +126,7 @@ static int uri_meets_conditions(const apr_uri_t *filter, const int pathlen,
     /* Url has met all of the filter conditions so far, determine
      * if the paths match.
      */
-    return !strncmp(filter->path, url->path, pathlen);
+    return !strncmp(filter->path, r->uri, pathlen);
 }
 
 static cache_provider_list *get_provider(request_rec *r, struct cache_enable *ent,
@@ -167,8 +168,7 @@ static cache_provider_list *get_provider(request_rec *r, struct cache_enable *en
 }
 
 cache_provider_list *cache_get_providers(request_rec *r,
-        cache_server_conf *conf,
-        apr_uri_t uri)
+                                         cache_server_conf *conf)
 {
     cache_dir_conf *dconf = ap_get_module_config(r->per_dir_config, &cache_module);
     cache_provider_list *providers = NULL;
@@ -183,7 +183,7 @@ cache_provider_list *cache_get_providers(request_rec *r,
     for (i = 0; i < conf->cachedisable->nelts; i++) {
         struct cache_disable *ent =
                                (struct cache_disable *)conf->cachedisable->elts;
-        if (uri_meets_conditions(&ent[i].url, ent[i].pathlen, &uri)) {
+        if (uri_meets_conditions(&ent[i].url, ent[i].pathlen, r)) {
             /* Stop searching now. */
             return NULL;
         }
@@ -200,7 +200,7 @@ cache_provider_list *cache_get_providers(request_rec *r,
     for (i = 0; i < conf->cacheenable->nelts; i++) {
         struct cache_enable *ent =
                                 (struct cache_enable *)conf->cacheenable->elts;
-        if (uri_meets_conditions(&ent[i].url, ent[i].pathlen, &uri)) {
+        if (uri_meets_conditions(&ent[i].url, ent[i].pathlen, r)) {
             providers = get_provider(r, &ent[i], providers);
         }
     }

diff --git a/modules/cache/cache_util.h b/modules/cache/cache_util.h
index 21a8ca34033c328fddbbdc33ce6e81b4bb189dd1..441a71a7860b14a4423d83e231961079a5d88cc8 100644 (file)
--- a/modules/cache/cache_util.h
+++ b/modules/cache/cache_util.h
@@ -300,7 +300,7 @@ apr_status_t cache_remove_lock(cache_server_conf *conf,
         cache_request_rec *cache, request_rec *r, apr_bucket_brigade *bb);
 
 cache_provider_list *cache_get_providers(request_rec *r,
-        cache_server_conf *conf, apr_uri_t uri);
+                                         cache_server_conf *conf);
 
 /**
  * Get a value from a table, where the table may contain multiple

diff --git a/modules/cache/mod_cache.c b/modules/cache/mod_cache.c
index ab2d51dc99f937c6758dc40dac12e521d6d9ecd4..9cda2e9b170cf01675a6182d8f7970f5c3dc7789 100644 (file)
--- a/modules/cache/mod_cache.c
+++ b/modules/cache/mod_cache.c
@@ -103,7 +103,7 @@ static int cache_quick_handler(request_rec *r, int lookup)
     /*
      * Which cache module (if any) should handle this request?
      */
-    if (!(providers = cache_get_providers(r, conf, r->parsed_uri))) {
+    if (!(providers = cache_get_providers(r, conf))) {
         return DECLINED;
     }
 
@@ -413,7 +413,7 @@ static int cache_handler(request_rec *r)
     /*
      * Which cache module (if any) should handle this request?
      */
-    if (!(providers = cache_get_providers(r, conf, r->parsed_uri))) {
+    if (!(providers = cache_get_providers(r, conf))) {
         return DECLINED;
     }