On Mon, Mar 25, 2013 at 11:58 PM, Roy T. Fielding <[email protected]> wrote:
> On Mar 13, 2013, at 10:20 AM, Graham Leggett wrote:
>> I don't read it that way from the spec, I think it all comes down to the
>> phrase "without successful revalidation with the origin server". I read it
>> as "without successful revalidation [of the whole request] with the origin
>> server". In other words, the origin server sent the original header, if the
>> origin server doesn't update the header in the 304 response then it means
>> "I've had my opportunity to revalidate the entity, current cached value is
>> fine, send it".
>>
>> Roy ultimately would be able to answer this?
>
> It means delete the header fields prior to storing them in the cache
> for later reuse. If the origin had wanted must-revalidate, it would
> simply use that directive instead. The successful revalidation bit
> is saying that the cache should forward all of the fields for the response
> to the original request and for any response that is revalidated
> (i.e., forward the new fields received in 304), but not for the
> requests that are entirely handled by the cache.
>
Thank you for clarification, hence mod_cache is allowed to serve the
cached response (with respect to the "other restrictions"), no
revalidation is needed (for the CC header fields at least).
The following patch implements this behaviour, with the CC header
fields not being stored while still played with the origin
(validation) response.
Regards,
Yann.
Index: modules/cache/cache_util.c
===================================================================
--- modules/cache/cache_util.c (revision 1461557)
+++ modules/cache/cache_util.c (working copy)
@@ -542,13 +542,10 @@
}
/* These come from the cached entity. */
- if (h->cache_obj->info.control.no_cache
- || h->cache_obj->info.control.no_cache_header
- || h->cache_obj->info.control.private_header) {
+ if (h->cache_obj->info.control.no_cache) {
/*
- * The cached entity contained Cache-Control: no-cache, or a
- * no-cache with a header present, or a private with a header
- * present, so treat as stale causing revalidation.
+ * The cached entity contained Cache-Control: no-cache,
+ * so treat as stale causing revalidation.
*/
return 0;
}
@@ -915,12 +912,23 @@
return ap_cache_cacheable_headers(r->pool, r->headers_in, r->server);
}
-/*
- * Create a new table consisting of those elements from an output
+static int cc_field_doo_doo(void *t, const char *key, const char *val)
+{
+ if (val) {
+ apr_table_addn(t, key, val);
+ return 0;
+ }
+ return 1;
+}
+
+/* Create a new table consisting of those elements from an output
* headers table that are allowed to be stored in a cache;
+ * when cc is not NULL, also strip the headers specified by the
+ * Cache-Control private= or no-cache= directives;
* ensure there is a content type and capture any errors.
*/
-CACHE_DECLARE(apr_table_t *)ap_cache_cacheable_headers_out(request_rec *r)
+CACHE_DECLARE(apr_table_t *)ap_cache_cacheable_headers_out(request_rec *r,
+ const cache_control_t *cc)
{
apr_table_t *headers_out;
@@ -944,6 +952,46 @@
r->content_encoding);
}
+ if (cc && (cc->no_cache_header || cc->private_header)) {
+ char *token;
+ const char *cc_out = apr_table_get(headers_out, "Cache-Control");
+ while (cc_out && (token = ap_get_list_item(r->pool, &cc_out))) {
+ apr_size_t len = strlen(token);
+
+ /* ap_get_list_item() strips the spurious whitespaces and
+ * lowercases anything (but the quoted-strings) */
+ if (len > 9 && strncmp(token, "no-cache=", 9) == 0) {
+ token += 9;
+ len -= 9;
+ }
+ else if (len > 8 && strncmp(token, "private=", 8) == 0) {
+ token += 8;
+ len -= 8;
+ }
+ else {
+ continue;
+ }
+
+ /* RFC2616 14.9: quoted list of field-names */
+ if (len > 2 && token[0] == '"' && token[--len] == '"') {
+ /* strip the header(s) from the cacheable headers out,
+ * but preserve the ones from the current response by
+ * adding them to the err_headers_out */
+ const char *tok, *header;
+ (++token)[--len] = '\0';
+ tok = token;
+ do {
+ if ((header = ap_cache_tokstr(r->pool, tok, &tok)) &&
+ !apr_table_do(cc_field_doo_doo, r->err_headers_out,
+ headers_out, header, NULL)) {
+ apr_table_unset(r->headers_out, header);
+ apr_table_unset(headers_out, header);
+ }
+ } while (tok);
+ }
+ }
+ }
+
return headers_out;
}
@@ -1069,9 +1117,7 @@
/* ...then try slowest cases */
else if (!strncasecmp(token, "no-cache", 8)) {
if (token[8] == '=') {
- if (apr_table_get(headers, token + 9)) {
- cc->no_cache_header = 1;
- }
+ cc->no_cache_header = 1;
}
else if (!token[8]) {
cc->no_cache = 1;
@@ -1146,9 +1192,7 @@
}
else if (!strncasecmp(token, "private", 7)) {
if (token[7] == '=') {
- if (apr_table_get(headers, token + 8)) {
- cc->private_header = 1;
- }
+ cc->private_header = 1;
}
else if (!token[7]) {
cc->private = 1;
Index: modules/cache/mod_cache.c
===================================================================
--- modules/cache/mod_cache.c (revision 1461557)
+++ modules/cache/mod_cache.c (working copy)
@@ -1075,7 +1075,7 @@
* err_headers_out and we also need to strip any hop-by-hop
* headers that might have snuck in.
*/
- r->headers_out = ap_cache_cacheable_headers_out(r);
+ r->headers_out = ap_cache_cacheable_headers_out(r, &control);
/* Merge in our cached headers. However, keep any
updated values. */
cache_accept_headers(cache->handle, r, 1);
@@ -1347,12 +1347,15 @@
* err_headers_out and we also need to strip any hop-by-hop
* headers that might have snuck in.
*/
- r->headers_out = ap_cache_cacheable_headers_out(r);
+ r->headers_out = ap_cache_cacheable_headers_out(r, NULL);
/* Merge in our cached headers. However, keep any updated values. */
cache_accept_headers(cache->handle, r, 1);
}
+ cache->handle->req_hdrs = ap_cache_cacheable_headers_in(r);
+ cache->handle->resp_hdrs = ap_cache_cacheable_headers_out(r, &control);
+
/* Write away header information to cache. It is possible that we are
* trying to update headers for an entity which has already been cached.
*
Index: modules/cache/mod_cache.h
===================================================================
--- modules/cache/mod_cache.h (revision 1461557)
+++ modules/cache/mod_cache.h (working copy)
@@ -152,9 +152,12 @@
/* Create a new table consisting of those elements from an output
* headers table that are allowed to be stored in a cache;
+ * when cc is not NULL, also strip the headers specified by the
+ * Cache-Control private= or no-cache= directives;
* ensure there is a content type and capture any errors.
*/
-CACHE_DECLARE(apr_table_t *)ap_cache_cacheable_headers_out(request_rec *r);
+CACHE_DECLARE(apr_table_t *)ap_cache_cacheable_headers_out(request_rec *r,
+ const cache_control_t *cc);
/**
* Parse the Cache-Control and Pragma headers in one go, marking
Index: modules/cache/mod_cache_disk.c
===================================================================
--- modules/cache/mod_cache_disk.c (revision 1461557)
+++ modules/cache/mod_cache_disk.c (working copy)
@@ -933,11 +933,11 @@
memcpy(&h->cache_obj->info, info, sizeof(cache_info));
if (r->headers_out) {
- dobj->headers_out = ap_cache_cacheable_headers_out(r);
+ dobj->headers_out = h->resp_hdrs;
}
if (r->headers_in) {
- dobj->headers_in = ap_cache_cacheable_headers_in(r);
+ dobj->headers_in = h->req_hdrs;
}
return APR_SUCCESS;
Index: modules/cache/mod_cache_socache.c
===================================================================
--- modules/cache/mod_cache_socache.c (revision 1461557)
+++ modules/cache/mod_cache_socache.c (working copy)
@@ -791,11 +791,11 @@
memcpy(&h->cache_obj->info, info, sizeof(cache_info));
if (r->headers_out) {
- sobj->headers_out = ap_cache_cacheable_headers_out(r);
+ sobj->headers_out = h->resp_hdrs;
}
if (r->headers_in) {
- sobj->headers_in = ap_cache_cacheable_headers_in(r);
+ sobj->headers_in = h->req_hdrs;
}
sobj->expire
