From 7d2caa8d9b4270a3d85f8d7070b02e8d8d19c63b Mon Sep 17 00:00:00 2001 From: Justin Erenkrantz Date: Tue, 28 Sep 2004 17:37:54 +0000 Subject: [PATCH] Try to correctly follow RFC 2616 13.3 on validating stale cache responses by teaching mod_cache's cache_select_url and cache_save_filter how to deal with this corner case. * modules/experimental/cache_storage.c (cache_select_url): If we have a stale entry, save the handle so that cache_save_filter can use it later, and make the request conditional. * modules/experimental/cache_util.c (ap_cache_request_is_conditional): Take in a table rather than request_rec. * modules/experimental/mod_cache.c (cache_out_filter): Fix bogus comment. (cache_save_filter): If we have already responded to the client, block all data; correctly merge in 'stale' handles that are not really stale; set r->status where appropriate; serve cached response if 'fresh' * modules/experimental/mod_cache.h (cache_info): Add a status field. (cache_request_rec): Add stale handle field and note we may block responses. (ap_cache_request_is_conditional): Update prototype. * modules/experimental/mod_disk_cache.c (store_headers): Use cache_info status instead of r->status. * modules/experimental/mod_mem_cache.c (recall_headers): Properly recall the status field. (store_headers): Store the status field via cache_info status. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@105322 13f79535-47bb-0310-9956-ffa450edef68 --- CHANGES | 3 ++ modules/experimental/cache_storage.c | 26 +++++++-- modules/experimental/cache_util.c | 10 ++-- modules/experimental/mod_cache.c | 78 +++++++++++++++++++-------- modules/experimental/mod_cache.h | 9 ++-- modules/experimental/mod_disk_cache.c | 7 +-- modules/experimental/mod_mem_cache.c | 2 + 7 files changed, 95 insertions(+), 40 deletions(-) diff --git a/CHANGES b/CHANGES index bd4923e479..8b55901c85 100644 --- a/CHANGES +++ b/CHANGES @@ -2,6 +2,9 @@ Changes with Apache 2.1.0-dev [Remove entries to the current 2.0 section below, when backported] + *) mod_cache: Try to correctly follow RFC 2616 13.3 on validating stale + cache responses. [Justin Erenkrantz] + *) mod_disk_cache: Do not store aborted content. PR 21492. [Rüdiger Plüm ] diff --git a/modules/experimental/cache_storage.c b/modules/experimental/cache_storage.c index 329031ec47..a300bcb334 100644 --- a/modules/experimental/cache_storage.c +++ b/modules/experimental/cache_storage.c @@ -245,10 +245,32 @@ int cache_select_url(request_rec *r, char *url) } } + cache->provider = list->provider; + cache->provider_name = list->provider_name; + /* Is our cached response fresh enough? */ fresh = ap_cache_check_freshness(h, r); if (!fresh) { - list->provider->remove_entity(h); + cache_info *info = &(h->cache_obj->info); + + /* Make response into a conditional */ + /* FIXME: What if the request is already conditional? */ + if (info && info->etag) { + /* if we have a cached etag */ + cache->stale_headers = apr_table_copy(r->pool, + r->headers_in); + apr_table_set(r->headers_in, "If-None-Match", info->etag); + cache->stale_handle = h; + } + else if (info && info->lastmods) { + /* if we have a cached Last-Modified header */ + cache->stale_headers = apr_table_copy(r->pool, + r->headers_in); + apr_table_set(r->headers_in, "If-Modified-Since", + info->lastmods); + cache->stale_handle = h; + } + return DECLINED; } @@ -258,8 +280,6 @@ int cache_select_url(request_rec *r, char *url) r->filename = apr_pstrdup(r->pool, h->cache_obj->info.filename); accept_headers(h, r); - cache->provider = list->provider; - cache->provider_name = list->provider_name; cache->handle = h; return OK; } diff --git a/modules/experimental/cache_util.c b/modules/experimental/cache_util.c index bfb0c740db..9ee9f84c9a 100644 --- a/modules/experimental/cache_util.c +++ b/modules/experimental/cache_util.c @@ -22,12 +22,12 @@ /* -------------------------------------------------------------- */ /* return true if the request is conditional */ -CACHE_DECLARE(int) ap_cache_request_is_conditional(request_rec *r) +CACHE_DECLARE(int) ap_cache_request_is_conditional(apr_table_t *table) { - if (apr_table_get(r->headers_in, "If-Match") || - apr_table_get(r->headers_in, "If-None-Match") || - apr_table_get(r->headers_in, "If-Modified-Since") || - apr_table_get(r->headers_in, "If-Unmodified-Since")) { + if (apr_table_get(table, "If-Match") || + apr_table_get(table, "If-None-Match") || + apr_table_get(table, "If-Modified-Since") || + apr_table_get(table, "If-Unmodified-Since")) { return 1; } return 0; diff --git a/modules/experimental/mod_cache.c b/modules/experimental/mod_cache.c index 9daff6f600..45928d68f9 100644 --- a/modules/experimental/mod_cache.c +++ b/modules/experimental/mod_cache.c @@ -219,7 +219,7 @@ static int cache_out_filter(ap_filter_t *f, apr_bucket_brigade *bb) ap_log_error(APLOG_MARK, APLOG_DEBUG, APR_SUCCESS, r->server, "cache: running CACHE_OUT filter"); - /* recall_body() was called in cache_select_url() */ + /* recall_headers() was called in cache_select_url() */ cache->provider->recall_body(cache->handle, r->pool, bb); /* This filter is done once it has served up its content */ @@ -290,6 +290,12 @@ static int cache_save_filter(ap_filter_t *f, apr_bucket_brigade *in) * 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? @@ -312,7 +318,6 @@ static int cache_save_filter(ap_filter_t *f, apr_bucket_brigade *in) * parameters, and decides whether this URL should be cached at * all. This section is* run before the above section. */ - info = apr_pcalloc(r->pool, sizeof(cache_info)); /* read expiry date; if a bad date, then leave it so the client can * read it @@ -332,7 +337,7 @@ static int cache_save_filter(ap_filter_t *f, apr_bucket_brigade *in) /* read the last-modified date; if the date is bad, then delete it */ lastmods = apr_table_get(r->err_headers_out, "Last-Modified"); - if (lastmods ==NULL) { + if (lastmods == NULL) { lastmods = apr_table_get(r->headers_out, "Last-Modified"); } if (lastmods != NULL) { @@ -384,7 +389,8 @@ static int cache_save_filter(ap_filter_t *f, apr_bucket_brigade *in) */ reason = "Query string present but no expires header"; } - else if (r->status == HTTP_NOT_MODIFIED && (NULL == cache->handle)) { + else if (r->status == HTTP_NOT_MODIFIED && + !cache->handle && !cache->stale_handle) { /* if the server said 304 Not Modified but we have no cache * file - pass this untouched to the user agent, it's not for us. */ @@ -503,35 +509,36 @@ static int cache_save_filter(ap_filter_t *f, apr_bucket_brigade *in) * - 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->handle != NULL. In this case there is a stale + * - 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; + } + 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, url, size); + info = apr_pcalloc(r->pool, sizeof(cache_info)); + /* We only set info->status upon the initial creation. */ + info->status = r->status; } - /* pre-existing cache handle and 304, make entity fresh */ - else if (r->status == HTTP_NOT_MODIFIED) { - /* update headers: TODO */ - - /* remove this filter ??? */ - /* XXX is this right? we must set rv to something other than OK - * in this path - */ - rv = HTTP_NOT_MODIFIED; - } - /* pre-existing cache handle and new entity, replace entity - * with this one - */ - else { - cache->provider->remove_entity(cache->handle); - rv = cache_create_entity(r, url, size); - } - if (rv != OK) { /* Caching layer declined the opportunity to cache the response */ ap_remove_output_filter(f); @@ -640,6 +647,31 @@ static int cache_save_filter(ap_filter_t *f, apr_bucket_brigade *in) * Write away header information to cache. */ rv = cache->provider->store_headers(cache->handle, r, info); + + /* Did we actually find an entity before, but it wasn't really stale? */ + if (rv == APR_SUCCESS && cache->stale_handle) { + apr_bucket_brigade *bb; + apr_bucket *bkt; + + bb = apr_brigade_create(r->pool, r->connection->bucket_alloc); + + /* Were we initially a conditional request? */ + if (ap_cache_request_is_conditional(cache->stale_headers)) { + /* FIXME: Should we now go and make sure it's really not + * modified since what the user thought? + */ + bkt = apr_bucket_eos_create(bb->bucket_alloc); + APR_BRIGADE_INSERT_TAIL(bb, bkt); + } + else { + r->status = info->status; + cache->provider->recall_body(cache->handle, r->pool, bb); + } + + cache->block_response = 1; + return ap_pass_brigade(f->next, bb); + } + if (rv == APR_SUCCESS) { rv = cache->provider->store_body(cache->handle, r, in); } diff --git a/modules/experimental/mod_cache.h b/modules/experimental/mod_cache.h index e519940e66..1135a4de1b 100644 --- a/modules/experimental/mod_cache.h +++ b/modules/experimental/mod_cache.h @@ -138,6 +138,7 @@ typedef struct { /* cache info information */ typedef struct cache_info cache_info; struct cache_info { + int status; char *content_type; char *etag; char *lastmods; /* last modified of cache entity */ @@ -153,7 +154,6 @@ struct cache_info { apr_time_t ius; /* If-UnModified_Since header value */ const char *im; /* If-Match header value */ const char *inm; /* If-None-Match header value */ - }; /* cache handle information */ @@ -217,7 +217,10 @@ typedef struct { const char *provider_name; /* current cache provider name */ int fresh; /* is the entitey fresh? */ cache_handle_t *handle; /* current cache handle */ - int in_checked; /* CACHE_IN must cache the entity */ + cache_handle_t *stale_handle; /* stale cache handle */ + apr_table_t *stale_headers; /* original request headers. */ + int in_checked; /* CACHE_SAVE must cache the entity */ + int block_response; /* CACHE_SAVE must block response. */ apr_bucket_brigade *saved_brigade; /* copy of partial response */ apr_off_t saved_size; /* length of saved_brigade */ apr_time_t exp; /* expiration */ @@ -243,7 +246,7 @@ CACHE_DECLARE(void) ap_cache_usec2hex(apr_time_t j, char *y); CACHE_DECLARE(char *) generate_name(apr_pool_t *p, int dirlevels, int dirlength, const char *name); -CACHE_DECLARE(int) ap_cache_request_is_conditional(request_rec *r); +CACHE_DECLARE(int) ap_cache_request_is_conditional(apr_table_t *table); CACHE_DECLARE(cache_provider_list *)ap_cache_get_providers(request_rec *r, cache_server_conf *conf, const char *url); CACHE_DECLARE(int) ap_cache_liststr(apr_pool_t *p, const char *list, const char *key, char **val); diff --git a/modules/experimental/mod_disk_cache.c b/modules/experimental/mod_disk_cache.c index 125bec9c40..c835f6f193 100644 --- a/modules/experimental/mod_disk_cache.c +++ b/modules/experimental/mod_disk_cache.c @@ -589,14 +589,9 @@ static apr_status_t store_headers(cache_handle_t *h, request_rec *r, cache_info disk_info.entity_version = dobj->disk_info.entity_version++; disk_info.request_time = info->request_time; disk_info.response_time = info->response_time; + disk_info.status = info->status; disk_info.name_len = strlen(dobj->name); - disk_info.status = r->status; - - /* This case only occurs when the content is generated locally */ - if (!r->status_line) { - r->status_line = ap_get_status_line(r->status); - } iov[0].iov_base = (void*)&disk_info; iov[0].iov_len = sizeof(disk_cache_info_t); diff --git a/modules/experimental/mod_mem_cache.c b/modules/experimental/mod_mem_cache.c index 108522f498..f34ba809d7 100644 --- a/modules/experimental/mod_mem_cache.c +++ b/modules/experimental/mod_mem_cache.c @@ -693,6 +693,7 @@ static apr_status_t recall_headers(cache_handle_t *h, request_rec *r) * CACHE_IN runs before header filters.... */ h->content_type = h->cache_obj->info.content_type; + h->status = h->cache_obj->info.status; return rc; } @@ -766,6 +767,7 @@ static apr_status_t store_headers(cache_handle_t *h, request_rec *r, cache_info } /* Init the info struct */ + obj->info.status = info->status; if (info->date) { obj->info.date = info->date; } -- 2.40.0