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 2011/07/13 22:38:33 UTC

svn commit: r1146418 - in /httpd/httpd/trunk: CHANGES modules/filters/mod_deflate.c

Author: sf
Date: Wed Jul 13 20:38:33 2011
New Revision: 1146418

URL: http://svn.apache.org/viewvc?rev=1146418&view=rev
Log:
Don't try to compress requests with a zero sized body.

PR: 51350

Modified:
    httpd/httpd/trunk/CHANGES
    httpd/httpd/trunk/modules/filters/mod_deflate.c

Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/CHANGES?rev=1146418&r1=1146417&r2=1146418&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Wed Jul 13 20:38:33 2011
@@ -2,6 +2,9 @@
 
 Changes with Apache 2.3.14
 
+  *) mod_deflate: Don't try to compress requests with a zero sized body.
+     PR 51350. [Stefan Fritsch]
+
   *) core: Fix startup on IP6-only systems. PR 50592. [Joe Orton,
      <root linkage white-void net>]
 

Modified: httpd/httpd/trunk/modules/filters/mod_deflate.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/filters/mod_deflate.c?rev=1146418&r1=1146417&r2=1146418&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/filters/mod_deflate.c (original)
+++ httpd/httpd/trunk/modules/filters/mod_deflate.c Wed Jul 13 20:38:33 2011
@@ -426,6 +426,8 @@ static apr_status_t deflate_out_filter(a
     request_rec *r = f->r;
     deflate_ctx *ctx = f->ctx;
     int zRC;
+    apr_size_t len;
+    const char *data;
     deflate_filter_config *c;
 
     /* Do nothing if asked to filter nothing. */
@@ -446,6 +448,28 @@ static apr_status_t deflate_out_filter(a
         char *token;
         const char *encoding;
 
+        /* Delay initialization until we have seen some data */
+        e = APR_BRIGADE_FIRST(bb);
+        while (1) {
+            apr_status_t rc;
+            if (e == APR_BRIGADE_SENTINEL(bb))
+                return ap_pass_brigade(f->next, bb);
+            if (APR_BUCKET_IS_EOS(e)) {
+                ap_remove_output_filter(f);
+                return ap_pass_brigade(f->next, bb);
+            }
+            if (APR_BUCKET_IS_METADATA(e))
+                continue;
+
+            rc = apr_bucket_read(e, &data, &len, APR_BLOCK_READ);
+            if (rc != APR_SUCCESS)
+                return rc;
+            if (len > 0)
+                break;
+
+            e = APR_BUCKET_NEXT(e);
+        }
+
         ctx = f->ctx = apr_pcalloc(r->pool, sizeof(*ctx));
 
         /*
@@ -645,9 +669,7 @@ static apr_status_t deflate_out_filter(a
 
     while (!APR_BRIGADE_EMPTY(bb))
     {
-        const char *data;
         apr_bucket *b;
-        apr_size_t len;
 
         /*
          * Optimization: If we are a HEAD request and bytes_sent is not zero



Re: svn commit: r1146418 - in /httpd/httpd/trunk: CHANGES modules/filters/mod_deflate.c

Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Saturday 30 July 2011, Nick Kew wrote:
> On 13 Jul 2011, at 21:38, sf@apache.org wrote:
> > Author: sf
> > Date: Wed Jul 13 20:38:33 2011
> > New Revision: 1146418
> > 
> > URL: http://svn.apache.org/viewvc?rev=1146418&view=rev
> > Log:
> > Don't try to compress requests with a zero sized body.
> > 
> > PR: 51350
> 
> PR 51590 tells us this introduces a new spinning bug.
> 
> Investigating that reveals what looks like a deeper problem:
> > +            rc = apr_bucket_read(e, &data, &len,
> > APR_BLOCK_READ);
> 
> Depending on the bucket type, this read might be discarding vital
> data! Or am I missing something?

AFAIK, apr_bucket_read() should only convert buckets of undetermined 
length into buckets of actual data. But as long as the bucket is not 
removed from the brigade, no data should be lost. The 
apr_bucket_read() is not executed for metadata buckets.

The exact same apr_bucket_read() call is made again later. This 
shouldn't make any difference, should it? I think Torsten's patch is 
correct. And the modules/deflate.t tests do compare undeflated and 
deflated output. So they should catch lost data.

Re: svn commit: r1146418 - in /httpd/httpd/trunk: CHANGES modules/filters/mod_deflate.c

Posted by Nick Kew <ni...@webthing.com>.
On 13 Jul 2011, at 21:38, sf@apache.org wrote:

> Author: sf
> Date: Wed Jul 13 20:38:33 2011
> New Revision: 1146418
> 
> URL: http://svn.apache.org/viewvc?rev=1146418&view=rev
> Log:
> Don't try to compress requests with a zero sized body.
> 
> PR: 51350

PR 51590 tells us this introduces a new spinning bug.

Investigating that reveals what looks like a deeper problem:

> +            rc = apr_bucket_read(e, &data, &len, APR_BLOCK_READ);

Depending on the bucket type, this read might be discarding vital data!
Or am I missing something?

-- 
Nick Kew

Available for work, contract or permanent
http://www.webthing.com/~nick/cv.html