From: Yann Ylavic <ylavic@apache.org>
Date: Tue, 30 Jan 2018 00:58:54 +0000 (+0000)
Subject: core: fix ap_request_core_filter()'s brigade lifetime.
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=106d0761c06ac39bdc16d139d356eebc83a8ff62;p=apache

core: fix ap_request_core_filter()'s brigade lifetime.

The filter should pass everything up to and including EOR, then bail out.
For EOR it can't use a brigade created on r->pool, so retain one created
on c->pool in c->notes (this avoids leaking a brigades for each request
on the same connection).



git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1822596 13f79535-47bb-0310-9956-ffa450edef68
---

diff --git a/server/request.c b/server/request.c
index 55c32b276b..a3605a57b7 100644
--- a/server/request.c
+++ b/server/request.c
@@ -2080,7 +2080,13 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_request_core_filter(ap_filter_t *f,
     }
 
     if (!tmp_bb) {
-        tmp_bb = f->ctx = apr_brigade_create(f->r->pool, f->c->bucket_alloc);
+        const char *tmp_bb_key = "ap_request_core_filter_bb";
+        tmp_bb = (void *)apr_table_get(f->c->notes, tmp_bb_key);
+        if (!tmp_bb) {
+            tmp_bb = apr_brigade_create(f->c->pool, f->c->bucket_alloc);
+            apr_table_setn(f->c->notes, tmp_bb_key, (void *)tmp_bb);
+        }
+        f->ctx = tmp_bb;
     }
 
     /* Reinstate any buffered content */
@@ -2089,22 +2095,31 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_request_core_filter(ap_filter_t *f,
     while (!APR_BRIGADE_EMPTY(bb)) {
         apr_bucket *bucket = APR_BRIGADE_FIRST(bb);
 
-        /* if the core has set aside data, back off and try later */
-        if (!flush_upto) {
-            if (ap_filter_should_yield(f)) {
-                break;
-            }
-        }
-        else if (flush_upto == bucket) {
-            flush_upto = NULL;
+        if (AP_BUCKET_IS_EOR(bucket)) {
+            /* pass out everything and never come back again,
+             * r is destroyed with this bucket!
+             */
+            APR_BRIGADE_CONCAT(tmp_bb, bb);
+            ap_remove_output_filter(f);
+            f->r = NULL;
         }
+        else {
+            /* if the core has set aside data, back off and try later */
+            if (!flush_upto) {
+                if (ap_filter_should_yield(f)) {
+                    break;
+                }
+            }
+            else if (flush_upto == bucket) {
+                flush_upto = NULL;
+            }
 
-        /* have we found a morphing bucket? if so, force it to morph into something
-         * safe to pass down to the connection filters without needing to be set
-         * aside.
-         */
-        if (!APR_BUCKET_IS_METADATA(bucket)) {
-            if (bucket->length == (apr_size_t) - 1) {
+            /* have we found a morphing bucket? if so, force it to morph into
+             * something safe to pass down to the connection filters without
+             * needing to be set aside.
+             */
+            if (!APR_BUCKET_IS_METADATA(bucket)
+                    && bucket->length == (apr_size_t) - 1) {
                 const char *data;
                 apr_size_t size;
                 if (APR_SUCCESS
@@ -2113,21 +2128,22 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_request_core_filter(ap_filter_t *f,
                     return status;
                 }
             }
-        }
 
-        /* pass each bucket down the chain */
-        APR_BUCKET_REMOVE(bucket);
-        APR_BRIGADE_INSERT_TAIL(tmp_bb, bucket);
+            /* pass each bucket down the chain */
+            APR_BUCKET_REMOVE(bucket);
+            APR_BRIGADE_INSERT_TAIL(tmp_bb, bucket);
+        }
 
         status = ap_pass_brigade(f->next, tmp_bb);
-        if (!APR_STATUS_IS_EOF(status) && (status != APR_SUCCESS)) {
+        if (!f->r || (status != APR_SUCCESS && !APR_STATUS_IS_EOF(status))) {
+            apr_brigade_cleanup(tmp_bb);
             return status;
         }
 
+        apr_brigade_cleanup(tmp_bb);
     }
 
-    ap_filter_setaside_brigade(f, bb);
-    return status;
+    return ap_filter_setaside_brigade(f, bb);
 }
 
 extern APR_OPTIONAL_FN_TYPE(authz_some_auth_required) *ap__authz_ap_some_auth_required;
diff --git a/server/util_filter.c b/server/util_filter.c
index ccfef6c254..716251e910 100644
--- a/server/util_filter.c
+++ b/server/util_filter.c
@@ -744,11 +744,10 @@ AP_DECLARE(apr_status_t) ap_filter_setaside_brigade(ap_filter_t *f,
     }
 
     if (!APR_BRIGADE_EMPTY(bb)) {
-        apr_pool_t *pool = NULL;
         /*
          * Set aside the brigade bb within f->bb.
          */
-        ap_filter_prepare_brigade(f, &pool);
+        ap_filter_prepare_brigade(f, NULL);
 
         /* decide what pool we setaside to, request pool or deferred pool? */
         if (f->r) {
@@ -762,7 +761,6 @@ AP_DECLARE(apr_status_t) ap_filter_setaside_brigade(ap_filter_t *f,
                     }
                 }
             }
-            pool = f->r->pool;
             APR_BRIGADE_CONCAT(f->bb, bb);
         }
         else {
@@ -770,8 +768,7 @@ AP_DECLARE(apr_status_t) ap_filter_setaside_brigade(ap_filter_t *f,
                 apr_pool_create(&f->deferred_pool, f->c->pool);
                 apr_pool_tag(f->deferred_pool, "deferred_pool");
             }
-            pool = f->deferred_pool;
-            return ap_save_brigade(f, &f->bb, &bb, pool);
+            return ap_save_brigade(f, &f->bb, &bb, f->deferred_pool);
         }
 
     }