mod_cache: Use a proper filter context to hold filter data instead

of misusing the per-request configuration. Fixes a segfault on trunk
when the normal handler is used.

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

diff --git a/CHANGES b/CHANGES
index a427fd0209305d4e77aeaded3266c94e825c6c2b..59b71ed5dcee005087805423cda301c75469f3bc 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -2,6 +2,10 @@
 
 Changes with Apache 2.3.9
 
+  *) mod_cache: Use a proper filter context to hold filter data instead
+     of misusing the per-request configuration. Fixes a segfault on trunk
+     when the normal handler is used. [Graham Leggett]
+
   *) mod_cgid: Log a warning if the ScriptSock path is truncated because
      it is too long. PR 49388.  [Stefan Fritsch]
 

diff --git a/modules/cache/cache_storage.c b/modules/cache/cache_storage.c
index 0e2a698f53db1babd70995f685245b8b0c4fd920..9137fca0b0dd9851c26f8d58479246ad62e6491b 100644 (file)
--- a/modules/cache/cache_storage.c
+++ b/modules/cache/cache_storage.c
@@ -72,8 +72,11 @@ int cache_create_entity(request_rec *r, apr_off_t size)
     cache_handle_t *h = apr_pcalloc(r->pool, sizeof(cache_handle_t));
     char *key;
     apr_status_t rv;
-    cache_request_rec *cache = (cache_request_rec *)
-                         ap_get_module_config(r->request_config, &cache_module);
+    cache_request_rec *cache;
+    void *data;
+
+    apr_pool_userdata_get(&data, MOD_CACHE_REQUEST_REC, r->pool);
+    cache = data;
 
     rv = cache_generate_key(r, r->pool, &key);
     if (rv != APR_SUCCESS) {
@@ -108,6 +111,10 @@ static int set_cookie_doo_doo(void *v, const char *key, const char *val)
     return 1;
 }
 
+/**
+ * Take headers from the cache, and overlap them over the existing response
+ * headers.
+ */
 CACHE_DECLARE(void) ap_cache_accept_headers(cache_handle_t *h, request_rec *r,
                                             int preserve_orig)
 {
@@ -186,8 +193,11 @@ int cache_select(request_rec *r)
     apr_status_t rv;
     cache_handle_t *h;
     char *key;
-    cache_request_rec *cache = (cache_request_rec *)
-                         ap_get_module_config(r->request_config, &cache_module);
+    cache_request_rec *cache;
+    void *data;
+
+    apr_pool_userdata_get(&data, MOD_CACHE_REQUEST_REC, r->pool);
+    cache = data;
 
     rv = cache_generate_key(r, r->pool, &key);
     if (rv != APR_SUCCESS) {
@@ -358,9 +368,10 @@ apr_status_t cache_generate_key_default(request_rec *r, apr_pool_t* p,
     const char *hostname, *scheme;
     int i;
     char *path, *querystring;
+    void *data;
 
-    cache = (cache_request_rec *) ap_get_module_config(r->request_config,
-                                                       &cache_module);
+    apr_pool_userdata_get(&data, MOD_CACHE_REQUEST_REC, r->pool);
+    cache = data;
     if (!cache) {
         /* This should never happen */
         ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server,

diff --git a/modules/cache/mod_cache.c b/modules/cache/mod_cache.c
index ad87936c4604cb39f58e3280105f6d83130371b9..76cde9a2efd19495e5c2185cbaadba614b8501a5 100644 (file)
--- a/modules/cache/mod_cache.c
+++ b/modules/cache/mod_cache.c
@@ -92,13 +92,11 @@ static int cache_quick_handler(request_rec *r, int lookup)
     }
 
     /* make space for the per request config */
-    cache = (cache_request_rec *) ap_get_module_config(r->request_config,
-                                                       &cache_module);
-    if (!cache) {
-        cache = apr_pcalloc(r->pool, sizeof(cache_request_rec));
-        cache->size = -1;
-        ap_set_module_config(r->request_config, &cache_module, cache);
-    }
+    cache = apr_pcalloc(r->pool, sizeof(cache_request_rec));
+    cache->size = -1;
+
+    /* store away the per request config where the API can find it */
+    apr_pool_userdata_setn(cache, MOD_CACHE_REQUEST_REC, NULL, r->pool);
 
     /* save away the possible providers */
     cache->providers = providers;
@@ -153,14 +151,14 @@ static int cache_quick_handler(request_rec *r, int lookup)
                                 "Adding CACHE_SAVE_SUBREQ filter for %s",
                                 r->uri);
                         ap_add_output_filter_handle(cache_save_subreq_filter_handle,
-                                NULL, r, r->connection);
+                                cache, r, r->connection);
                     }
                     else {
                         ap_log_error(APLOG_MARK, APLOG_DEBUG, APR_SUCCESS,
                                 r->server, "Adding CACHE_SAVE filter for %s",
                                 r->uri);
                         ap_add_output_filter_handle(cache_save_filter_handle,
-                                NULL, r, r->connection);
+                                cache, r, r->connection);
                     }
 
                     ap_log_error(APLOG_MARK, APLOG_DEBUG, APR_SUCCESS, r->server,
@@ -191,9 +189,6 @@ static int cache_quick_handler(request_rec *r, int lookup)
 
                     r->headers_in = cache->stale_headers;
                 }
-
-                /* Delete our per-request configuration. */
-                ap_set_module_config(r->request_config, &cache_module, NULL);
             }
         }
         else {
@@ -213,9 +208,6 @@ static int cache_quick_handler(request_rec *r, int lookup)
                          "Restoring request headers.");
             r->headers_in = cache->stale_headers;
         }
