httpd-cvs mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From rpl...@apache.org
Subject svn commit: r488817 - in /httpd/httpd/branches/2.2.x: CHANGES modules/filters/mod_deflate.c
Date Tue, 19 Dec 2006 21:45:36 GMT
Author: rpluem
Date: Tue Dec 19 13:45:35 2006
New Revision: 488817

URL: http://svn.apache.org/viewvc?view=rev&rev=488817
Log:
Merge r422731, r422736, r422739, r423940, r424759, r424950, r425109, r426790,
r426791, r426793, r426795, r416165, r426799, r475403, r475406, r475915,
r475920, r475922, r476600, r437668 from trunk, which makes mod_deflate.c the
same as r483597 from trunk:

All comments to these changes for the sake of completeness:

* Fix wrong FLUSH bucket handling in deflate_out_filter: Actually the internal
  structures of libz never got flushed as ctx->stream.avail_in is always zero
  here. As the EOS and the FLUSH bucket case use the same code for flushing
  libz's internal buffers move this code to the new function flush_zlib_buffer.

* Fix potential memory leaks in deflate_out_filter if bailing out due to an
  error (either in the lower filter chain or during a libz operation). We need
  to call deflateEnd as it is very likely that this filter is never called
  again to ensures that libz's internal structures get cleaned properly.

* Remove ourselves from the filter chain if we failed to init libz, as we
  pass data down the filter chain uncompressed afterwards.

* In preparation of the changes of the inflate out filter:
  - rename flush_zlib_buffer to flush_libz_buffer (this name seems
    better)
  - add a parameter to tell flush_libz_buffer whether it should
    deflate or inflate as this function should be also used for the
    inflate out filter.

* This shortcut is too short. It is not up to the filters to decide
  whether filters down the chain can do something useful with this empty
  brigade.

* Initialize zRC to avoid a compiler warning.

* Rather use a pool cleanup function than calling deflateEnd before every
  return from function to ensure that libz's internals get cleaned up.

* Add parameter crc to flush_libz_buffer in order to call the libz's crc32
  function on the output buffer if needed. This is actually needed by the
  later rework of the inflate out filter.

* Use a define for the number of validation bytes (CRC and length) after the
  compressed data

* We can ignore Z_BUF_ERROR in flush_libz_buffer because:
  When we call libz_func we can assume that

   - avail_in is zero (due to the surrounding code that calls
     flush_libz_buffer)
   - avail_out is non zero due to the fact that we just emptied
     the output buffer and stored it into a brigade

  So the only reason for Z_BUF_ERROR is that the internal libz
  buffers are now empty and thus we called libz_func one time
  too often. This does not hurt. It simply says that we are done.

* some optimizations taken from the inflate out filter

* Remove bogus code that chokes on flush buckets

* Rework inflate out filter and give it a similar workflow as the deflate out
  filter. This fixes the following bugs in the inflate out filter:

  - Incorrect handling of flush buckets.
  - Excessive memory usage for large compressed content (because we now
    already sent parts down the chain and do not process the whole brigade
    first before sending something down the chain).
  - Handle the case correctly where the validation bytes at the end of
    the compressed data stream are distributed across different buckets /
    brigades.
  - Fix a memory leak due to not cleaning up the internal structures of
    libz in some error cases.

* Ensure that we do not try to inflate validation data or garbage data. None
  of this is zlib's business.

* Ensure that we do not perform our final operations twice if a second EOS
  falls down the chain by accident. Otherwise we are likely to run in a
  SEGFAULT. So remove ourselves from the chain.

* Actually append new data to the validation buffer and do not overwrite old
  data already there by setting the correct offset for the target buffer.

* Also log the presence of garbage data at the end of the stream if all
  validation data is available in the first round.

* Housekeeping: keep track of size even in the edge-case where validation
  bytes trickle in over multiple buckets.


PR: 39854
Reviewed by: rpluem, niq, jerenkrantz, wrowe

Modified:
    httpd/httpd/branches/2.2.x/CHANGES
    httpd/httpd/branches/2.2.x/modules/filters/mod_deflate.c

