You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by mi...@apache.org on 2011/02/13 03:03:30 UTC
svn commit: r1070179 - in /httpd/httpd/trunk: CHANGES
docs/manual/mod/mod_cache.xml modules/cache/cache_storage.c
modules/cache/cache_storage.h modules/cache/mod_cache.c
modules/cache/mod_cache.h
Author: minfrin
Date: Sun Feb 13 02:03:29 2011
New Revision: 1070179
URL: http://svn.apache.org/viewvc?rev=1070179&view=rev
Log:
mod_cache: When a request other than GET or HEAD arrives, we must
invalidate existing cache entities as per RFC2616 13.10. PR 15868.
Modified:
httpd/httpd/trunk/CHANGES
httpd/httpd/trunk/docs/manual/mod/mod_cache.xml
httpd/httpd/trunk/modules/cache/cache_storage.c
httpd/httpd/trunk/modules/cache/cache_storage.h
httpd/httpd/trunk/modules/cache/mod_cache.c
httpd/httpd/trunk/modules/cache/mod_cache.h
Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1070179&r1=1070178&r2=1070179&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Sun Feb 13 02:03:29 2011
@@ -2,6 +2,10 @@
Changes with Apache 2.3.11
+ *) mod_cache: When a request other than GET or HEAD arrives, we must
+ invalidate existing cache entities as per RFC2616 13.10. PR 15868.
+ [Graham Leggett]
+
*) modules: Fix many modules that were not correctly initializing if they
were not active during server startup but got enabled later during a
graceful restart. [Stefan Fritsch]
Modified: httpd/httpd/trunk/docs/manual/mod/mod_cache.xml
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/manual/mod/mod_cache.xml?rev=1070179&r1=1070178&r2=1070179&view=diff
==============================================================================
--- httpd/httpd/trunk/docs/manual/mod/mod_cache.xml (original)
+++ httpd/httpd/trunk/docs/manual/mod/mod_cache.xml Sun Feb 13 02:03:29 2011
@@ -302,13 +302,15 @@
</example>
<p>Based on the caching decision made, the reason is also written to the
- subprocess environment under one the following three keys, as appropriate:</p>
+ subprocess environment under one the following four keys, as appropriate:</p>
<dl>
<dt>cache-hit</dt><dd>The response was served from cache.</dd>
<dt>cache-revalidate</dt><dd>The response was stale and was successfully
revalidated, then served from cache.</dd>
<dt>cache-miss</dt><dd>The response was served from the upstream server.</dd>
+ <dt>cache-invalidate</dt><dd>The cached entity was invalidated by a request
+ method other than GET or HEAD.</dd>
</dl>
<p>This makes it possible to support conditional logging of cached requests
@@ -318,10 +320,14 @@
CustomLog cached-requests.log common env=cache-hit<br />
CustomLog uncached-requests.log common env=cache-miss<br />
CustomLog revalidated-requests.log common env=cache-revalidate<br />
+ CustomLog invalidated-requests.log common env=cache-invalidate<br />
</example>
+ <p>For module authors, a hook called <var>cache_status</var> is available,
+ allowing modules to respond to the caching outcomes above in customised
+ ways.</p>
</section>
-
+
<directivesynopsis>
<name>CacheEnable</name>
<description>Enable caching of specified URLs using a specified storage
Modified: httpd/httpd/trunk/modules/cache/cache_storage.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/cache/cache_storage.c?rev=1070179&r1=1070178&r2=1070179&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/cache/cache_storage.c (original)
+++ httpd/httpd/trunk/modules/cache/cache_storage.c Sun Feb 13 02:03:29 2011
@@ -389,6 +389,53 @@ int cache_select(cache_request_rec *cach
return DECLINED;
}
+/*
+ * invalidate a specific URL entity in all caches
+ *
+ * All cached entities for this URL are removed, usually in
+ * response to a POST/PUT or DELETE.
+ *
+ * This function returns OK if at least one entity was found and
+ * removed, and DECLINED if no cached entities were removed.
+ */
+int cache_invalidate(cache_request_rec *cache, request_rec *r)
+{
+ cache_provider_list *list;
+ apr_status_t rv, status = DECLINED;
+ cache_handle_t *h;
+
+ if (!cache) {
+ /* This should never happen */
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, APR_EGENERAL, r,
+ "cache: No cache request information available for key"
+ " generation");
+ return DECLINED;
+ }
+
+ if (!cache->key) {
+ rv = cache_generate_key(r, r->pool, &cache->key);
+ if (rv != APR_SUCCESS) {
+ return DECLINED;
+ }
+ }
+
+ /* go through the cache types */
+ h = apr_palloc(r->pool, sizeof(cache_handle_t));
+
+ list = cache->providers;
+
+ while (list) {
+ rv = list->provider->open_entity(h, r, cache->key);
+ if (OK == rv) {
+ list->provider->remove_url(h, r);
+ status = OK;
+ }
+ list = list->next;
+ }
+
+ return status;
+}
+
apr_status_t cache_generate_key_default(request_rec *r, apr_pool_t* p,
const char **key)
{
Modified: httpd/httpd/trunk/modules/cache/cache_storage.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/cache/cache_storage.h?rev=1070179&r1=1070178&r2=1070179&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/cache/cache_storage.h (original)
+++ httpd/httpd/trunk/modules/cache/cache_storage.h Sun Feb 13 02:03:29 2011
@@ -40,6 +40,20 @@ int cache_remove_url(cache_request_rec *
int cache_create_entity(cache_request_rec *cache, request_rec *r,
apr_off_t size, apr_bucket_brigade *in);
int cache_select(cache_request_rec *cache, request_rec *r);
+
+/**
+ * invalidate a specific URL entity in all caches
+ *
+ * All cached entities for this URL are removed, usually in
+ * response to a POST/PUT or DELETE.
+ *
+ * This function returns OK if at least one entity was found and
+ * removed, and DECLINED if no cached entities were removed.
+ * @param h cache_handle_t
+ * @param r request_rec
+ */
+int cache_invalidate(cache_request_rec *cache, request_rec *r);
+
apr_status_t cache_generate_key_default(request_rec *r, apr_pool_t* p,
const char **key);
Modified: httpd/httpd/trunk/modules/cache/mod_cache.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/cache/mod_cache.c?rev=1070179&r1=1070178&r2=1070179&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/cache/mod_cache.c (original)
+++ httpd/httpd/trunk/modules/cache/mod_cache.c Sun Feb 13 02:03:29 2011
@@ -75,11 +75,6 @@ static int cache_quick_handler(request_r
ap_filter_rec_t *cache_out_handle;
cache_server_conf *conf;
- /* Delay initialization until we know we are handling a GET */
- if (r->method_number != M_GET) {
- return DECLINED;
- }
-
conf = (cache_server_conf *) ap_get_module_config(r->server->module_config,
&cache_module);
@@ -117,6 +112,25 @@ static int cache_quick_handler(request_r
return DECLINED;
}
+ /* Are we something other than GET or HEAD? If so, invalidate
+ * the cached entities.
+ */
+ if (r->method_number != M_GET) {
+
+ ap_log_rerror(APLOG_MARK, APLOG_DEBUG, APR_SUCCESS, r,
+ "Invalidating all cached entities in response to '%s' request for %s",
+ r->method, r->uri);
+
+ cache_invalidate(cache, r);
+
+ /* we've got a cache invalidate! tell everyone who cares */
+ cache_run_cache_status(cache->handle, r, r->headers_out,
+ AP_CACHE_INVALIDATE, apr_psprintf(r->pool,
+ "cache invalidated by %s", r->method));
+
+ return DECLINED;
+ }
+
/*
* Try to serve this request from the cache.
*
@@ -176,9 +190,10 @@ static int cache_quick_handler(request_r
* is available later during running the filter may be
* different due to an internal redirect.
*/
- cache->remove_url_filter =
- ap_add_output_filter_handle(cache_remove_url_filter_handle,
- cache, r, r->connection);
+ cache->remove_url_filter = ap_add_output_filter_handle(
+ cache_remove_url_filter_handle, cache, r,
+ r->connection);
+
}
else {
ap_log_rerror(APLOG_MARK, APLOG_DEBUG, rv,
@@ -356,11 +371,6 @@ static int cache_handler(request_rec *r)
ap_filter_rec_t *cache_save_handle;
cache_server_conf *conf;
- /* Delay initialization until we know we are handling a GET */
- if (r->method_number != M_GET) {
- return DECLINED;
- }
-
conf = (cache_server_conf *) ap_get_module_config(r->server->module_config,
&cache_module);
@@ -384,6 +394,26 @@ static int cache_handler(request_rec *r)
/* save away the possible providers */
cache->providers = providers;
+ /* Are we something other than GET or HEAD? If so, invalidate
+ * the cached entities.
+ */
+ if (r->method_number != M_GET) {
+
+ ap_log_rerror(APLOG_MARK, APLOG_DEBUG, APR_SUCCESS, r,
+ "Invalidating all cached entities in response to '%s' request for %s",
+ r->method, r->uri);
+
+ cache_invalidate(cache, r);
+
+ /* we've got a cache invalidate! tell everyone who cares */
+ cache_run_cache_status(cache->handle, r, r->headers_out,
+ AP_CACHE_INVALIDATE, apr_psprintf(r->pool,
+ "cache invalidated by %s", r->method));
+
+ return DECLINED;
+
+ }
+
/*
* Try to serve this request from the cache.
*
@@ -464,9 +494,10 @@ static int cache_handler(request_rec *r)
* is available later during running the filter may be
* different due to an internal redirect.
*/
- cache->remove_url_filter =
- ap_add_output_filter_handle(cache_remove_url_filter_handle,
- cache, r, r->connection);
+ cache->remove_url_filter
+ = ap_add_output_filter_handle(
+ cache_remove_url_filter_handle, cache, r,
+ r->connection);
}
else {
@@ -1538,6 +1569,10 @@ static int cache_status(cache_handle_t *
apr_table_setn(r->subprocess_env, AP_CACHE_MISS_ENV, reason);
break;
}
+ case AP_CACHE_INVALIDATE: {
+ apr_table_setn(r->subprocess_env, AP_CACHE_INVALIDATE_ENV, reason);
+ break;
+ }
}
apr_table_setn(r->subprocess_env, AP_CACHE_STATUS_ENV, reason);
@@ -1549,11 +1584,11 @@ static int cache_status(cache_handle_t *
x_cache = conf->x_cache;
}
if (x_cache) {
- apr_table_setn(headers, "X-Cache",
- apr_psprintf(r->pool, "%s from %s",
- status == AP_CACHE_HIT ? "HIT" : status
- == AP_CACHE_REVALIDATE ? "REVALIDATE" : "MISS",
- r->server->server_hostname));
+ apr_table_setn(headers, "X-Cache", apr_psprintf(r->pool, "%s from %s",
+ status == AP_CACHE_HIT ? "HIT"
+ : status == AP_CACHE_REVALIDATE ? "REVALIDATE" : status
+ == AP_CACHE_INVALIDATE ? "INVALIDATE" : "MISS",
+ r->server->server_hostname));
}
if (dconf && dconf->x_cache_detail_set) {
Modified: httpd/httpd/trunk/modules/cache/mod_cache.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/cache/mod_cache.h?rev=1070179&r1=1070178&r2=1070179&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/cache/mod_cache.h (original)
+++ httpd/httpd/trunk/modules/cache/mod_cache.h Sun Feb 13 02:03:29 2011
@@ -114,12 +114,14 @@ typedef struct {
typedef enum {
AP_CACHE_HIT,
AP_CACHE_REVALIDATE,
- AP_CACHE_MISS
+ AP_CACHE_MISS,
+ AP_CACHE_INVALIDATE
} ap_cache_status_e;
#define AP_CACHE_HIT_ENV "cache-hit"
#define AP_CACHE_REVALIDATE_ENV "cache-revalidate"
#define AP_CACHE_MISS_ENV "cache-miss"
+#define AP_CACHE_INVALIDATE_ENV "cache-invalidate"
#define AP_CACHE_STATUS_ENV "cache-status"
Re: svn commit: r1070179 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_cache.xml
modules/cache/cache_storage.c modules/cache/cache_storage.h modules/cache/mod_cache.c
modules/cache/mod_cache.h
Posted by Ruediger Pluem <rp...@apache.org>.
On 02/13/2011 11:29 PM, Graham Leggett wrote:
> On 13 Feb 2011, at 5:08 PM, Ruediger Pluem wrote:
>
>>> +/*
>>> + * invalidate a specific URL entity in all caches
>>> + *
>>> + * All cached entities for this URL are removed, usually in
>>> + * response to a POST/PUT or DELETE.
>>> + *
>>> + * This function returns OK if at least one entity was found and
>>> + * removed, and DECLINED if no cached entities were removed.
>>> + */
>>> +int cache_invalidate(cache_request_rec *cache, request_rec *r)
>>
>> Why not adjusting cache_remove_url accordingly? It does nearly the
>> same and
>> has the same prototype.
>
> Nearly the same, but not enough. cache_remove_url() only takes effect if
> a previous cached entry was found, while cache_invalidate() is
> unconditional.
I guess current cache_remove_url has a bug that you correctly pointed to with
cache_invalidate:
while(list) {
list->provider->remove_url(h, r);
list = list->next;
}
looks wrong because we must pass a provider specific h to each remove_url call.
So I think the
rv = list->provider->open_entity(h, r, cache->key);
step would be needed in remove_url as well, which of course makes
if (!cache->key) {
rv = cache_generate_key(r, r->pool, &cache->key);
if (rv != APR_SUCCESS) {
return DECLINED;
}
}
needed there as well.
Or cache_remove_url should be changed that it only flushes the cache entry in the
backend original selected (cache->provider).
Regards
Rüdiger
Re: svn commit: r1070179 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_cache.xml modules/cache/cache_storage.c modules/cache/cache_storage.h modules/cache/mod_cache.c modules/cache/mod_cache.h
Posted by Graham Leggett <mi...@sharp.fm>.
On 13 Feb 2011, at 5:08 PM, Ruediger Pluem wrote:
>> +/*
>> + * invalidate a specific URL entity in all caches
>> + *
>> + * All cached entities for this URL are removed, usually in
>> + * response to a POST/PUT or DELETE.
>> + *
>> + * This function returns OK if at least one entity was found and
>> + * removed, and DECLINED if no cached entities were removed.
>> + */
>> +int cache_invalidate(cache_request_rec *cache, request_rec *r)
>
> Why not adjusting cache_remove_url accordingly? It does nearly the
> same and
> has the same prototype.
Nearly the same, but not enough. cache_remove_url() only takes effect
if a previous cached entry was found, while cache_invalidate() is
unconditional.
As it turns out, invalidating the entry unconditionally is wrong, as
we need to do the invalidation only on 2xx. This means this code needs
to be changed, waiting for Roy to confirm the exact behaviour before
changing it.
Regards,
Graham
--
Re: svn commit: r1070179 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_cache.xml
modules/cache/cache_storage.c modules/cache/cache_storage.h modules/cache/mod_cache.c
modules/cache/mod_cache.h
Posted by Ruediger Pluem <rp...@apache.org>.
On 02/13/2011 03:03 AM, minfrin@apache.org wrote:
> Author: minfrin
> Date: Sun Feb 13 02:03:29 2011
> New Revision: 1070179
>
> URL: http://svn.apache.org/viewvc?rev=1070179&view=rev
> Log:
> mod_cache: When a request other than GET or HEAD arrives, we must
> invalidate existing cache entities as per RFC2616 13.10. PR 15868.
>
> Modified:
> httpd/httpd/trunk/CHANGES
> httpd/httpd/trunk/docs/manual/mod/mod_cache.xml
> httpd/httpd/trunk/modules/cache/cache_storage.c
> httpd/httpd/trunk/modules/cache/cache_storage.h
> httpd/httpd/trunk/modules/cache/mod_cache.c
> httpd/httpd/trunk/modules/cache/mod_cache.h
>
> Modified: httpd/httpd/trunk/modules/cache/cache_storage.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/cache/cache_storage.c?rev=1070179&r1=1070178&r2=1070179&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/modules/cache/cache_storage.c (original)
> +++ httpd/httpd/trunk/modules/cache/cache_storage.c Sun Feb 13 02:03:29 2011
> @@ -389,6 +389,53 @@ int cache_select(cache_request_rec *cach
> return DECLINED;
> }
>
> +/*
> + * invalidate a specific URL entity in all caches
> + *
> + * All cached entities for this URL are removed, usually in
> + * response to a POST/PUT or DELETE.
> + *
> + * This function returns OK if at least one entity was found and
> + * removed, and DECLINED if no cached entities were removed.
> + */
> +int cache_invalidate(cache_request_rec *cache, request_rec *r)
Why not adjusting cache_remove_url accordingly? It does nearly the same and
has the same prototype.
Regards
Rüdiger
Re: svn commit: r1070179 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_cache.xml
modules/cache/cache_storage.c modules/cache/cache_storage.h modules/cache/mod_cache.c
modules/cache/mod_cache.h
Posted by Ruediger Pluem <rp...@apache.org>.
On 02/14/2011 01:23 AM, Graham Leggett wrote:
> On 14 Feb 2011, at 2:15 AM, Paul Querna wrote:
>
>> It does a single request to the backend, but doesn't _invalidate_ the
>> existing cache, which would cause a flood of other, non-attacker
>> clients to come in.
>
> I think that would be the origin of Roy saying that we should only
> invalidate if the result is 2xx. Someone trying methods in the hope they
> would do something would get a 405 Method Not Supported.
Not sure about Roy, but I guess Pauls fear was more about POST which could be handled
by the backend in the same way as GET and hence would provide an easy remote way
to remove cache entries. Of course one could argue that the backend application
should be fixed then, but I guess we have many examples where the flexibility of
httpd makes it possible to work around bugs in backends where they are not fixable
for whatever reason :-).
Regards
Rüdiger
Re: svn commit: r1070179 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_cache.xml modules/cache/cache_storage.c modules/cache/cache_storage.h modules/cache/mod_cache.c modules/cache/mod_cache.h
Posted by Graham Leggett <mi...@sharp.fm>.
On 14 Feb 2011, at 2:15 AM, Paul Querna wrote:
> It does a single request to the backend, but doesn't _invalidate_ the
> existing cache, which would cause a flood of other, non-attacker
> clients to come in.
I think that would be the origin of Roy saying that we should only
invalidate if the result is 2xx. Someone trying methods in the hope
they would do something would get a 405 Method Not Supported.
Regards,
Graham
--
Re: svn commit: r1070179 - in /httpd/httpd/trunk: CHANGES
docs/manual/mod/mod_cache.xml modules/cache/cache_storage.c
modules/cache/cache_storage.h modules/cache/mod_cache.c modules/cache/mod_cache.h
Posted by Paul Querna <pa...@querna.org>.
On Sun, Feb 13, 2011 at 4:00 PM, Graham Leggett <mi...@sharp.fm> wrote:
> On 14 Feb 2011, at 1:56 AM, Paul Querna wrote:
>
>> Additionally, this should be a configurable behavior.
>>
>> Lets say you run a popular website that depends on mod_cache to
>> protect backend systems from complete overload.
>>
>> All you need to do now as an attacker is POST / DELETE to / or another
>> important URL every 200ms, and the cache becomes invalidated, causing
>> a flood of requests to backends that might not be able to support it.
>>
>> Thoughts?
>
> How is this different from "Cache-Control: no-cache" in the request?
It does a single request to the backend, but doesn't _invalidate_ the
existing cache, which would cause a flood of other, non-attacker
clients to come in.
Re: svn commit: r1070179 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_cache.xml modules/cache/cache_storage.c modules/cache/cache_storage.h modules/cache/mod_cache.c modules/cache/mod_cache.h
Posted by Graham Leggett <mi...@sharp.fm>.
On 14 Feb 2011, at 1:56 AM, Paul Querna wrote:
> Additionally, this should be a configurable behavior.
>
> Lets say you run a popular website that depends on mod_cache to
> protect backend systems from complete overload.
>
> All you need to do now as an attacker is POST / DELETE to / or another
> important URL every 200ms, and the cache becomes invalidated, causing
> a flood of requests to backends that might not be able to support it.
>
> Thoughts?
How is this different from "Cache-Control: no-cache" in the request?
Regards,
Graham
--
Re: svn commit: r1070179 - in /httpd/httpd/trunk: CHANGES
docs/manual/mod/mod_cache.xml modules/cache/cache_storage.c
modules/cache/cache_storage.h modules/cache/mod_cache.c modules/cache/mod_cache.h
Posted by Paul Querna <pa...@querna.org>.
On Sun, Feb 13, 2011 at 5:03 AM, Graham Leggett <mi...@sharp.fm> wrote:
> On 13 Feb 2011, at 9:59 AM, Roy T. Fielding wrote:
>
>>> URL: http://svn.apache.org/viewvc?rev=1070179&view=rev
>>> Log:
>>> mod_cache: When a request other than GET or HEAD arrives, we must
>>> invalidate existing cache entities as per RFC2616 13.10. PR 15868.
>>
>> Cache entries should not be invalidated unless the response comes
>> back as a success (2xx). Likewise, it only applies to methods with
>> known write effects (not including TRACE, OPTIONS, PROPFIND, etc.).
>>
>> This has already been updated in httpbis p6.
>
> Hmmm...
>
> So is it right to say that POST, PUT and DELETE should invalidate, while all
> others would be left alone?
>
> Or do we need some kind of mechanism for modules that respond to new methods
> to declare those methods as needing to be invalidated or not?
>
> Should the cache try and cache any of the other methods, or is cacheability
> limited to GET only?
Additionally, this should be a configurable behavior.
Lets say you run a popular website that depends on mod_cache to
protect backend systems from complete overload.
All you need to do now as an attacker is POST / DELETE to / or another
important URL every 200ms, and the cache becomes invalidated, causing
a flood of requests to backends that might not be able to support it.
Thoughts?
Thanks,
Paul
Re: svn commit: r1070179 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_cache.xml modules/cache/cache_storage.c modules/cache/cache_storage.h modules/cache/mod_cache.c modules/cache/mod_cache.h
Posted by "Roy T. Fielding" <fi...@gbiv.com>.
On Feb 13, 2011, at 5:03 AM, Graham Leggett wrote:
> On 13 Feb 2011, at 9:59 AM, Roy T. Fielding wrote:
>
>>> URL: http://svn.apache.org/viewvc?rev=1070179&view=rev
>>> Log:
>>> mod_cache: When a request other than GET or HEAD arrives, we must
>>> invalidate existing cache entities as per RFC2616 13.10. PR 15868.
>>
>> Cache entries should not be invalidated unless the response comes
>> back as a success (2xx). Likewise, it only applies to methods with
>> known write effects (not including TRACE, OPTIONS, PROPFIND, etc.).
>>
>> This has already been updated in httpbis p6.
>
> Hmmm...
>
> So is it right to say that POST, PUT and DELETE should invalidate, while all others would be left alone?
>
> Or do we need some kind of mechanism for modules that respond to new methods to declare those methods as needing to be invalidated or not?
Well, I thought I could answer that with a simple link to the
drafts, but they just got changed in the wrong direction.
It should be implemented as invalidating when the response
is 2xx and the method is *not* one of the safe ones
(GET, HEAD, OPTIONS, TRACE, PROPFIND, and maybe REPORT).
That way, a new extension method would invalidate on success.
> Should the cache try and cache any of the other methods, or is cacheability limited to GET only?
It can cache other methods if it knows how to. PROPFIND is
probably the only other method worth caching, but its key
depends on a bunch of webdav stuff outside the URI.
....Roy
Re: svn commit: r1070179 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_cache.xml modules/cache/cache_storage.c modules/cache/cache_storage.h modules/cache/mod_cache.c modules/cache/mod_cache.h
Posted by Graham Leggett <mi...@sharp.fm>.
On 13 Feb 2011, at 9:59 AM, Roy T. Fielding wrote:
>> URL: http://svn.apache.org/viewvc?rev=1070179&view=rev
>> Log:
>> mod_cache: When a request other than GET or HEAD arrives, we must
>> invalidate existing cache entities as per RFC2616 13.10. PR 15868.
>
> Cache entries should not be invalidated unless the response comes
> back as a success (2xx). Likewise, it only applies to methods with
> known write effects (not including TRACE, OPTIONS, PROPFIND, etc.).
>
> This has already been updated in httpbis p6.
Hmmm...
So is it right to say that POST, PUT and DELETE should invalidate,
while all others would be left alone?
Or do we need some kind of mechanism for modules that respond to new
methods to declare those methods as needing to be invalidated or not?
Should the cache try and cache any of the other methods, or is
cacheability limited to GET only?
Regards,
Graham
--
Re: svn commit: r1070179 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_cache.xml modules/cache/cache_storage.c modules/cache/cache_storage.h modules/cache/mod_cache.c modules/cache/mod_cache.h
Posted by Graham Leggett <mi...@sharp.fm>.
On 30 Nov 2011, at 1:30 PM, Graham Leggett wrote:
> The change has been reverted on v2.4, and the following patch gives the API changes that allow us to fix this in the v2.4 series:
Applied in r 1208822 and backported in r1208824.
Regards,
Graham
--
Re: svn commit: r1070179 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_cache.xml modules/cache/cache_storage.c modules/cache/cache_storage.h modules/cache/mod_cache.c modules/cache/mod_cache.h
Posted by Graham Leggett <mi...@sharp.fm>.
On 23 Nov 2011, at 1:15 PM, Graham Leggett wrote:
> In order to make invalidation possible, we would need to add an invalidate() function to the mod_cache provider, and keep AP_CACHE_INVALIDATE inside ap_cache_status_e. Invalidation involves marking the entry as invalidated and "to be freshened on next hit", as opposed to deleted.
>
> What I could do is back out the change, and make just the above API changes, allowing us to backport this to v2.4 when we are ready.
>
> This releases v2.4 to get on with it, and keeps the option open for us to fix this properly at a future date in the v2.4 cycle when we're ready.
The change has been reverted on v2.4, and the following patch gives the API changes that allow us to fix this in the v2.4 series:
Index: modules/cache/mod_cache.h
===================================================================
--- modules/cache/mod_cache.h (revision 1208384)
+++ modules/cache/mod_cache.h (working copy)
@@ -109,17 +109,20 @@
const char *urlkey);
int (*remove_url) (cache_handle_t *h, request_rec *r);
apr_status_t (*commit_entity)(cache_handle_t *h, request_rec *r);
+ apr_status_t (*invalidate_entity)(cache_handle_t *h, request_rec *r);
} cache_provider;
typedef enum {
AP_CACHE_HIT,
AP_CACHE_REVALIDATE,
- AP_CACHE_MISS
+ AP_CACHE_MISS,
+ AP_CACHE_INVALIDATE
} ap_cache_status_e;
#define AP_CACHE_HIT_ENV "cache-hit"
#define AP_CACHE_REVALIDATE_ENV "cache-revalidate"
#define AP_CACHE_MISS_ENV "cache-miss"
+#define AP_CACHE_INVALIDATE_ENV "cache-invalidate"
#define AP_CACHE_STATUS_ENV "cache-status"
Index: modules/cache/mod_cache_disk.c
===================================================================
--- modules/cache/mod_cache_disk.c (revision 1208379)
+++ modules/cache/mod_cache_disk.c (working copy)
@@ -1325,6 +1325,11 @@
return APR_SUCCESS;
}
+static apr_status_t invalidate_entity(cache_handle_t *h, request_rec *r)
+{
+ return APR_ENOTIMPL;
+}
+
static void *create_dir_config(apr_pool_t *p, char *dummy)
{
disk_cache_dir_conf *dconf = apr_pcalloc(p, sizeof(disk_cache_dir_conf));
@@ -1502,7 +1507,8 @@
&create_entity,
&open_entity,
&remove_url,
- &commit_entity
+ &commit_entity,
+ &invalidate_entity
};
static void disk_cache_register_hook(apr_pool_t *p)
Regards,
Graham
--
Re: svn commit: r1070179 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_cache.xml modules/cache/cache_storage.c modules/cache/cache_storage.h modules/cache/mod_cache.c modules/cache/mod_cache.h
Posted by Graham Leggett <mi...@sharp.fm>.
On 22 Nov 2011, at 11:58 PM, Stefan Fritsch wrote:
> From the thread, it is not clear to me that there was consensous on
> how to fix it. Therefore I would vote for reverting r1070179 from 2.4,
> fixing it in trunk, and then backporting the whole thing to 2.4 again
> (maybe after 2.4.0).
In order to make invalidation possible, we would need to add an
invalidate() function to the mod_cache provider, and keep
AP_CACHE_INVALIDATE inside ap_cache_status_e. Invalidation involves
marking the entry as invalidated and "to be freshened on next hit", as
opposed to deleted.
What I could do is back out the change, and make just the above API
changes, allowing us to backport this to v2.4 when we are ready.
This releases v2.4 to get on with it, and keeps the option open for us
to fix this properly at a future date in the v2.4 cycle when we're
ready.
Thoughts?
Regards,
Graham
--
Re: svn commit: r1070179 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_cache.xml modules/cache/cache_storage.c modules/cache/cache_storage.h modules/cache/mod_cache.c modules/cache/mod_cache.h
Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Tuesday 22 November 2011, Graham Leggett wrote:
> On 13 Feb 2011, at 9:59 AM, Roy T. Fielding wrote:
> > On Feb 12, 2011, at 6:03 PM, minfrin@apache.org wrote:
> >> Author: minfrin
> >> Date: Sun Feb 13 02:03:29 2011
> >> New Revision: 1070179
> >>
> >> URL: http://svn.apache.org/viewvc?rev=1070179&view=rev
> >> Log:
> >> mod_cache: When a request other than GET or HEAD arrives, we
> >> must invalidate existing cache entities as per RFC2616 13.10.
> >> PR 15868.
> >
> > Cache entries should not be invalidated unless the response comes
> > back as a success (2xx). Likewise, it only applies to methods
> > with known write effects (not including TRACE, OPTIONS,
> > PROPFIND, etc.).
> >
> > This has already been updated in httpbis p6.
>
> Reviving this unresolved issue.
>
> I need to update r1070179 to fix this, do I a) revert r1070179 on
> v2.4.x and continue to fix on trunk, or b) fix this on trunk and
> v2.4.x at the risk of potential further delay?
From the thread, it is not clear to me that there was consensous on
how to fix it. Therefore I would vote for reverting r1070179 from 2.4,
fixing it in trunk, and then backporting the whole thing to 2.4 again
(maybe after 2.4.0).
Re: svn commit: r1070179 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_cache.xml modules/cache/cache_storage.c modules/cache/cache_storage.h modules/cache/mod_cache.c modules/cache/mod_cache.h
Posted by Graham Leggett <mi...@sharp.fm>.
On 13 Feb 2011, at 9:59 AM, Roy T. Fielding wrote:
> On Feb 12, 2011, at 6:03 PM, minfrin@apache.org wrote:
>
>> Author: minfrin
>> Date: Sun Feb 13 02:03:29 2011
>> New Revision: 1070179
>>
>> URL: http://svn.apache.org/viewvc?rev=1070179&view=rev
>> Log:
>> mod_cache: When a request other than GET or HEAD arrives, we must
>> invalidate existing cache entities as per RFC2616 13.10. PR 15868.
>
> Cache entries should not be invalidated unless the response comes
> back as a success (2xx). Likewise, it only applies to methods with
> known write effects (not including TRACE, OPTIONS, PROPFIND, etc.).
>
> This has already been updated in httpbis p6.
Reviving this unresolved issue.
I need to update r1070179 to fix this, do I a) revert r1070179 on
v2.4.x and continue to fix on trunk, or b) fix this on trunk and
v2.4.x at the risk of potential further delay?
Regards,
Graham
--
Re: svn commit: r1070179 - in /httpd/httpd/trunk: CHANGES docs/manual/mod/mod_cache.xml modules/cache/cache_storage.c modules/cache/cache_storage.h modules/cache/mod_cache.c modules/cache/mod_cache.h
Posted by "Roy T. Fielding" <fi...@gbiv.com>.
On Feb 12, 2011, at 6:03 PM, minfrin@apache.org wrote:
> Author: minfrin
> Date: Sun Feb 13 02:03:29 2011
> New Revision: 1070179
>
> URL: http://svn.apache.org/viewvc?rev=1070179&view=rev
> Log:
> mod_cache: When a request other than GET or HEAD arrives, we must
> invalidate existing cache entities as per RFC2616 13.10. PR 15868.
Cache entries should not be invalidated unless the response comes
back as a success (2xx). Likewise, it only applies to methods with
known write effects (not including TRACE, OPTIONS, PROPFIND, etc.).
This has already been updated in httpbis p6.
....Roy