Move parts of cache_save_filter() to a smaller check_cacheable_request()
function that is easier to read and understand. Also, a useless assignment
is removed.
Index: modules/cache/mod_cache.c
===================================================================
--- modules/cache/mod_cache.c.orig
+++ modules/cache/mod_cache.c
@@ -313,65 +313,11 @@
* Finally, pass the data to the next filter (the network or whatever)
*/
-static int cache_save_filter(ap_filter_t *f, apr_bucket_brigade *in)
+static const char* check_cacheable_request(request_rec *r,
+ cache_request_rec *cache, cache_server_conf *conf)
{
- int rv = !OK;
- int date_in_errhdr = 0;
- request_rec *r = f->r;
- cache_request_rec *cache;
- cache_server_conf *conf;
- const char *cc_out, *cl;
- const char *exps, *lastmods, *dates, *etag;
- apr_time_t exp, date, lastmod, now;
- apr_off_t size;
- cache_info *info = NULL;
- char *reason;
- apr_pool_t *p;
-
- 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);
- }
-
- reason = NULL;
- p = r->pool;
- /*
- * Pass Data to Cache
- * ------------------
- * This section passes the brigades into the cache modules, but only
- * if the setup section (see below) is complete.
- */
- if (cache->block_response) {
- /* We've already sent down the response and EOS. So, ignore
- * whatever comes now.
- */
- return APR_SUCCESS;
- }
-
- /* have we already run the cachability check and set up the
- * cached file handle?
- */
- if (cache->in_checked) {
- /* pass the brigades into the cache, then pass them
- * up the filter stack
- */
- rv = cache->provider->store_body(cache->handle, r, in);
- if (rv != APR_SUCCESS) {
- ap_log_error(APLOG_MARK, APLOG_DEBUG, rv, r->server,
- "cache: Cache provider's store_body failed!");
- ap_remove_output_filter(f);
- }
- return ap_pass_brigade(f->next, in);
- }
+ apr_time_t exp, lastmod;
+ const char *exps, *lastmods, *etag, *cc_out, *reason = NULL;
/*
* Setup Data in Cache
@@ -439,11 +385,11 @@
* We include 304 Not Modified here too as this is the origin server
* telling us to serve the cached copy.
*/
- reason = apr_psprintf(p, "Response status %d", r->status);
+ reason = apr_psprintf(r->pool, "Response status %d", r->status);
}
else if (exps != NULL && exp == APR_DATE_BAD) {
/* if a broken Expires header is present, don't cache it */
- reason = apr_pstrcat(p, "Broken expires header: ", exps, NULL);
+ reason = apr_pstrcat(r->pool, "Broken expires header: ", exps, NULL);
}
else if (exp != APR_DATE_BAD && exp < r->request_time)
{
@@ -522,20 +468,16 @@
reason = "r->no_cache present";
}
- if (reason) {
- ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
- "cache: %s not cached. Reason: %s", r->unparsed_uri,
- reason);
+ cache->exp = exp;
+ cache->lastmod = lastmod;
- /* remove this filter from the chain */
- ap_remove_output_filter(f);
-
- /* ship the data up the stack */
- return ap_pass_brigade(f->next, in);
- }
+ return reason;
+}
- /* Make it so that we don't execute this path again. */
- cache->in_checked = 1;
+static apr_off_t request_cl(request_rec *r, apr_bucket_brigade *in)
+{
+ const char *cl;
+ apr_off_t size;
/* Set the content length if known.
*/
@@ -580,48 +522,18 @@
}
}
- /* It's safe to cache the response.
- *
- * There are two possiblities at this point:
- * - cache->handle == NULL. In this case there is no previously
- * cached entity anywhere on the system. We must create a brand
- * new entity and store the response in it.
- * - cache->stale_handle != NULL. In this case there is a stale
- * entity in the system which needs to be replaced by new
- * content (unless the result was 304 Not Modified, which means
- * the cached entity is actually fresh, and we should update
- * the headers).
- */
-
- /* Did we have a stale cache entry that really is stale? */
- if (cache->stale_handle) {
- if (r->status == HTTP_NOT_MODIFIED) {
- /* Oh, hey. It isn't that stale! Yay! */
- cache->handle = cache->stale_handle;
- info = &cache->handle->cache_obj->info;
- rv = OK;
- }
- else {
- /* Oh, well. Toss it. */
- cache->provider->remove_entity(cache->stale_handle);
- /* Treat the request as if it wasn't conditional. */
- cache->stale_handle = NULL;
- }
- }
+ return size;
+}
- /* no cache handle, create a new entity */
- if (!cache->handle) {
- rv = cache_create_entity(r, size);
- info = apr_pcalloc(r->pool, sizeof(cache_info));
- /* We only set info->status upon the initial creation. */
- info->status = r->status;
- }
+static void prepare_cacheable_request(request_rec *r, cache_request_rec *cache,
+ cache_info *info, cache_server_conf *conf)
+{
+ const char *dates;
+ int date_in_errhdr = 0;
+ apr_time_t exp, lastmod, now, date;
- if (rv != OK) {
- /* Caching layer declined the opportunity to cache the response */
- ap_remove_output_filter(f);
- return ap_pass_brigade(f->next, in);
- }
+ exp = cache->exp;
+ lastmod = cache->lastmod;
ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
"cache: Caching url: %s", r->unparsed_uri);
@@ -690,7 +602,6 @@
if (lastmod != APR_DATE_BAD && lastmod > date) {
/* if it's in the future, then replace by date */
lastmod = date;
- lastmods = dates;
ap_log_error(APLOG_MARK, APLOG_DEBUG, 0,
r->server,
"cache: Last modified is in the future, "
@@ -729,7 +640,129 @@
apr_table_set(r->headers_out, "Expires", expire_hdr);
}
}
+
info->expire = exp;
+}
+
+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_server_conf *conf;
+ apr_off_t size;
+ cache_info *info = NULL;
+ const char *reason;
+ apr_pool_t *p;
+
+ 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);
+ }
+
+ p = r->pool;
+
+ /*
+ * Pass Data to Cache
+ * ------------------
+ * This section passes the brigades into the cache modules, but only
+ * if the setup section (see below) is complete.
+ */
+ if (cache->block_response) {
+ /* We've already sent down the response and EOS. So, ignore
+ * whatever comes now.
+ */
+ return APR_SUCCESS;
+ }
+
+ /* have we already run the cachability check and set up the
+ * cached file handle?
+ */
+ if (cache->in_checked) {
+ /* pass the brigades into the cache, then pass them
+ * up the filter stack
+ */
+ rv = cache->provider->store_body(cache->handle, r, in);
+ if (rv != APR_SUCCESS) {
+ ap_log_error(APLOG_MARK, APLOG_DEBUG, rv, r->server,
+ "cache: Cache provider's store_body failed!");
+ ap_remove_output_filter(f);
+ }
+ return ap_pass_brigade(f->next, in);
+ }
+
+ reason = check_cacheable_request(r, cache, conf);
+
+ if (reason) {
+ ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
+ "cache: %s not cached. Reason: %s", r->unparsed_uri,
+ reason);
+
+ /* remove this filter from the chain */
+ ap_remove_output_filter(f);
+
+ /* ship the data up the stack */
+ return ap_pass_brigade(f->next, in);
+ }
+
+ /* Make it so that we don't execute this path again. */
+ cache->in_checked = 1;
+
+ size = request_cl(r, in);
+
+ /* It's safe to cache the response.
+ *
+ * There are two possiblities at this point:
+ * - cache->handle == NULL. In this case there is no previously
+ * cached entity anywhere on the system. We must create a brand
+ * new entity and store the response in it.
+ * - cache->stale_handle != NULL. In this case there is a stale
+ * entity in the system which needs to be replaced by new
+ * content (unless the result was 304 Not Modified, which means
+ * the cached entity is actually fresh, and we should update
+ * the headers).
+ */
+
+ /* Did we have a stale cache entry that really is stale? */
+ if (cache->stale_handle) {
+ if (r->status == HTTP_NOT_MODIFIED) {
+ /* Oh, hey. It isn't that stale! Yay! */
+ cache->handle = cache->stale_handle;
+ info = &cache->handle->cache_obj->info;
+ rv = OK;
+ }
+ else {
+ /* Oh, well. Toss it. */
+ cache->provider->remove_entity(cache->stale_handle);
+ /* Treat the request as if it wasn't conditional. */
+ cache->stale_handle = NULL;
+ }
+ }
+
+ /* no cache handle, create a new entity */
+ if (!cache->handle) {
+ rv = cache_create_entity(r, size);
+ info = apr_pcalloc(r->pool, sizeof(cache_info));
+ /* We only set info->status upon the initial creation. */
+ info->status = r->status;
+ }
+
+ if (rv != OK) {
+ /* Caching layer declined the opportunity to cache the response */
+ ap_remove_output_filter(f);
+ return ap_pass_brigade(f->next, in);
+ }
+
+ prepare_cacheable_request(r, cache, info, conf);
/* We found a stale entry which wasn't really stale. */
if (cache->stale_handle) {
--