Modified: httpd/httpd/branches/2.2.x/CHANGES
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/CHANGES?view=diff&rev=488817&r1=488816&r2=488817
==============================================================================
--- httpd/httpd/branches/2.2.x/CHANGES [utf-8] (original)
+++ httpd/httpd/branches/2.2.x/CHANGES [utf-8] Tue Dec 19 13:45:35 2006
@@ -1,6 +1,11 @@
                                                         -*- coding: utf-8 -*-
 Changes with Apache 2.2.4
 
+  *) mod_deflate: Rework inflate output and deflate output filter to fix several
+     issues: Incorrect handling of flush buckets, potential memory leaks,
+     excessive memory usage in inflate output filter for large compressed
+     content. PR 39854. [Ruediger Pluem, Nick Kew, Justin Erenkrantz]
+
   *) mod_mem_cache: Memory leak fix: Unconditionally free the buffer.
      [Davi Arnaut <davi haxent.com.br>]
 

Modified: httpd/httpd/branches/2.2.x/modules/filters/mod_deflate.c
URL: http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/modules/filters/mod_deflate.c?view=diff&rev=488817&r1=488816&r2=488817
==============================================================================
--- httpd/httpd/branches/2.2.x/modules/filters/mod_deflate.c (original)
+++ httpd/httpd/branches/2.2.x/modules/filters/mod_deflate.c Tue Dec 19 13:45:35 2006
@@ -212,8 +212,88 @@
     unsigned char *buffer;
     unsigned long crc;
     apr_bucket_brigade *bb, *proc_bb;
+    int (*libz_end_func)(z_streamp);
+    unsigned char *validation_buffer;
+    apr_size_t validation_buffer_length;
 } deflate_ctx;
 
+/* Number of validation bytes (CRC and length) after the compressed data */
+#define VALIDATION_SIZE 8
+/* Do not update ctx->crc, see comment in flush_libz_buffer */
+#define NO_UPDATE_CRC 0
+/* Do update ctx->crc, see comment in flush_libz_buffer */
+#define UPDATE_CRC 1
+
+static int flush_libz_buffer(deflate_ctx *ctx, deflate_filter_config *c,
+                             struct apr_bucket_alloc_t *bucket_alloc,
+                             int (*libz_func)(z_streamp, int), int flush,
+                             int crc)
+{
+    int zRC = Z_OK;
+    int done = 0;
+    unsigned int deflate_len;
+    apr_bucket *b;
+
+    for (;;) {
+         deflate_len = c->bufferSize - ctx->stream.avail_out;
+
+         if (deflate_len != 0) {
+             /*
+              * Do we need to update ctx->crc? Usually this is the case for
+              * inflate action where we need to do a crc on the output, whereas
+              * in the deflate case we need to do a crc on the input
+              */
+             if (crc) {
+                 ctx->crc = crc32(ctx->crc, (const Bytef *)ctx->buffer,
+                                  deflate_len);
+             }
+             b = apr_bucket_heap_create((char *)ctx->buffer,
+                                        deflate_len, NULL,
+                                        bucket_alloc);
+             APR_BRIGADE_INSERT_TAIL(ctx->bb, b);
+             ctx->stream.next_out = ctx->buffer;
+             ctx->stream.avail_out = c->bufferSize;
+         }
+
+         if (done)
+             break;
+
+         zRC = libz_func(&ctx->stream, flush);
+
+         /*
+          * We can ignore Z_BUF_ERROR because:
+          * When we call libz_func we can assume that
+          *
+          * - avail_in is zero (due to the surrounding code that calls
+          *   flush_libz_buffer)
+          * - avail_out is non zero due to our actions some lines above
+          *
+          * So the only reason for Z_BUF_ERROR is that the internal libz
+          * buffers are now empty and thus we called libz_func one time
+          * too often. This does not hurt. It simply says that we are done.
+          */
+         if (zRC == Z_BUF_ERROR) {
+             zRC = Z_OK;
+             break;
+         }
+
+         done = (ctx->stream.avail_out != 0 || zRC == Z_STREAM_END);
+
+         if (zRC != Z_OK && zRC != Z_STREAM_END)
+             break;
+    }
+    return zRC;
+}
+
+static apr_status_t deflate_ctx_cleanup(void *data)
+{
+    deflate_ctx *ctx = (deflate_ctx *)data;
+
+    if (ctx)
+        ctx->libz_end_func(&ctx->stream);
+    return APR_SUCCESS;
+}
+
 static apr_status_t deflate_out_filter(ap_filter_t *f,
                                        apr_bucket_brigade *bb)
 {
@@ -221,14 +301,16 @@
     request_rec *r = f->r;
     deflate_ctx *ctx = f->ctx;
     int zRC;
-    deflate_filter_config *c = ap_get_module_config(r->server->module_config,
-                                                    &deflate_module);
+    deflate_filter_config *c;
 
     /* Do nothing if asked to filter nothing. */
     if (APR_BRIGADE_EMPTY(bb)) {
-        return APR_SUCCESS;
+        return ap_pass_brigade(f->next, bb);
     }
 
+    c = ap_get_module_config(r->server->module_config,
+                             &deflate_module);
+
     /* If we don't have a context, we need to ensure that it is okay to send
      * the deflated content.  If we have a context, that means we've done
      * this before and we liked it.
@@ -362,19 +444,32 @@
         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);
+        ctx->libz_end_func = deflateEnd;
 
         zRC = deflateInit2(&ctx->stream, c->compressionlevel, Z_DEFLATED,
                            c->windowSize, c->memlevel,
                            Z_DEFAULT_STRATEGY);
 
         if (zRC != Z_OK) {
-            f->ctx = NULL;
+            deflateEnd(&ctx->stream);
             ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
                           "unable to init Zlib: "
                           "deflateInit2 returned %d: URL %s",
                           zRC, r->uri);
+            /*
+             * Remove ourselves as it does not make sense to return:
+             * We are not able to init libz and pass data down the chain
+             * uncompressed.
+             */
+            ap_remove_output_filter(f);
             return ap_pass_brigade(f->next, bb);
         }
