Backport r821477 from trunk:

    Make sure to not destroy bucket brigades that have been created by earlier
    filters. Otherwise the pool cleanups would be removed causing potential
    memory leaks later on.

Submitted by: Stefan Fritsch
Reviewed by: sf, rpluem, pgollucci

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.2.x@916627 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index fef32ad78c600a607f8cd49780c79e5e814aac59..5d1cd42f976aa3bcfb236a2b4c9d7af1d3fde1ea 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -9,6 +9,10 @@ Changes with Apache 2.2.15
      access control is still vulnerable, unless using OpenSSL >= 0.9.8l.
      [Joe Orton, Ruediger Pluem, Hartmut Keil <Hartmut.Keil adnovum.ch>]
 
+  *) core: Fix potential memory leaks by making sure to not destroy
+     bucket brigades that have been created by earlier filters.
+     [Stefan Fritsch]
+
   *) mod_authnz_ldap: Add AuthLDAPBindAuthoritative to allow Authentication to
      try other providers in the case of an LDAP bind failure.
      PR 46608 [Justin Erenkrantz, Joe Schaefer, Tony Stevenson]

diff --git a/STATUS b/STATUS
index 3903d0ab39671d908b401be0fb5d9c2341530813..31e0baec18f4da73eb80179eb6c9f731aa20ad76 100644 (file)
--- a/STATUS
+++ b/STATUS
@@ -123,13 +123,6 @@ PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
    2.2.x Patch: http://people.apache.org/~minfrin/httpd-cache-thundering.patch
    +1: minfrin, jim, pgollucci
 
-  * core: Make sure to not destroy bucket brigades that have been created
-    by earlier filters. Otherwise the pool cleanups would be removed causing
-    potential memory leaks later on.
-    Trunk patch: http://svn.apache.org/viewvc?view=revision&revision=821477
-    2.2.x patch: http://people.apache.org/~sf/avoid_apr_brigade_destroy-2.2.x.diff
-    +1: sf, rpluem, pgollucci
-
 PATCHES PROPOSED TO BACKPORT FROM TRUNK:
   [ New proposals should be added at the end of the list ]
 

diff --git a/modules/http/byterange_filter.c b/modules/http/byterange_filter.c
index a25d1e5957d8a96c2fc4c2c4e0ecc8d4d1ef40b0..e38d366e077e0464bbb9823b84ef08bc9f32b9de 100644 (file)
--- a/modules/http/byterange_filter.c
+++ b/modules/http/byterange_filter.c
@@ -308,7 +308,7 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_byterange_filter(ap_filter_t *f,
     APR_BRIGADE_INSERT_TAIL(bsend, e);
 
     /* we're done with the original content - all of our data is in bsend. */
-    apr_brigade_destroy(bb);
+    apr_brigade_cleanup(bb);
 
     /* send our multipart output */
     return ap_pass_brigade(f->next, bsend);

diff --git a/modules/http/http_filters.c b/modules/http/http_filters.c
index 81295b242a3dea733cfc59023bbfbff97fc4635f..7fbc3692ac080a7ecd5e970cce323cfcd97200ee 100644 (file)
--- a/modules/http/http_filters.c
+++ b/modules/http/http_filters.c
@@ -1120,7 +1120,7 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_http_header_filter(ap_filter_t *f,
             ctx = f->ctx = apr_pcalloc(r->pool, sizeof(header_filter_ctx));
         }
         else if (ctx->headers_sent) {
-            apr_brigade_destroy(b);
+            apr_brigade_cleanup(b);
             return OK;
         }
     }
@@ -1291,7 +1291,7 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_http_header_filter(ap_filter_t *f,
     ap_pass_brigade(f->next, b2);
 
     if (r->header_only) {
-        apr_brigade_destroy(b);
+        apr_brigade_cleanup(b);
         ctx->headers_sent = 1;
         return OK;
     }

diff --git a/server/core_filters.c b/server/core_filters.c
index c2ebe981c8f77c7fc602a155814883b2cec2845e..7feb157e2cede1382a9031f7dcfcd2548bab9153 100644 (file)
--- a/server/core_filters.c
+++ b/server/core_filters.c
@@ -655,6 +655,8 @@ apr_status_t ap_core_output_filter(ap_filter_t *f, apr_bucket_brigade *b)
                             /* Create a temporary brigade as a means
                              * of concatenating a bunch of buckets together
                              */
+                            temp_brig = apr_brigade_create(f->c->pool,
+                                                       f->c->bucket_alloc);
                             if (last_merged_bucket) {
                                 /* If we've concatenated together small
                                  * buckets already in a previous pass,
@@ -667,15 +669,8 @@ apr_status_t ap_core_output_filter(ap_filter_t *f, apr_bucket_brigade *b)
                                  * these buckets, so that the content
                                  * in them doesn't have to be copied again.
                                  */
-                                apr_bucket_brigade *bb;
-                                bb = apr_brigade_split(b,
-                                         APR_BUCKET_NEXT(last_merged_bucket));
-                                temp_brig = b;
-                                b = bb;
-                            }
-                            else {
-                                temp_brig = apr_brigade_create(f->c->pool,
-                                                           f->c->bucket_alloc);
+                                APR_BRIGADE_PREPEND(b, temp_brig);
+                                brigade_move(temp_brig, b, APR_BUCKET_NEXT(last_merged_bucket));
                             }
 
                             temp = APR_BRIGADE_FIRST(b);
@@ -879,7 +874,7 @@ apr_status_t ap_core_output_filter(ap_filter_t *f, apr_bucket_brigade *b)
                 logio_add_bytes_out(c, bytes_sent);
         }
 
-        apr_brigade_destroy(b);
+        apr_brigade_cleanup(b);
 
         /* drive cleanups for resources which were set aside
          * this may occur before or after termination of the request which
@@ -910,7 +905,7 @@ apr_status_t ap_core_output_filter(ap_filter_t *f, apr_bucket_brigade *b)
                           "core_output_filter: writing data to the network");
 
             if (more)
-                apr_brigade_destroy(more);
+                apr_brigade_cleanup(more);
 
             /* No need to check for SUCCESS, we did that above. */
             if (!APR_STATUS_IS_EAGAIN(rv)) {