You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by je...@apache.org on 2005/02/09 02:39:01 UTC
svn commit: r152973 - in httpd/httpd/trunk/modules/cache: cache_storage.c
mod_cache.c mod_cache.h mod_disk_cache.c
Author: jerenkrantz
Date: Tue Feb 8 17:38:59 2005
New Revision: 152973
URL: http://svn.apache.org/viewcvs?view=rev&rev=152973
Log:
More work to properly handle revalidated responses correctly.
* modules/cache/mod_cache.c: If we add a new Expires value, tell our client;
merge in headers properly (or better than before) so that we can update the
header fields on a revalidated but with updated header fields.
* modules/cache/mod_cache.h, modules/cache/cache_storage.c:
Add preserve_orig flag to ap_cache_accept_headers to allow updating of fields.
* modules/cache/mod_disk_cache.c: Load status value from disk.
Modified:
httpd/httpd/trunk/modules/cache/cache_storage.c
httpd/httpd/trunk/modules/cache/mod_cache.c
httpd/httpd/trunk/modules/cache/mod_cache.h
httpd/httpd/trunk/modules/cache/mod_disk_cache.c
Modified: httpd/httpd/trunk/modules/cache/cache_storage.c
URL: http://svn.apache.org/viewcvs/httpd/httpd/trunk/modules/cache/cache_storage.c?view=diff&r1=152972&r2=152973
==============================================================================
--- httpd/httpd/trunk/modules/cache/cache_storage.c (original)
+++ httpd/httpd/trunk/modules/cache/cache_storage.c Tue Feb 8 17:38:59 2005
@@ -105,9 +105,10 @@
return 1;
}
-CACHE_DECLARE(void) ap_cache_accept_headers(cache_handle_t *h, request_rec *r)
+CACHE_DECLARE(void) ap_cache_accept_headers(cache_handle_t *h, request_rec *r,
+ int preserve_orig)
{
- apr_table_t *cookie_table;
+ apr_table_t *cookie_table, *hdr_copy;
const char *v;
v = apr_table_get(h->resp_hdrs, "Content-Type");
@@ -140,7 +141,14 @@
apr_table_unset(r->err_headers_out, "Set-Cookie");
apr_table_unset(h->resp_hdrs, "Set-Cookie");
- apr_table_overlap(r->headers_out, h->resp_hdrs, APR_OVERLAP_TABLES_SET);
+ if (preserve_orig) {
+ hdr_copy = apr_table_copy(r->pool, h->resp_hdrs);
+ apr_table_overlap(hdr_copy, r->headers_out, APR_OVERLAP_TABLES_SET);
+ r->headers_out = hdr_copy;
+ }
+ else {
+ apr_table_overlap(r->headers_out, h->resp_hdrs, APR_OVERLAP_TABLES_SET);
+ }
if (!apr_is_empty_table(cookie_table)) {
r->err_headers_out = apr_table_overlay(r->pool, r->err_headers_out,
cookie_table);
@@ -274,7 +282,7 @@
}
/* Okay, this response looks okay. Merge in our stuff and go. */
- ap_cache_accept_headers(h, r);
+ ap_cache_accept_headers(h, r, 0);
cache->handle = h;
return OK;
Modified: httpd/httpd/trunk/modules/cache/mod_cache.c
URL: http://svn.apache.org/viewcvs/httpd/httpd/trunk/modules/cache/mod_cache.c?view=diff&r1=152972&r2=152973
==============================================================================
--- httpd/httpd/trunk/modules/cache/mod_cache.c (original)
+++ httpd/httpd/trunk/modules/cache/mod_cache.c Tue Feb 8 17:38:59 2005
@@ -532,12 +532,6 @@
/* Oh, hey. It isn't that stale! Yay! */
cache->handle = cache->stale_handle;
info = &cache->handle->cache_obj->info;
- /* Load in the saved status. */
- r->status = info->status;
- /* The cached response will override our err_headers_out. */
- apr_table_clear(r->err_headers_out);
- /* Merge in our headers. */
- ap_cache_accept_headers(cache->handle, r);
rv = OK;
}
else {
@@ -636,6 +630,8 @@
* expire date = date + defaultexpire
*/
if (exp == APR_DATE_BAD) {
+ char expire_hdr[APR_RFC822_DATE_LEN];
+
/* if lastmod == date then you get 0*conf->factor which results in
* an expiration time of now. This causes some problems with
* freshness calculations, so we choose the else path...
@@ -647,19 +643,50 @@
x = conf->maxex;
}
exp = date + x;
+ apr_rfc822_date(expire_hdr, exp);
+ apr_table_set(r->headers_out, "Expires", expire_hdr);
}
else {
exp = date + conf->defex;
+ apr_rfc822_date(expire_hdr, exp);
+ apr_table_set(r->headers_out, "Expires", expire_hdr);
}
}
info->expire = exp;
- /*
- * Write away header information to cache.
- */
+ /* We found a stale entry which wasn't really stale. */
+ if (cache->stale_handle) {
+ /* Load in the saved status and clear the status line. */
+ r->status = info->status;
+ r->status_line = NULL;
+
+ /* RFC 2616 10.3.5 states that entity headers are not supposed
+ * to be in the 304 response. Therefore, we need to load in the
+ * cached headers before we update the cached headers.
+ *
+ * However, before doing that, we need to first merge in
+ * err_headers_out and we also need to strip any hop-by-hop
+ * headers that might have snuck in.
+ */
+ r->headers_out = apr_table_overlay(r->pool, r->headers_out,
+ r->err_headers_out);
+ r->headers_out = ap_cache_cacheable_hdrs_out(r->pool, r->headers_out,
+ r->server);
+ apr_table_clear(r->err_headers_out);
+
+ /* Merge in our cached headers. However, keep any updated values. */
+ ap_cache_accept_headers(cache->handle, r, 1);
+ }
+
+ /* 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? */
+ /* Did we just update the cached headers on a revalidated response?
+ *
+ * If so, we can now decide what to serve to the client:
+ * - If the original request was conditional and is satisified, send 304.
+ * - Otherwise, send the cached body.
+ */
if (rv == APR_SUCCESS && cache->stale_handle) {
apr_bucket_brigade *bb;
apr_bucket *bkt;
@@ -668,14 +695,15 @@
/* 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?
- */
+ /* FIXME: We must ensure that the request meets conditions. */
+
+ /* Set the status to be a 304. */
+ r->status = HTTP_NOT_MODIFIED;
+
bkt = apr_bucket_flush_create(bb->bucket_alloc);
APR_BRIGADE_INSERT_TAIL(bb, bkt);
}
else {
- r->status = info->status;
cache->provider->recall_body(cache->handle, r->pool, bb);
}
Modified: httpd/httpd/trunk/modules/cache/mod_cache.h
URL: http://svn.apache.org/viewcvs/httpd/httpd/trunk/modules/cache/mod_cache.h?view=diff&r1=152972&r2=152973
==============================================================================
--- httpd/httpd/trunk/modules/cache/mod_cache.h (original)
+++ httpd/httpd/trunk/modules/cache/mod_cache.h Tue Feb 8 17:38:59 2005
@@ -241,7 +241,15 @@
*/
CACHE_DECLARE(int) ap_cache_check_freshness(cache_handle_t *h, request_rec *r);
-CACHE_DECLARE(void) ap_cache_accept_headers(cache_handle_t *h, request_rec *r);
+/**
+ * Merge in cached headers into the response
+ * @param h cache_handle_t
+ * @param r request_rec
+ * @param preserve_orig If 1, the values in r->headers_out are preserved.
+ * Otherwise, they are overwritten by the cached value.
+ */
+CACHE_DECLARE(void) ap_cache_accept_headers(cache_handle_t *h, request_rec *r,
+ int preserve_orig);
CACHE_DECLARE(apr_time_t) ap_cache_hex2usec(const char *x);
CACHE_DECLARE(void) ap_cache_usec2hex(apr_time_t j, char *y);
Modified: httpd/httpd/trunk/modules/cache/mod_disk_cache.c
URL: http://svn.apache.org/viewcvs/httpd/httpd/trunk/modules/cache/mod_disk_cache.c?view=diff&r1=152972&r2=152973
==============================================================================
--- httpd/httpd/trunk/modules/cache/mod_disk_cache.c (original)
+++ httpd/httpd/trunk/modules/cache/mod_disk_cache.c Tue Feb 8 17:38:59 2005
@@ -221,6 +221,7 @@
/* Store it away so we can get it later. */
dobj->disk_info = disk_info;
+ info->status = disk_info.status;
info->date = disk_info.date;
info->expire = disk_info.expire;
info->request_time = disk_info.request_time;
Entity headers in 304 reponse, WAS: RE: svn commit: r152973 - in httpd/httpd/trunk/modules/cache: cache_storage.c mod_cache.c mod_cache.h mod_disk_cache.c
Posted by Sander Striker <st...@apache.org>.
> From: jerenkrantz@apache.org [mailto:jerenkrantz@apache.org]
> Sent: Wednesday, February 09, 2005 2:39 AM
> New Revision: 152973
[...]
+ /* RFC 2616 10.3.5 states that entity headers are not supposed
+ * to be in the 304 response. Therefore, we need to load in the
+ * cached headers before we update the cached headers.
That's not how I read that section. 14.26 has some specific text
stating cache-related header fields should be included.
Sander