You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by sf...@apache.org on 2012/05/29 21:55:38 UTC
svn commit: r1343951 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS
modules/cache/mod_cache.c modules/cache/mod_disk_cache.c
modules/cache/mod_mem_cache.c
Author: sf
Date: Tue May 29 19:55:37 2012
New Revision: 1343951
URL: http://svn.apache.org/viewvc?rev=1343951&view=rev
Log:
Merge r933919, r951222:
* Do not cache 206 responses in any case since we currently do not provide any
backends that are capable to cache partial responses. PR 49113.
Fixes regression of r724093.
Submitted by: minfrin
Reviewed by: rjung, wrowe, sf
Modified:
httpd/httpd/branches/2.2.x/CHANGES
httpd/httpd/branches/2.2.x/STATUS
httpd/httpd/branches/2.2.x/modules/cache/mod_cache.c
httpd/httpd/branches/2.2.x/modules/cache/mod_disk_cache.c
httpd/httpd/branches/2.2.x/modules/cache/mod_mem_cache.c
Modified: httpd/httpd/branches/2.2.x/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/CHANGES?rev=1343951&r1=1343950&r2=1343951&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/CHANGES [utf-8] (original)
+++ httpd/httpd/branches/2.2.x/CHANGES [utf-8] Tue May 29 19:55:37 2012
@@ -5,6 +5,11 @@ Changes with Apache 2.2.23
envvars: Fix insecure handling of LD_LIBRARY_PATH that could lead to the
current working directory to be searched for DSOs. [Stefan Fritsch]
+ *) mod_disk_cache, mod_mem_cache: Decline the opportunity to cache if the
+ response is a 206 Partial Content. This stops a reverse proxied partial
+ response from becoming cached, and then being served in subsequent
+ responses. [Graham Leggett]
+
*) configure: Fix usage with external apr and apu in non-default paths
and recent gcc versions >= 4.6. [Jean-Frederic Clere]
Modified: httpd/httpd/branches/2.2.x/STATUS
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/STATUS?rev=1343951&r1=1343950&r2=1343951&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/STATUS (original)
+++ httpd/httpd/branches/2.2.x/STATUS Tue May 29 19:55:37 2012
@@ -93,20 +93,6 @@ RELEASE SHOWSTOPPERS:
PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
[ start all new proposals below, under PATCHES PROPOSED. ]
- * mod_cache: * Do not cache 206 responses in any case since we currently do not provide any
- backends that are capable to cache partial responses. PR 49113.
- Fixes regression of r724093.
- Trunk version of patch:
- http://svn.apache.org/viewcvs.cgi?rev=933919&view=rev
- Backport version for 2.2.x of patch:
- Trunk version of patch works
- +1: rpluem
- minfrin: The r933919 patch is obsolete, as this trunk code no longer
- exists. The fix you're after is r951222, backported to both
- mod_disk_cache.c and mod_mem_cache.c.
- rjung: I combined backports of r933919 and r951222:
- http://people.apache.org/~rjung/patches/mod_cache_partial_content-2.2.x.patch
- +1: rjung, wrowe, sf
PATCHES PROPOSED TO BACKPORT FROM TRUNK:
[ New proposals should be added at the end of the list ]
Modified: httpd/httpd/branches/2.2.x/modules/cache/mod_cache.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/modules/cache/mod_cache.c?rev=1343951&r1=1343950&r2=1343951&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/modules/cache/mod_cache.c (original)
+++ httpd/httpd/branches/2.2.x/modules/cache/mod_cache.c Tue May 29 19:55:37 2012
@@ -473,7 +473,8 @@ static int cache_save_filter(ap_filter_t
* We include 304 Not Modified here too as this is the origin server
* telling us to serve the cached copy.
*/
- if (exps != NULL || cc_out != NULL) {
+ if ((exps != NULL || cc_out != NULL)
+ && r->status != HTTP_PARTIAL_CONTENT) {
/* We are also allowed to cache any response given that it has a
* valid Expires or Cache Control header. If we find a either of
* those here, we pass request through the rest of the tests. From
@@ -486,6 +487,9 @@ static int cache_save_filter(ap_filter_t
* include the following: an Expires header (section 14.21); a
* "max-age", "s-maxage", "must-revalidate", "proxy-revalidate",
* "public" or "private" cache-control directive (section 14.9).
+ *
+ * But do NOT store 206 responses in any case since we
+ * don't (yet) cache partial responses.
*/
}
else {
Modified: httpd/httpd/branches/2.2.x/modules/cache/mod_disk_cache.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/modules/cache/mod_disk_cache.c?rev=1343951&r1=1343950&r2=1343951&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/modules/cache/mod_disk_cache.c (original)
+++ httpd/httpd/branches/2.2.x/modules/cache/mod_disk_cache.c Tue May 29 19:55:37 2012
@@ -330,6 +330,14 @@ static int create_entity(cache_handle_t
return DECLINED;
}
+ /* we don't support caching of range requests (yet) */
+ if (r->status == HTTP_PARTIAL_CONTENT) {
+ ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
+ "disk_cache: URL %s partial content response not cached",
+ key);
+ return DECLINED;
+ }
+
/* Allocate and initialize cache_object_t and disk_cache_object_t */
h->cache_obj = obj = apr_pcalloc(r->pool, sizeof(*obj));
obj->vobj = dobj = apr_pcalloc(r->pool, sizeof(*dobj));
Modified: httpd/httpd/branches/2.2.x/modules/cache/mod_mem_cache.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/modules/cache/mod_mem_cache.c?rev=1343951&r1=1343950&r2=1343951&view=diff
==============================================================================
--- httpd/httpd/branches/2.2.x/modules/cache/mod_mem_cache.c (original)
+++ httpd/httpd/branches/2.2.x/modules/cache/mod_mem_cache.c Tue May 29 19:55:37 2012
@@ -313,6 +313,14 @@ static int create_entity(cache_handle_t
cache_object_t *obj, *tmp_obj;
mem_cache_object_t *mobj;
+ /* we don't support caching of range requests (yet) */
+ if (r->status == HTTP_PARTIAL_CONTENT) {
+ ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
+ "disk_cache: URL %s partial content response not cached",
+ key);
+ return DECLINED;
+ }
+
if (len == -1) {
/* Caching a streaming response. Assume the response is
* less than or equal to max_streaming_buffer_size. We will
Re: svn commit: r1343951 - in /httpd/httpd/branches/2.2.x: CHANGES
STATUS modules/cache/mod_cache.c modules/cache/mod_disk_cache.c modules/cache/mod_mem_cache.c
Posted by Ruediger Pluem <rp...@apache.org>.
Stefan Fritsch wrote:
> On Friday 01 June 2012, Ruediger Pluem wrote:
>> sf@apache.org wrote:
>>> Author: sf
>>> Date: Tue May 29 19:55:37 2012
>>> New Revision: 1343951
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1343951&view=rev
>>> Log:
>>> Merge r933919, r951222:
>>>
>>> * Do not cache 206 responses in any case since we currently do
>>> not provide any
>>>
>>> backends that are capable to cache partial responses. PR 49113.
>>> Fixes regression of r724093.
>>>
>>> Submitted by: minfrin
>>> Reviewed by: rjung, wrowe, sf
>>>
>
>
>> Sorry for chiming in that late
>> I don't think that this is the correct combination of the
>> backports. The r->status != HTTP_PARTIAL_CONTENT needs to move up
>> one if condition such that the check allows to cache partial
>> responses in any case and let the providers deny it (like in trunk
>> and 2.4). The current patch never forwards a partial response to
>> the providers and hence never triggers the code in the providers.
>
> Yes, I have noticed that the proposed patch basically forbids caching
> 206 responses, twice. I have still voted +1 because it is a
> significant improvement (inefficient caching versus corrupt response)
> and because it seems very unlikely that any of the 2.2 providers will
> get support for 206 (considering that 2.4 is out).
>
> If anyone thinks this is (or may become) a problem, we can backport
> r952823, too.
Sounds like a good plan. Keeps 2.2.x and trunk more in sync.
Regards
Rüdiger
Re: svn commit: r1343951 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS modules/cache/mod_cache.c modules/cache/mod_disk_cache.c modules/cache/mod_mem_cache.c
Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Friday 01 June 2012, Ruediger Pluem wrote:
> sf@apache.org wrote:
> > Author: sf
> > Date: Tue May 29 19:55:37 2012
> > New Revision: 1343951
> >
> > URL: http://svn.apache.org/viewvc?rev=1343951&view=rev
> > Log:
> > Merge r933919, r951222:
> >
> > * Do not cache 206 responses in any case since we currently do
> > not provide any
> >
> > backends that are capable to cache partial responses. PR 49113.
> > Fixes regression of r724093.
> >
> > Submitted by: minfrin
> > Reviewed by: rjung, wrowe, sf
> >
> Sorry for chiming in that late
> I don't think that this is the correct combination of the
> backports. The r->status != HTTP_PARTIAL_CONTENT needs to move up
> one if condition such that the check allows to cache partial
> responses in any case and let the providers deny it (like in trunk
> and 2.4). The current patch never forwards a partial response to
> the providers and hence never triggers the code in the providers.
Yes, I have noticed that the proposed patch basically forbids caching
206 responses, twice. I have still voted +1 because it is a
significant improvement (inefficient caching versus corrupt response)
and because it seems very unlikely that any of the 2.2 providers will
get support for 206 (considering that 2.4 is out).
If anyone thinks this is (or may become) a problem, we can backport
r952823, too.
Cheers,
Stefan
Re: svn commit: r1343951 - in /httpd/httpd/branches/2.2.x: CHANGES
STATUS modules/cache/mod_cache.c modules/cache/mod_disk_cache.c modules/cache/mod_mem_cache.c
Posted by Ruediger Pluem <rp...@apache.org>.
sf@apache.org wrote:
> Author: sf
> Date: Tue May 29 19:55:37 2012
> New Revision: 1343951
>
> URL: http://svn.apache.org/viewvc?rev=1343951&view=rev
> Log:
> Merge r933919, r951222:
>
> * Do not cache 206 responses in any case since we currently do not provide any
> backends that are capable to cache partial responses. PR 49113.
> Fixes regression of r724093.
>
> Submitted by: minfrin
> Reviewed by: rjung, wrowe, sf
>
> Modified:
> httpd/httpd/branches/2.2.x/CHANGES
> httpd/httpd/branches/2.2.x/STATUS
> httpd/httpd/branches/2.2.x/modules/cache/mod_cache.c
> httpd/httpd/branches/2.2.x/modules/cache/mod_disk_cache.c
> httpd/httpd/branches/2.2.x/modules/cache/mod_mem_cache.c
>
> Modified: httpd/httpd/branches/2.2.x/modules/cache/mod_cache.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/modules/cache/mod_cache.c?rev=1343951&r1=1343950&r2=1343951&view=diff
> ==============================================================================
> --- httpd/httpd/branches/2.2.x/modules/cache/mod_cache.c (original)
> +++ httpd/httpd/branches/2.2.x/modules/cache/mod_cache.c Tue May 29 19:55:37 2012
> @@ -473,7 +473,8 @@ static int cache_save_filter(ap_filter_t
> * We include 304 Not Modified here too as this is the origin server
> * telling us to serve the cached copy.
> */
> - if (exps != NULL || cc_out != NULL) {
> + if ((exps != NULL || cc_out != NULL)
> + && r->status != HTTP_PARTIAL_CONTENT) {
> /* We are also allowed to cache any response given that it has a
> * valid Expires or Cache Control header. If we find a either of
> * those here, we pass request through the rest of the tests. From
> @@ -486,6 +487,9 @@ static int cache_save_filter(ap_filter_t
> * include the following: an Expires header (section 14.21); a
> * "max-age", "s-maxage", "must-revalidate", "proxy-revalidate",
> * "public" or "private" cache-control directive (section 14.9).
> + *
> + * But do NOT store 206 responses in any case since we
> + * don't (yet) cache partial responses.
> */
> }
> else {
>
Sorry for chiming in that late
I don't think that this is the correct combination of the backports.
The r->status != HTTP_PARTIAL_CONTENT needs to move up one if condition such that the check allows to cache partial
responses in any case and let the providers deny it (like in trunk and 2.4).
The current patch never forwards a partial response to the providers and hence never triggers the code in the providers.
Regards
Rüdiger