+        /*
+         * Register a cleanup function to ensure that we cleanup the internal
+         * libz resources.
+         */
+        apr_pool_cleanup_register(r->pool, ctx, deflate_ctx_cleanup,
+                                  apr_pool_cleanup_null);
 
         /* add immortal gzip header */
         e = apr_bucket_immortal_create(gzip_header, sizeof gzip_header,
@@ -400,49 +495,23 @@
         const char *data;
         apr_bucket *b;
         apr_size_t len;
-        int done = 0;
 
         e = APR_BRIGADE_FIRST(bb);
 
         if (APR_BUCKET_IS_EOS(e)) {
             char *buf;
-            unsigned int deflate_len;
 
             ctx->stream.avail_in = 0; /* should be zero already anyway */
-            for (;;) {
-                deflate_len = c->bufferSize - ctx->stream.avail_out;
+            /* flush the remaining data from the zlib buffers */
+            flush_libz_buffer(ctx, c, f->c->bucket_alloc, deflate, Z_FINISH,
+                              NO_UPDATE_CRC);
 
-                if (deflate_len != 0) {
-                    b = apr_bucket_heap_create((char *)ctx->buffer,
-                                               deflate_len, NULL,
-                                               f->c->bucket_alloc);
-                    APR_BRIGADE_INSERT_TAIL(ctx->bb, b);
-                    ctx->stream.next_out = ctx->buffer;
-                    ctx->stream.avail_out = c->bufferSize;
-                }
-
-                if (done) {
-                    break;
-                }
-
-                zRC = deflate(&ctx->stream, Z_FINISH);
-
-                if (deflate_len == 0 && zRC == Z_BUF_ERROR) {
-                    zRC = Z_OK;
-                }
-
-                done = (ctx->stream.avail_out != 0 || zRC == Z_STREAM_END);
-
-                if (zRC != Z_OK && zRC != Z_STREAM_END) {
-                    break;
-                }
-            }
-
-            buf = apr_palloc(r->pool, 8);
+            buf = apr_palloc(r->pool, VALIDATION_SIZE);
             putLong((unsigned char *)&buf[0], ctx->crc);
             putLong((unsigned char *)&buf[4], ctx->stream.total_in);
 
-            b = apr_bucket_pool_create(buf, 8, r->pool, f->c->bucket_alloc);
+            b = apr_bucket_pool_create(buf, VALIDATION_SIZE, r->pool,
+                                       f->c->bucket_alloc);
             APR_BRIGADE_INSERT_TAIL(ctx->bb, b);
             ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
                           "Zlib: Compressed %ld to %ld : URL %s",
@@ -476,6 +545,8 @@
             }
 
             deflateEnd(&ctx->stream);
+            /* No need for cleanup any longer */
+            apr_pool_cleanup_kill(r->pool, ctx, deflate_ctx_cleanup);
 
             /* Remove EOS from the old list, and insert into the new. */
             APR_BUCKET_REMOVE(e);
@@ -488,28 +559,18 @@
         }
 
         if (APR_BUCKET_IS_FLUSH(e)) {
-            apr_bucket *bkt;
             apr_status_t rv;
 
-            apr_bucket_delete(e);
-
-            if (ctx->stream.avail_in > 0) {
-                zRC = deflate(&(ctx->stream), Z_SYNC_FLUSH);
-                if (zRC != Z_OK) {
-                    return APR_EGENERAL;
-                }
+            /* flush the remaining data from the zlib buffers */
+            zRC = flush_libz_buffer(ctx, c, f->c->bucket_alloc, deflate,
+                                    Z_SYNC_FLUSH, NO_UPDATE_CRC);
+            if (zRC != Z_OK) {
+                return APR_EGENERAL;
             }
 
-            ctx->stream.next_out = ctx->buffer;
-            len = c->bufferSize - ctx->stream.avail_out;
-
-            b = apr_bucket_heap_create((char *)ctx->buffer, len,
-                                       NULL, f->c->bucket_alloc);
-            APR_BRIGADE_INSERT_TAIL(ctx->bb, b);
-            ctx->stream.avail_out = c->bufferSize;
-
-            bkt = apr_bucket_flush_create(f->c->bucket_alloc);
-            APR_BRIGADE_INSERT_TAIL(ctx->bb, bkt);
+            /* Remove flush bucket from old brigade anf insert into the new. */
+            APR_BUCKET_REMOVE(e);
+            APR_BRIGADE_INSERT_TAIL(ctx->bb, e);
             rv = ap_pass_brigade(f->next, ctx->bb);
             if (rv != APR_SUCCESS) {
                 return rv;
@@ -549,8 +610,9 @@
 
             zRC = deflate(&(ctx->stream), Z_NO_FLUSH);
 
-            if (zRC != Z_OK)
+            if (zRC != Z_OK) {
                 return APR_EGENERAL;
+            }
         }
 
         apr_bucket_delete(e);
@@ -831,8 +893,8 @@
 {
     int zlib_method;
     int zlib_flags;
-    int deflate_init = 1;
-    apr_bucket *bkt;
+    int inflate_init = 1;
+    apr_bucket *e;
     request_rec *r = f->r;
     deflate_ctx *ctx = f->ctx;
     int zRC;
@@ -841,7 +903,7 @@
 
     /* Do nothing if asked to filter nothing. */
     if (APR_BRIGADE_EMPTY(bb)) {
-        return APR_SUCCESS;
+        return ap_pass_brigade(f->next, bb);
     }
 
     c = ap_get_module_config(r->server->module_config, &deflate_module);
@@ -857,8 +919,9 @@
             return ap_pass_brigade(f->next, bb);
         }
 
-        /* Let's see what our current Content-Encoding is.
-         * If gzip is present, don't gzip again.  (We could, but let's not.)
+        /*
+         * Let's see what our current Content-Encoding is.
+         * Only inflate if gzip is present.
          */
         encoding = apr_table_get(r->headers_out, "Content-Encoding");
         if (encoding) {
@@ -890,9 +953,11 @@
 
 
         f->ctx = ctx = apr_pcalloc(f->r->pool, sizeof(*ctx));
-        ctx->proc_bb = apr_brigade_create(r->pool, f->c->bucket_alloc);
+        ctx->bb = apr_brigade_create(r->pool, f->c->bucket_alloc);
         ctx->buffer = apr_palloc(r->pool, c->bufferSize);
-
+        ctx->libz_end_func = inflateEnd;
+        ctx->validation_buffer = NULL;
+        ctx->validation_buffer_length = 0;
 
         zRC = inflateInit2(&ctx->stream, c->windowSize);
 
@@ -903,60 +968,124 @@
                           "unable to init Zlib: "
                           "inflateInit2 returned %d: URL %s",
                           zRC, r->uri);
+            /*
+             * Remove ourselves as it does not make sense to return:
+             * We are not able to init libz and pass data down the chain
+             * compressed.
+             */
             ap_remove_output_filter(f);
             return ap_pass_brigade(f->next, bb);
         }
 
-        /* initialize deflate output buffer */
+        /*
+         * Register a cleanup function to ensure that we cleanup the internal
+         * libz resources.
+         */
+        apr_pool_cleanup_register(r->pool, ctx, deflate_ctx_cleanup,
+                                  apr_pool_cleanup_null);
+
+        apr_table_unset(r->headers_out, "Content-Length");
+
+        /* initialize inflate output buffer */
         ctx->stream.next_out = ctx->buffer;
         ctx->stream.avail_out = c->bufferSize;
 
-        deflate_init = 0;
+        inflate_init = 0;
     }
 
