caching-proxied-conditional requests. My preliminary thought is:
1) If cache_select_url() determines the cached response is out-of-date, it can then add the 'If-None-Match' header (and any other headers as required) to the request. 2) We'll unconditionally add cache_save_filter because we don't have a usable cached entity. 3) cache_save_filter can then determine if we receive a 304 back by looking at r->status. 4) If we receive a 304, then cache_save_filter can update the cached headers and leave the body alone. [We'd have to add a lookup function at this point so we can identify which provider needs to be updated.] 5) If we did get a 304 *and* the original request was conditional, cache_save_filter can then remove itself (after updating headers) and get out of the way. 6) If we did get a 304 *and* the original request was *not* conditional (somehow we'd have to signal that, not sure how at the moment), then cache_save_filter can serve the cached body by calling recall_body and then eating all saved output. 7) If we didn't get a 304 from the origin server, cache_save_filter continues normally.
What do you think? -- justin
More or less, the following patch does the above. I haven't had a chance to test it, and won't likely to be able to do so until early next week.
There are a couple of FIXMEs it'd be nice to resolve first. -- justin
Index: modules/experimental/cache_storage.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/experimental/cache_storage.c,v
retrieving revision 1.39
diff -u -r1.39 cache_storage.c
--- modules/experimental/cache_storage.c 24 Sep 2004 01:22:47 -0000 1.39
+++ modules/experimental/cache_storage.c 24 Sep 2004 21:42:35 -0000
@@ -245,10 +245,32 @@
}
}+ 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 @@
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;
}
Index: modules/experimental/cache_util.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/experimental/cache_util.c,v
retrieving revision 1.34
diff -u -r1.34 cache_util.c
--- modules/experimental/cache_util.c 21 Sep 2004 22:56:23 -0000 1.34
+++ modules/experimental/cache_util.c 24 Sep 2004 21:42:35 -0000
@@ -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;
Index: modules/experimental/mod_cache.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/experimental/mod_cache.c,v
retrieving revision 1.91
diff -u -r1.91 mod_cache.c
--- modules/experimental/mod_cache.c 22 Sep 2004 22:02:13 -0000 1.91
+++ modules/experimental/mod_cache.c 24 Sep 2004 21:42:35 -0000
@@ -219,7 +219,7 @@
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 @@
* 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 EOS and response. 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 @@
* 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 @@ /* 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 @@
*/
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,32 @@
* - 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;
+ }
+ 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);
}
- /* 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);
@@ -550,6 +553,7 @@
* In addition, we make HTTP/1.1 age calculations and write them away
* too.
*/
+ info = apr_pcalloc(r->pool, sizeof(cache_info));/* Read the date. Generate one if one is not supplied */
dates = apr_table_get(r->err_headers_out, "Date");
@@ -640,6 +644,36 @@
* 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 {
+ /* FIXME: This can't be correct! But we can *not* return 304.
+ * Is it acceptable to return the cached status of what we had
+ * before?
+ * Add status field to cache_info?
+ * r->status = cache->stale_handle->status;
+ */
+ r->status = OK;
+ 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);
}
Index: modules/experimental/mod_cache.h
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/experimental/mod_cache.h,v
retrieving revision 1.50
diff -u -r1.50 mod_cache.h
--- modules/experimental/mod_cache.h 21 Sep 2004 22:56:23 -0000 1.50
+++ modules/experimental/mod_cache.h 24 Sep 2004 21:42:35 -0000
@@ -217,7 +217,10 @@
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(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);
