You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Stefan Fritsch <sf...@sfritsch.de> on 2010/06/21 23:00:03 UTC
Re: mod_deflate DoS using HEAD
On Thursday 16 July 2009, William A. Rowe, Jr. wrote:
> Plüm, Rüdiger, VF-Group wrote:
> > Good point. So your patch would invalidate a cached entity if the
> > response to a GET delivered a C-L header, since HEAD and GET
> > would deliver different C-L headers.
> > OTOH I think only very small or extremely compressable responses
> > (whether static or not) would have a C-L in the response to a
> > GET, because everything that exceeeds a zlib buffer would be
> > delivered chunked anyway.
>
> We don't really want to gzip that single buffer though, either.
> The prime concern here is CPU cycles. In this case, there is no
> advantage to performing that compression, and inconsistent
> behavior leads cache and proxy authors down unfortunate
> assumptions.
Going back to that old thread, was there consensus on which patch is
preferable?
As I understand it, Rüdiger's patch may be better for caching but uses
more CPU cycles. But it uses way less CPU than no patch at all.
Therefore I propose to include that patch unless there is clear
consensus that Eric's patch is to be preferred.
As an added data point, Rüdiger's patch is in Debian 5.0 and various
Ubuntus and AFAIK hasn't caused any issues.
Re: mod_deflate DoS using HEAD
Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Tuesday 22 June 2010, Plüm, Rüdiger, VF-Group wrote:
> I am currently +0 on wether to use the patch above or my original
> proposal. Both have its pros and cons (Saving more CPU vs. be more
> picky about caching and implement an RFC SHOULD).
I have now commited your original patch because it is less likely to
break something. If the CPU usage for compressing the first buffer
turns out to be a problem, we can still change it later.
Cheers,
Stefan
RE: mod_deflate DoS using HEAD
Posted by "Plüm, Rüdiger, VF-Group" <ru...@vodafone.com>.
> -----Original Message-----
> From: Eric Covener
> Sent: Dienstag, 22. Juni 2010 15:33
> To: dev@httpd.apache.org
> Subject: Re: mod_deflate DoS using HEAD
>
> On Tue, Jun 22, 2010 at 2:37 AM, Ruediger Pluem
> <rp...@apache.org> wrote:
> > Does someone still have Eric's patch? Seems that I can't
> find it right now.
>
> This is what I have, but I never got off the fence back then
> much less now:
>
> Index: modules/filters/mod_deflate.c
> ===================================================================
> --- modules/filters/mod_deflate.c (revision 793619)
> +++ modules/filters/mod_deflate.c (working copy)
> @@ -578,7 +578,7 @@
> deflate_check_etag(r, "gzip");
>
> /* For a 304 response, only change the headers */
> - if (r->status == HTTP_NOT_MODIFIED) {
> + if (r->status == HTTP_NOT_MODIFIED || r->header_only) {
> ap_remove_output_filter(f);
> return ap_pass_brigade(f->next, bb);
> }
>
>
Thanks for that. I guess the patch is not complete for current trunk.
IMHO it should look like:
Index: modules/filters/mod_deflate.c
===================================================================
--- modules/filters/mod_deflate.c (revision 955960)
+++ modules/filters/mod_deflate.c (working copy)
@@ -562,7 +562,7 @@
* send out the headers).
*/
- if (r->status != HTTP_NOT_MODIFIED) {
+ if ((r->status != HTTP_NOT_MODIFIED) && !r->header_only) {
ctx = f->ctx = apr_pcalloc(r->pool, sizeof(*ctx));
ctx->bb = apr_brigade_create(r->pool, f->c->bucket_alloc);
ctx->buffer = apr_palloc(r->pool, c->bufferSize);
@@ -616,7 +616,7 @@
deflate_check_etag(r, "gzip");
/* For a 304 response, only change the headers */
- if (r->status == HTTP_NOT_MODIFIED) {
+ if ((r->status == HTTP_NOT_MODIFIED) || r->header_only) {
ap_remove_output_filter(f);
return ap_pass_brigade(f->next, bb);
}
I am currently +0 on wether to use the patch above or my original
proposal. Both have its pros and cons (Saving more CPU vs. be more
picky about caching and implement an RFC SHOULD).
Regards
Rüdiger
Re: mod_deflate DoS using HEAD
Posted by Eric Covener <co...@gmail.com>.
On Tue, Jun 22, 2010 at 2:37 AM, Ruediger Pluem <rp...@apache.org> wrote:
> Does someone still have Eric's patch? Seems that I can't find it right now.
This is what I have, but I never got off the fence back then much less now:
Index: modules/filters/mod_deflate.c
===================================================================
--- modules/filters/mod_deflate.c (revision 793619)
+++ modules/filters/mod_deflate.c (working copy)
@@ -578,7 +578,7 @@
deflate_check_etag(r, "gzip");
/* For a 304 response, only change the headers */
- if (r->status == HTTP_NOT_MODIFIED) {
+ if (r->status == HTTP_NOT_MODIFIED || r->header_only) {
ap_remove_output_filter(f);
return ap_pass_brigade(f->next, bb);
}
--
Eric Covener
covener@gmail.com
Re: mod_deflate DoS using HEAD
Posted by Ruediger Pluem <rp...@apache.org>.
On 06/21/2010 11:37 PM, William A. Rowe Jr. wrote:
> On 6/21/2010 4:00 PM, Stefan Fritsch wrote:
>> As I understand it, Rüdiger's patch may be better for caching but uses
>> more CPU cycles. But it uses way less CPU than no patch at all.
>> Therefore I propose to include that patch unless there is clear
>> consensus that Eric's patch is to be preferred.
>
> Not a significant number, and Rüdiger's patch gathered +1's from myself,
> gregames, nick is on the wall with a +.5 - I think your question is to
> Rüdiger, with the emphasis on 'what is your decision?' based on this
> last rather indecisive posting.
Does someone still have Eric's patch? Seems that I can't find it right now.
Regards
Rüdiger
Re: mod_deflate DoS using HEAD
Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 6/21/2010 4:00 PM, Stefan Fritsch wrote:
>
> As I understand it, Rüdiger's patch may be better for caching but uses
> more CPU cycles. But it uses way less CPU than no patch at all.
> Therefore I propose to include that patch unless there is clear
> consensus that Eric's patch is to be preferred.
Not a significant number, and Rüdiger's patch gathered +1's from myself,
gregames, nick is on the wall with a +.5 - I think your question is to
Rüdiger, with the emphasis on 'what is your decision?' based on this
last rather indecisive posting.
On 7/16/2009 9:24 AM, "Plüm, Rüdiger, VF-Group" wrote:
>>
>> For a large static file, Ruedigers patch suppresses the C-L entirely
>> (it gets added back in down the chain for my patch, for static files
>> at least) which I thought would make that prefered, if we're confident
>> that we'll never do more than a zlib buffer worth of work the first
>> go-round.
>
> Good point. So your patch would invalidate a cached entity if the
> response to a GET delivered a C-L header, since HEAD and GET would
> deliver different C-L headers.
> OTOH I think only very small or extremely compressable responses (whether
> static or not) would have a C-L in the response to a GET, because everything
> that exceeeds a zlib buffer would be delivered chunked anyway.