-    for (bkt = APR_BRIGADE_FIRST(bb);
-         bkt != APR_BRIGADE_SENTINEL(bb);
-         bkt = APR_BUCKET_NEXT(bkt))
+    while (!APR_BRIGADE_EMPTY(bb))
     {
         const char *data;
+        apr_bucket *b;
         apr_size_t len;
 
-        /* If we actually see the EOS, that means we screwed up! */
-        /* no it doesn't - not in a HEAD or 204/304 */
-        if (APR_BUCKET_IS_EOS(bkt)) {
+        e = APR_BRIGADE_FIRST(bb);
+
+        if (APR_BUCKET_IS_EOS(e)) {
+            /*
+             * We are really done now. Ensure that we never return here, even
+             * if a second EOS bucket falls down the chain. Thus remove
+             * ourselves.
+             */
+            ap_remove_output_filter(f);
+            /* should be zero already anyway */
+            ctx->stream.avail_in = 0;
+            /*
+             * Flush the remaining data from the zlib buffers. It is correct
+             * to use Z_SYNC_FLUSH in this case and not Z_FINISH as in the
+             * deflate case. In the inflate case Z_FINISH requires to have a
+             * large enough output buffer to put ALL data in otherwise it
+             * fails, whereas in the deflate case you can empty a filled output
+             * buffer and call it again until no more output can be created.
+             */
+            flush_libz_buffer(ctx, c, f->c->bucket_alloc, inflate, Z_SYNC_FLUSH,
+                              UPDATE_CRC);
+            ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
+                          "Zlib: Inflated %ld to %ld : URL %s",
+                          ctx->stream.total_in, ctx->stream.total_out, r->uri);
+
+            if (ctx->validation_buffer_length == VALIDATION_SIZE) {
+                unsigned long compCRC, compLen;
+                compCRC = getLong(ctx->validation_buffer);
+                if (ctx->crc != compCRC) {
+                    ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+                                  "Zlib: Checksum of inflated stream invalid");
+                    return APR_EGENERAL;
+                }
+                ctx->validation_buffer += VALIDATION_SIZE / 2;
+                compLen = getLong(ctx->validation_buffer);
+                if (ctx->stream.total_out != compLen) {
+                    ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+                                  "Zlib: Length of inflated stream invalid");
+                    return APR_EGENERAL;
+                }
+            }
+            else {
+                ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+                              "Zlib: Validation bytes not present");
+                return APR_EGENERAL;
+            }
+
             inflateEnd(&ctx->stream);