-
-        /* Delete our per-request configuration. */
-        ap_set_module_config(r->request_config, &cache_module, NULL);
     }
 
     rv = ap_meets_conditions(r);
@@ -254,7 +246,7 @@ static int cache_quick_handler(request_rec *r, int lookup)
     else {
         cache_out_handle = cache_out_filter_handle;
     }
-    ap_add_output_filter_handle(cache_out_handle, NULL, r, r->connection);
+    ap_add_output_filter_handle(cache_out_handle, cache, r, r->connection);
 
     /*
      * Remove all filters that are before the cache_out filter. This ensures
@@ -359,12 +351,11 @@ static int cache_handler(request_rec *r)
     }
 
     /* make space for the per request config */
-    cache = (cache_request_rec *) ap_get_module_config(r->request_config,
-                                                       &cache_module);
-    if (!cache) {
-        cache = apr_pcalloc(r->pool, sizeof(cache_request_rec));
-        ap_set_module_config(r->request_config, &cache_module, cache);
-    }
+    cache = apr_pcalloc(r->pool, sizeof(cache_request_rec));
+    cache->size = -1;
+
+    /* store away the per request config where the API can find it */
+    apr_pool_userdata_setn(cache, MOD_CACHE_REQUEST_REC, NULL, r->pool);
 
     /* save away the possible providers */
     cache->providers = providers;
@@ -412,7 +403,7 @@ static int cache_handler(request_rec *r)
                     cache_save_handle = cache_save_filter_handle;
                 }
                 ap_add_output_filter_handle(cache_save_handle,
-                        NULL, r, r->connection);
+                        cache, r, r->connection);
 
                 /*
                  * Did the user indicate the precise location of the
@@ -481,7 +472,7 @@ static int cache_handler(request_rec *r)
     else {
         cache_out_handle = cache_out_filter_handle;
     }
-    ap_add_output_filter_handle(cache_out_handle, NULL, r, r->connection);
+    ap_add_output_filter_handle(cache_out_handle, cache, r, r->connection);
 
     /*
      * Did the user indicate the precise location of the CACHE_OUT filter by
@@ -542,16 +533,13 @@ static int cache_handler(request_rec *r)
 static int cache_out_filter(ap_filter_t *f, apr_bucket_brigade *bb)
 {
     request_rec *r = f->r;
-    cache_request_rec *cache;
-
-    cache = (cache_request_rec *) ap_get_module_config(r->request_config,
-                                                       &cache_module);
+    cache_request_rec *cache = (cache_request_rec *)f->ctx;
 
     if (!cache) {
         /* user likely configured CACHE_OUT manually; they should use mod_cache
          * configuration to do that */
         ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server,
-                     "CACHE_OUT enabled unexpectedly");
+                     "CACHE/CACHE_OUT filter enabled while caching is disabled, ignoring");
         ap_remove_output_filter(f);
         return ap_pass_brigade(f->next, bb);
     }
@@ -599,7 +587,7 @@ static int cache_save_filter(ap_filter_t *f, apr_bucket_brigade *in)
 {
     int rv = !OK;
     request_rec *r = f->r;
-    cache_request_rec *cache;
+    cache_request_rec *cache = (cache_request_rec *)f->ctx;
     cache_server_conf *conf;
     const char *cc_out, *cl;
     const char *exps, *lastmods, *dates, *etag;
@@ -609,20 +597,20 @@ static int cache_save_filter(ap_filter_t *f, apr_bucket_brigade *in)
     char *reason;
     apr_pool_t *p;
     apr_bucket *e;
+    void *data;
 
     conf = (cache_server_conf *) ap_get_module_config(r->server->module_config,
                                                       &cache_module);
 
     /* Setup cache_request_rec */
-    cache = (cache_request_rec *) ap_get_module_config(r->request_config,
-                                                       &cache_module);
     if (!cache) {
         /* user likely configured CACHE_SAVE manually; they should really use
          * mod_cache configuration to do that
          */
-        cache = apr_pcalloc(r->pool, sizeof(cache_request_rec));
-        ap_set_module_config(r->request_config, &cache_module, cache);
-        cache->size = -1;
+        ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server,
+                     "CACHE/CACHE_SAVE filter enabled while caching is disabled, ignoring");
+        ap_remove_output_filter(f);
+        return ap_pass_brigade(f->next, in);
     }
 
     reason = NULL;

diff --git a/modules/cache/mod_cache.h b/modules/cache/mod_cache.h
index 0dedfc48a469c0a9e2d31d7a04771ce2b209acad..d2984a45573d76f3c08a4e73e33bfd6c3e9e0fa7 100644 (file)
--- a/modules/cache/mod_cache.h
+++ b/modules/cache/mod_cache.h
@@ -374,10 +374,13 @@ CACHE_DECLARE(apr_table_t *)ap_cache_cacheable_hdrs_out(apr_pool_t *pool,
 /**
  * cache_storage.c
  */
+#define MOD_CACHE_REQUEST_REC "mod_cache_request_rec"
 int cache_remove_url(cache_request_rec *cache, apr_pool_t *p);
 int cache_create_entity(request_rec *r, apr_off_t size);
 int cache_select(request_rec *r);
 apr_status_t cache_generate_key_default( request_rec *r, apr_pool_t*p, char**key );
+
+
 /**
  * create a key for the cache based on the request record
  * this is the 'default' version, which can be overridden by a default function