-            return ap_pass_brigade(f->next, bb);
+            /* No need for cleanup any longer */
+            apr_pool_cleanup_kill(r->pool, ctx, deflate_ctx_cleanup);
+
+            /* Remove EOS from the old list, and insert into the new. */
+            APR_BUCKET_REMOVE(e);
+            APR_BRIGADE_INSERT_TAIL(ctx->bb, e);
+
+            /*
+             * Okay, we've seen the EOS.
+             * Time to pass it along down the chain.
+             */
+            return ap_pass_brigade(f->next, ctx->bb);
         }
 
-        if (APR_BUCKET_IS_FLUSH(bkt)) {
-            apr_bucket *tmp_heap;
-            zRC = inflate(&(ctx->stream), Z_SYNC_FLUSH);
+        if (APR_BUCKET_IS_FLUSH(e)) {
+            apr_status_t rv;
+
+            /* flush the remaining data from the zlib buffers */
+            zRC = flush_libz_buffer(ctx, c, f->c->bucket_alloc, inflate,
+                                    Z_SYNC_FLUSH, UPDATE_CRC);
             if (zRC != Z_OK) {
-                ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
-                              "Inflate error %d on flush", zRC);
-                inflateEnd(&ctx->stream);
                 return APR_EGENERAL;
             }
 
-            ctx->stream.next_out = ctx->buffer;
-            len = c->bufferSize - ctx->stream.avail_out;
-
-            ctx->crc = crc32(ctx->crc, (const Bytef *)ctx->buffer, len);
-            tmp_heap = apr_bucket_heap_create((char *)ctx->buffer, len,
-                                             NULL, f->c->bucket_alloc);
-            APR_BRIGADE_INSERT_TAIL(ctx->proc_bb, tmp_heap);
-            ctx->stream.avail_out = c->bufferSize;
-
-            /* Move everything to the returning brigade. */
-            APR_BUCKET_REMOVE(bkt);
-            break;
+            /* Remove flush bucket from old brigade anf insert into the new. */
+            APR_BUCKET_REMOVE(e);
+            APR_BRIGADE_INSERT_TAIL(ctx->bb, e);
+            rv = ap_pass_brigade(f->next, ctx->bb);
+            if (rv != APR_SUCCESS) {
+                return rv;
+            }
+            continue;
         }
 
         /* read */
-        apr_bucket_read(bkt, &data, &len, APR_BLOCK_READ);
+        apr_bucket_read(e, &data, &len, APR_BLOCK_READ);
 
         /* first bucket contains zlib header */
-        if (!deflate_init++) {
+        if (!inflate_init++) {
             if (len < 10) {
                 ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
                               "Insufficient data for inflate");
@@ -1010,84 +1139,92 @@
         ctx->stream.next_in = (unsigned char *)data;
         ctx->stream.avail_in = len;
 
+        if (ctx->validation_buffer) {
+            if (ctx->validation_buffer_length < VALIDATION_SIZE) {
+                apr_size_t copy_size;
+
+                copy_size = VALIDATION_SIZE - ctx->validation_buffer_length;
+                if (copy_size > ctx->stream.avail_in)
+                    copy_size = ctx->stream.avail_in;
+                memcpy(ctx->validation_buffer + ctx->validation_buffer_length,
+                       ctx->stream.next_in, copy_size);
+                /* Saved copy_size bytes */
+                ctx->stream.avail_in -= copy_size;
+                ctx->validation_buffer_length += copy_size;
+            }
+            if (ctx->stream.avail_in) {
+                ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
+                              "Zlib: %d bytes of garbage at the end of "
+                              "compressed stream.", ctx->stream.avail_in);
+                /*
+                 * There is nothing worth consuming for zlib left, because it is
+                 * either garbage data or the data has been copied to the
+                 * validation buffer (processing validation data is no business
+                 * for zlib). So set ctx->stream.avail_in to zero to indicate
+                 * this to the following while loop.
+                 */
+                ctx->stream.avail_in = 0;
+            }
+        }
+
         zRC = Z_OK;
 
         while (ctx->stream.avail_in != 0) {
             if (ctx->stream.avail_out == 0) {
-                apr_bucket *tmp_heap;
+
                 ctx->stream.next_out = ctx->buffer;
                 len = c->bufferSize - ctx->stream.avail_out;
 
                 ctx->crc = crc32(ctx->crc, (const Bytef *)ctx->buffer, len);
-                tmp_heap = apr_bucket_heap_create((char *)ctx->buffer, len,
-                                                  NULL, f->c->bucket_alloc);
-                APR_BRIGADE_INSERT_TAIL(ctx->proc_bb, tmp_heap);
+                b = apr_bucket_heap_create((char *)ctx->buffer, len,
+                                           NULL, f->c->bucket_alloc);
+                APR_BRIGADE_INSERT_TAIL(ctx->bb, b);
                 ctx->stream.avail_out = c->bufferSize;
+                /* Send what we have right now to the next filter. */
+                rv = ap_pass_brigade(f->next, ctx->bb);
+                if (rv != APR_SUCCESS) {
+                    return rv;
+                }
             }
 
             zRC = inflate(&ctx->stream, Z_NO_FLUSH);
 
             if (zRC == Z_STREAM_END) {
+                /*
+                 * We have inflated all data. Now try to capture the
+                 * validation bytes. We may not have them all available
+                 * right now, but capture what is there.
+                 */
+                ctx->validation_buffer = apr_pcalloc(f->r->pool,
+                                                     VALIDATION_SIZE);
+                if (ctx->stream.avail_in > VALIDATION_SIZE) {
+                    ctx->validation_buffer_length = VALIDATION_SIZE;
+                    ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
+                                  "Zlib: %d bytes of garbage at the end of "
+                                  "compressed stream.",
+                                  ctx->stream.avail_in - VALIDATION_SIZE);
+                } else if (ctx->stream.avail_in > 0) {
+                           ctx->validation_buffer_length = ctx->stream.avail_in;
+                }
+                if (ctx->validation_buffer_length)
+                    memcpy(ctx->validation_buffer, ctx->stream.next_in,
+                           ctx->validation_buffer_length);
                 break;
             }
 
             if (zRC != Z_OK) {
-                    inflateEnd(&ctx->stream);
-                    return APR_EGENERAL;
-            }
-        }
-        if (zRC == Z_STREAM_END) {
-            apr_bucket *tmp_heap, *eos;
-
-            ap_log_rerror(APLOG_MARK, APLOG_DEBUG, 0, r,
-                          "Zlib: Inflated %ld to %ld : URL %s",
-                          ctx->stream.total_in, ctx->stream.total_out,
-                          r->uri);
-
-            len = c->bufferSize - ctx->stream.avail_out;
-
-            ctx->crc = crc32(ctx->crc, (const Bytef *)ctx->buffer, len);
-            tmp_heap = apr_bucket_heap_create((char *)ctx->buffer, len,
-                                              NULL, f->c->bucket_alloc);
-            APR_BRIGADE_INSERT_TAIL(ctx->proc_bb, tmp_heap);
-            ctx->stream.avail_out = c->bufferSize;
-
-            /* Is the remaining 8 bytes already in the avail stream? */
-            if (ctx->stream.avail_in >= 8) {
-                unsigned long compCRC, compLen;
-                compCRC = getLong(ctx->stream.next_in);
-                if (ctx->crc != compCRC) {
-                    inflateEnd(&ctx->stream);
-                    return APR_EGENERAL;
-                }
-                ctx->stream.next_in += 4;
-                compLen = getLong(ctx->stream.next_in);
-                if (ctx->stream.total_out != compLen) {
-                    inflateEnd(&ctx->stream);
-                    return APR_EGENERAL;
-                }
-            }
-            else {
-                /* FIXME: We need to grab the 8 verification bytes
-                 * from the wire! */
-                inflateEnd(&ctx->stream);
                 return APR_EGENERAL;
             }
-
-            inflateEnd(&ctx->stream);
-
-            eos = apr_bucket_eos_create(f->c->bucket_alloc);
-            APR_BRIGADE_INSERT_TAIL(ctx->proc_bb, eos);
-            break;
         }
 
+        apr_bucket_delete(e);
     }
 
-    rv = ap_pass_brigade(f->next, ctx->proc_bb);
-    apr_brigade_cleanup(ctx->proc_bb);
-    return rv ;
+    apr_brigade_cleanup(bb);
+    return APR_SUCCESS;
 }
 
+#define PROTO_FLAGS AP_FILTER_PROTO_CHANGE|AP_FILTER_PROTO_CHANGE_LENGTH
 static void register_hooks(apr_pool_t *p)
 {
     ap_register_output_filter(deflateFilterName, deflate_out_filter, NULL,



Mime
View raw message