]> granicus.if.org Git - apache/commitdiff
util_filter: protect ap_filter_t private fields from external (ab)use.
authorYann Ylavic <ylavic@apache.org>
Wed, 5 Sep 2018 17:27:43 +0000 (17:27 +0000)
committerYann Ylavic <ylavic@apache.org>
Wed, 5 Sep 2018 17:27:43 +0000 (17:27 +0000)
Introduce opaque struct ap_filter_private to move ap_filter_t "pending", "bb"
and "deferred_pool" fields to the "priv" side of things.

This allows to trust values set internally (only!) in util_filter code, and
make useful assertions between the different functions calls, along with the
usual nice extensibility property.

Likewise, the private struct ap_filter_conn_ctx in conn_rec (from r1839997)
allows now to implement the new ap_acquire_brigade() and ap_release_brigade()
functions useful to get a brigade with c->pool's lifetime. They obsolete
ap_reuse_brigade_from_pool() which is replaced where previously used.

Some comments added in ap_request_core_filter() regarding the lifetime of the
data it plays with, up to EOR...

MAJOR bumped (once again).

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

include/ap_mmn.h
include/httpd.h
include/util_filter.h
modules/http/http_request.c
server/core_filters.c
server/request.c
server/util.c
server/util_filter.c

index 1c6d198f3a11c6661316f25694bc5faddcd2843b..7f6398d68d555f099fa423003fce146f17a1c3c8 100644 (file)
  *                         filter_conn_ctx, remove argument pool from
  *                         ap_filter_prepare_brigade()
  * 20180903.2 (2.5.1-dev)  Add ap_filter_recycle()
+ * 20180905.1 (2.5.1-dev)  Axe ap_reuse_brigade_from_pool(), replaced by
+ *                         ap_acquire_brigade()/ap_release_brigade(), and
+ *                         and replace pending/bb/deferred_pool fields in
+ *                         ap_filter_t by struct ap_filter_private priv field
  */
 
 #define MODULE_MAGIC_COOKIE 0x41503235UL /* "AP25" */
 
 #ifndef MODULE_MAGIC_NUMBER_MAJOR
-#define MODULE_MAGIC_NUMBER_MAJOR 20180903
+#define MODULE_MAGIC_NUMBER_MAJOR 20180905
 #endif
-#define MODULE_MAGIC_NUMBER_MINOR 2                 /* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 1                 /* 0...n */
 
 /**
  * Determine if the server's current MODULE_MAGIC_NUMBER is at least a
index 977c042897bb16dcf14309f3da0854303bdd2c0f..1f6a60820049472ad91955f8eb2fd693551a8d89 100644 (file)
@@ -1111,7 +1111,7 @@ typedef enum {
     AP_CONN_KEEPALIVE
 } ap_conn_keepalive_e;
 
-/* For struct ap_filter_conn_ctx */
+/* For struct ap_filter and ap_filter_conn_ctx */
 #include "util_filter.h"
 
 /**
@@ -1224,7 +1224,7 @@ struct conn_rec {
     /** Array of requests being handled under this connection. */
     apr_array_header_t *requests;
 
-    /** Filters' context for this connection */
+    /** Filters private/opaque context for this connection */
     struct ap_filter_conn_ctx *filter_conn_ctx;
 
     /** The minimum level of filter type to allow setaside buckets */
@@ -2193,19 +2193,6 @@ AP_DECLARE(int) ap_request_has_body(request_rec *r);
  */
 AP_DECLARE(int) ap_request_tainted(request_rec *r, int flags);
 
-/**
- * Reuse a brigade from a pool, or create it on the given pool/alloc and
- * associate it with the given key for further reuse.
- *
- * @param key the key/id of the brigade
- * @param pool the pool to cache and create the brigade from
- * @param alloc the bucket allocator to be used by the brigade
- * @return the reused and cleaned up brigade, or a new one
- */
-AP_DECLARE(apr_bucket_brigade *) ap_reuse_brigade_from_pool(const char *key,
-                                                            apr_pool_t *pool,
-                                                    apr_bucket_alloc_t *alloc);
-
 /**
  * Cleanup a string (mainly to be filesystem safe)
  * We only allow '_' and alphanumeric chars. Non-printable
index 7f0e85658025e5bf87daeeac333af1406724bc75..135ff35f42ab58a3d0093011dad7a5059a1be87f 100644 (file)
@@ -263,6 +263,11 @@ struct ap_filter_rec_t {
     ap_filter_direction_e direction;
 };
 
+/**
+ * @brief The private/opaque data in ap_filter_t.
+ */
+struct ap_filter_private;
+
 /**
  * @brief The representation of a filter chain.
  *
@@ -293,21 +298,29 @@ struct ap_filter_t {
      */
     conn_rec *c;
 
-    /** Buffered data associated with the current filter. */
-    apr_bucket_brigade *bb;
-
-    /** Dedicated pool to use for deferred writes. */
-    apr_pool_t *deferred_pool;
-
-    /** Entry in ring of pending filters (with setaside buckets). */
-    APR_RING_ENTRY(ap_filter_t) pending;
+    /** Filter private/opaque data */
+    struct ap_filter_private *priv;
 };
 
 /**
- * @brief The filters' context in conn_rec (opaque).
+ * @brief The filters private/opaque context in conn_rec.
  */
 struct ap_filter_conn_ctx;
 
+/**
+ * Acquire a brigade created on the connection pool/alloc.
+ * @param c The connection
+ * @return The brigade (cleaned up)
+ */
+AP_DECLARE(apr_bucket_brigade *) ap_acquire_brigade(conn_rec *c);
+
+/**
+ * Release and cleanup a brigade (created on the connection pool/alloc!).
+ * @param c The connection
+ * @param bb The brigade
+ */
+AP_DECLARE(void) ap_release_brigade(conn_rec *c, apr_bucket_brigade *bb);
+
 /**
  * Get the current bucket brigade from the next filter on the filter
  * stack.  The filter returns an apr_status_t value.  If the bottom-most
index d993d5793b99a1d51dc9b64b371efbee8440368b..039dd25a2652d28cf67444f43e4c8f15d72e1cb8 100644 (file)
@@ -352,11 +352,12 @@ AP_DECLARE(void) ap_process_request_after_handler(request_rec *r)
     conn_rec *c = r->connection;
     ap_filter_t *f;
 
+    bb = ap_acquire_brigade(c);
+
     /* Send an EOR bucket through the output filter chain.  When
      * this bucket is destroyed, the request will be logged and
      * its pool will be freed
      */
-    bb = ap_reuse_brigade_from_pool("ap_prah_bb", c->pool, c->bucket_alloc);
     b = ap_bucket_eor_create(c->bucket_alloc, r);
     APR_BRIGADE_INSERT_HEAD(bb, b);
 
@@ -399,7 +400,8 @@ AP_DECLARE(void) ap_process_request_after_handler(request_rec *r)
      * until the next/real request comes in or the keepalive timeout expires.
      */
     (void)ap_check_pipeline(c, bb, DEFAULT_LIMIT_BLANK_LINES);
-    apr_brigade_cleanup(bb);
+
+    ap_release_brigade(c, bb);
 
     if (!c->aborted) {
         ap_filter_recycle(c);
@@ -507,7 +509,7 @@ AP_DECLARE(void) ap_process_request(request_rec *r)
     ap_process_async_request(r);
 
     if (ap_run_input_pending(c) != OK) {
-        bb = ap_reuse_brigade_from_pool("ap_pr_bb", c->pool, c->bucket_alloc);
+        bb = ap_acquire_brigade(c);
         b = apr_bucket_flush_create(c->bucket_alloc);
         APR_BRIGADE_INSERT_HEAD(bb, b);
         rv = ap_pass_brigade(c->output_filters, bb);
@@ -520,7 +522,7 @@ AP_DECLARE(void) ap_process_request(request_rec *r)
             ap_log_cerror(APLOG_MARK, APLOG_INFO, rv, c, APLOGNO(01581)
                           "flushing data to the client");
         }
-        apr_brigade_cleanup(bb);
+        ap_release_brigade(c, bb);
     }
     if (ap_extended_status) {
         ap_time_process_request(c->sbh, STOP_PREQUEST);
index a964286613e3a8b5ef28d00ff86cbecdf57f16d7..9704fcc095638f74490bef0fe0c68562979c9ffe 100644 (file)
@@ -85,6 +85,7 @@ struct core_output_filter_ctx {
 };
 
 struct core_filter_ctx {
+    apr_bucket_brigade *bb;
     apr_bucket_brigade *tmpbb;
 };
 
@@ -116,19 +117,19 @@ apr_status_t ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b,
     if (!ctx)
     {
         net->in_ctx = ctx = apr_palloc(f->c->pool, sizeof(*ctx));
-        ap_filter_prepare_brigade(f);
+        ctx->bb = apr_brigade_create(f->c->pool, f->c->bucket_alloc);
         ctx->tmpbb = apr_brigade_create(f->c->pool, f->c->bucket_alloc);
         /* seed the brigade with the client socket. */
-        rv = ap_run_insert_network_bucket(f->c, f->bb, net->client_socket);
+        rv = ap_run_insert_network_bucket(f->c, ctx->bb, net->client_socket);
         if (rv != APR_SUCCESS)
             return rv;
     }
-    else if (APR_BRIGADE_EMPTY(f->bb)) {
+    else if (APR_BRIGADE_EMPTY(ctx->bb)) {
         return APR_EOF;
     }
 
     /* ### This is bad. */
-    BRIGADE_NORMALIZE(f->bb);
+    BRIGADE_NORMALIZE(ctx->bb);
 
     /* check for empty brigade again *AFTER* BRIGADE_NORMALIZE()
      * If we have lost our socket bucket (see above), we are EOF.
@@ -136,13 +137,13 @@ apr_status_t ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b,
      * Ideally, this should be returning SUCCESS with EOS bucket, but
      * some higher-up APIs (spec. read_request_line via ap_rgetline)
      * want an error code. */
-    if (APR_BRIGADE_EMPTY(f->bb)) {
+    if (APR_BRIGADE_EMPTY(ctx->bb)) {
         return APR_EOF;
     }
 
     if (mode == AP_MODE_GETLINE) {
         /* we are reading a single LF line, e.g. the HTTP headers */
-        rv = apr_brigade_split_line(b, f->bb, block, HUGE_STRING_LEN);
+        rv = apr_brigade_split_line(b, ctx->bb, block, HUGE_STRING_LEN);
         /* We should treat EAGAIN here the same as we do for EOF (brigade is
          * empty).  We do this by returning whatever we have read.  This may
          * or may not be bogus, but is consistent (for now) with EOF logic.
@@ -170,10 +171,10 @@ apr_status_t ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b,
          * mean that there is another request, just a blank line.
          */
         while (1) {
-            if (APR_BRIGADE_EMPTY(f->bb))
+            if (APR_BRIGADE_EMPTY(ctx->bb))
                 return APR_EOF;
 
-            e = APR_BRIGADE_FIRST(f->bb);
+            e = APR_BRIGADE_FIRST(ctx->bb);
 
             rv = apr_bucket_read(e, &str, &len, APR_NONBLOCK_READ);
 
@@ -212,7 +213,7 @@ apr_status_t ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b,
         apr_bucket *e;
 
         /* Tack on any buckets that were set aside. */
-        APR_BRIGADE_CONCAT(b, f->bb);
+        APR_BRIGADE_CONCAT(b, ctx->bb);
 
         /* Since we've just added all potential buckets (which will most
          * likely simply be the socket bucket) we know this is the end,
@@ -230,7 +231,7 @@ apr_status_t ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b,
 
         AP_DEBUG_ASSERT(readbytes > 0);
 
-        e = APR_BRIGADE_FIRST(f->bb);
+        e = APR_BRIGADE_FIRST(ctx->bb);
         rv = apr_bucket_read(e, &str, &len, block);
 
         if (APR_STATUS_IS_EAGAIN(rv) && block == APR_NONBLOCK_READ) {
@@ -247,7 +248,7 @@ apr_status_t ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b,
              *
              * When we are in normal mode, return an EOS bucket to the
              * caller.
-             * When we are in speculative mode, leave ctx->b empty, so
+             * When we are in speculative mode, leave ctx->bb empty, so
              * that the next call returns an EOS bucket.
              */
             apr_bucket_delete(e);
@@ -267,7 +268,7 @@ apr_status_t ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b,
             /* We already registered the data in e in len */
             e = APR_BUCKET_NEXT(e);
             while ((len < readbytes) && (rv == APR_SUCCESS)
-                   && (e != APR_BRIGADE_SENTINEL(f->bb))) {
+                   && (e != APR_BRIGADE_SENTINEL(ctx->bb))) {
                 /* Check for the availability of buckets with known length */
                 if (e->length != -1) {
                     len += e->length;
@@ -295,22 +296,22 @@ apr_status_t ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b,
             readbytes = len;
         }
 
-        rv = apr_brigade_partition(f->bb, readbytes, &e);
+        rv = apr_brigade_partition(ctx->bb, readbytes, &e);
         if (rv != APR_SUCCESS) {
             return rv;
         }
 
         /* Must do move before CONCAT */
-        ctx->tmpbb = apr_brigade_split_ex(f->bb, e, ctx->tmpbb);
+        ctx->tmpbb = apr_brigade_split_ex(ctx->bb, e, ctx->tmpbb);
 
         if (mode == AP_MODE_READBYTES) {
-            APR_BRIGADE_CONCAT(b, f->bb);
+            APR_BRIGADE_CONCAT(b, ctx->bb);
         }
         else if (mode == AP_MODE_SPECULATIVE) {
             apr_bucket *copy_bucket;
 
-            for (e = APR_BRIGADE_FIRST(f->bb);
-                 e != APR_BRIGADE_SENTINEL(f->bb);
+            for (e = APR_BRIGADE_FIRST(ctx->bb);
+                 e != APR_BRIGADE_SENTINEL(ctx->bb);
                  e = APR_BUCKET_NEXT(e))
             {
                 rv = apr_bucket_copy(e, &copy_bucket);
@@ -321,8 +322,8 @@ apr_status_t ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b,
             }
         }
 
-        /* Take what was originally there and place it back on ctx->b */
-        APR_BRIGADE_CONCAT(f->bb, ctx->tmpbb);
+        /* Take what was originally there and place it back on ctx->bb */
+        APR_BRIGADE_CONCAT(ctx->bb, ctx->tmpbb);
     }
     return APR_SUCCESS;
 }
index abfabb7cfd0ff303be244e7a6d0b3e6b358e473f..b3c9ba8e25ae9706b3604a20e4a45e05a1d0409d 100644 (file)
@@ -2068,7 +2068,7 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_request_core_filter(ap_filter_t *f,
 {
     apr_bucket *flush_upto = NULL;
     apr_status_t status = APR_SUCCESS;
-    apr_bucket_brigade *tmp_bb = f->ctx;
+    apr_bucket_brigade *tmp_bb;
     int seen_eor = 0;
 
     /*
@@ -2080,15 +2080,17 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_request_core_filter(ap_filter_t *f,
         return ap_pass_brigade(f->next, bb);
     }
 
-    if (!tmp_bb) {
-        f->ctx = tmp_bb = ap_reuse_brigade_from_pool("ap_rcf_bb", f->c->pool,
-                                                     f->c->bucket_alloc);
-    }
-
     /* Reinstate any buffered content */
     ap_filter_reinstate_brigade(f, bb, &flush_upto);
 
-    while (!APR_BRIGADE_EMPTY(bb)) {
+    /* After EOR is passed downstream, anything pooled on the request may
+     * be destroyed (including bb!), but not tmp_bb which is acquired from
+     * c->pool (and released after the below loop).
+     */
+    tmp_bb = ap_acquire_brigade(f->c);
+
+    /* Don't touch *bb after seen_eor */
+    while (status == APR_SUCCESS && !seen_eor && !APR_BRIGADE_EMPTY(bb)) {
         apr_bucket *bucket = APR_BRIGADE_FIRST(bb);
 
         if (AP_BUCKET_IS_EOR(bucket)) {
@@ -2118,10 +2120,9 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_request_core_filter(ap_filter_t *f,
                     && bucket->length == (apr_size_t)-1) {
                 const char *data;
                 apr_size_t size;
-                if (APR_SUCCESS
-                        != (status = apr_bucket_read(bucket, &data, &size,
-                                APR_BLOCK_READ))) {
-                    return status;
+                status = apr_bucket_read(bucket, &data, &size, APR_BLOCK_READ);
+                if (status != APR_SUCCESS) {
+                    break;
                 }
             }
 
@@ -2132,13 +2133,15 @@ AP_CORE_DECLARE_NONSTD(apr_status_t) ap_request_core_filter(ap_filter_t *f,
 
         status = ap_pass_brigade(f->next, tmp_bb);
         apr_brigade_cleanup(tmp_bb);
-
-        if (status != APR_SUCCESS || seen_eor) {
-            return status;
-        }
     }
 
-    return ap_filter_setaside_brigade(f, bb);
+    ap_release_brigade(f->c, tmp_bb);
+
+    /* Don't touch *bb after seen_eor */
+    if (status == APR_SUCCESS && !seen_eor) {
+        status = ap_filter_setaside_brigade(f, bb);
+    }
+    return status;
 }
 
 extern APR_OPTIONAL_FN_TYPE(authz_some_auth_required) *ap__authz_ap_some_auth_required;
index 0ef1a749681b7a602a5bf0e2965765353d102d22..b99bf4812d6ee68b060927cb5e910c0dd134aa0c 100644 (file)
@@ -2679,22 +2679,6 @@ AP_DECLARE_NONSTD(apr_status_t) ap_pool_cleanup_set_null(void *data_)
     return APR_SUCCESS;
 }
 
-AP_DECLARE(apr_bucket_brigade *) ap_reuse_brigade_from_pool(const char *key,
-                                                            apr_pool_t *pool,
-                                                    apr_bucket_alloc_t *alloc)
-{
-    apr_bucket_brigade *bb = NULL;
-    apr_pool_userdata_get((void **)&bb, key, pool);
-    if (bb == NULL) {
-        bb = apr_brigade_create(pool, alloc);
-        apr_pool_userdata_set(bb, key, NULL, pool);
-    }
-    else {
-        apr_brigade_cleanup(bb);
-    }
-    return bb;
-}
-
 AP_DECLARE(apr_status_t) ap_str2_alnum(const char *src, char *dest) {
 
     for ( ; *src; src++, dest++)
index 02f40f64c1dc0931266148aa8115a75d4b9d3f85..0beb6642ded9b612aa674fa91b207a7116ea0c95 100644 (file)
 #undef APLOG_MODULE_INDEX
 #define APLOG_MODULE_INDEX AP_CORE_MODULE_INDEX
 
+struct ap_filter_private {
+    /* Link to a pending_ring (keep first preferably) */
+    APR_RING_ENTRY(ap_filter_private) pending;
+
+    /* Backref to owning filter */
+    ap_filter_t *f;
+
+    /* Pending buckets */
+    apr_bucket_brigade *bb;
+    /* Dedicated pool to use for deferred writes. */
+    apr_pool_t *deferred_pool;
+};
+APR_RING_HEAD(pending_ring, ap_filter_private);
+
 struct spare_data {
     APR_RING_ENTRY(spare_data) link;
     void *data;
 };
-
-APR_RING_HEAD(ap_filter_ring, ap_filter_t);
-APR_RING_HEAD(ap_filter_spare_ring, spare_data);
+APR_RING_HEAD(spare_ring, spare_data);
 
 struct ap_filter_conn_ctx {
-    struct ap_filter_ring *pending_input_filters;
-    struct ap_filter_ring *pending_output_filters;
+    struct pending_ring *pending_input_filters;
+    struct pending_ring *pending_output_filters;
 
-    struct ap_filter_spare_ring *spare_containers,
-                                *spare_brigades,
-                                *spare_filters,
-                                *dead_filters;
+    struct spare_ring *spare_containers,
+                      *spare_brigades,
+                      *spare_filters,
+                      *dead_filters;
 };
 
 typedef struct filter_trie_node filter_trie_node;
@@ -309,7 +321,7 @@ static struct ap_filter_conn_ctx *get_conn_ctx(conn_rec *c)
     return x;
 }
 
-static void make_spare_ring(struct ap_filter_spare_ring **ring, apr_pool_t *p)
+static void make_spare_ring(struct spare_ring **ring, apr_pool_t *p)
 {
     if (!*ring) {
         *ring = apr_palloc(p, sizeof(**ring));
@@ -317,7 +329,7 @@ static void make_spare_ring(struct ap_filter_spare_ring **ring, apr_pool_t *p)
     }
 }
 
-static void *get_spare(conn_rec *c, struct ap_filter_spare_ring *ring)
+static void *get_spare(conn_rec *c, struct spare_ring *ring)
 {
     void *data = NULL;
 
@@ -335,8 +347,7 @@ static void *get_spare(conn_rec *c, struct ap_filter_spare_ring *ring)
     return data;
 }
 
-static void put_spare(conn_rec *c, void *data,
-                      struct ap_filter_spare_ring **ring)
+static void put_spare(conn_rec *c, void *data, struct spare_ring **ring)
 {
     struct ap_filter_conn_ctx *x = c->filter_conn_ctx;
     struct spare_data *sdata;
@@ -355,6 +366,24 @@ static void put_spare(conn_rec *c, void *data,
     APR_RING_INSERT_TAIL(*ring, sdata, spare_data, link);
 }
 
+AP_DECLARE(apr_bucket_brigade *) ap_acquire_brigade(conn_rec *c)
+{
+    struct ap_filter_conn_ctx *x = get_conn_ctx(c);
+    apr_bucket_brigade *bb = get_spare(c, x->spare_brigades);
+
+    return bb ? bb : apr_brigade_create(c->pool, c->bucket_alloc);
+}
+
+AP_DECLARE(void) ap_release_brigade(conn_rec *c, apr_bucket_brigade *bb)
+{
+    struct ap_filter_conn_ctx *x = get_conn_ctx(c);
+
+    AP_DEBUG_ASSERT(bb->p == c->pool && bb->bucket_alloc == c->bucket_alloc);
+
+    apr_brigade_cleanup(bb);
+    put_spare(c, bb, &x->spare_brigades);
+}
+
 static apr_status_t request_filter_cleanup(void *arg)
 {
     ap_filter_t *f = arg;
@@ -395,6 +424,7 @@ static ap_filter_t *add_any_filter_handle(ap_filter_rec_t *frec, void *ctx,
     ap_filter_t *f;
     ap_filter_t **outf;
     struct ap_filter_conn_ctx *x;
+    struct ap_filter_private *fp;
 
     if (frec->ftype < AP_FTYPE_PROTOCOL) {
         if (r) {
@@ -422,10 +452,18 @@ static ap_filter_t *add_any_filter_handle(ap_filter_rec_t *frec, void *ctx,
 
     x = get_conn_ctx(c);
     f = get_spare(c, x->spare_filters);
-    if (!f) {
+    if (f) {
+        fp = f->priv;
+    }
+    else {
         f = apr_palloc(c->pool, sizeof(*f));
+        fp = apr_palloc(c->pool, sizeof(*fp));
     }
     memset(f, 0, sizeof(*f));
+    memset(fp, 0, sizeof(*fp));
+    APR_RING_ELEM_INIT(fp, pending);
+    f->priv = fp;
+    fp->f = f;
 
     f->frec = frec;
     f->ctx = ctx;
@@ -436,7 +474,6 @@ static ap_filter_t *add_any_filter_handle(ap_filter_rec_t *frec, void *ctx,
         f->r = r;
     }
     f->c = c;
-    APR_RING_ELEM_INIT(f, pending);
 
     if (INSERT_BEFORE(f, *outf)) {
         f->next = *outf;
@@ -562,22 +599,23 @@ AP_DECLARE(ap_filter_t *) ap_add_output_filter_handle(ap_filter_rec_t *f,
 
 static APR_INLINE int is_pending_filter(ap_filter_t *f)
 {
-    return APR_RING_NEXT(f, pending) != f;
+    struct ap_filter_private *fp = f->priv;
+    return APR_RING_NEXT(fp, pending) != fp;
 }
 
 static apr_status_t pending_filter_cleanup(void *arg)
 {
     ap_filter_t *f = arg;
+    struct ap_filter_private *fp = f->priv;
 
     if (is_pending_filter(f)) {
-        APR_RING_REMOVE(f, pending);
-        APR_RING_ELEM_INIT(f, pending);
+        APR_RING_REMOVE(fp, pending);
+        APR_RING_ELEM_INIT(fp, pending);
     }
 
-    if (f->bb) {
-        apr_brigade_cleanup(f->bb);
-        put_spare(f->c, f->bb, &f->c->filter_conn_ctx->spare_brigades);
-        f->bb = NULL;
+    if (fp->bb) {
+        ap_release_brigade(f->c, fp->bb);
+        fp->bb = NULL;
     }
 
     return APR_SUCCESS;
@@ -617,14 +655,13 @@ AP_DECLARE(void) ap_remove_input_filter(ap_filter_t *f)
 
 AP_DECLARE(void) ap_remove_output_filter(ap_filter_t *f)
 {
+    struct ap_filter_private *fp = f->priv;
 
-    if ((f->bb) && !APR_BRIGADE_EMPTY(f->bb)) {
-        apr_brigade_cleanup(f->bb);
-    }
-
-    if (f->deferred_pool) {
-        apr_pool_destroy(f->deferred_pool);
-        f->deferred_pool = NULL;
+    if (fp->deferred_pool) {
+        AP_DEBUG_ASSERT(fp->bb);
+        apr_brigade_cleanup(fp->bb);
+        apr_pool_destroy(fp->deferred_pool);
+        fp->deferred_pool = NULL;
     }
 
     remove_any_filter(f, f->r ? &f->r->output_filters : NULL,
@@ -833,23 +870,31 @@ AP_DECLARE(int) ap_filter_prepare_brigade(ap_filter_t *f)
 {
     conn_rec *c = f->c;
     struct ap_filter_conn_ctx *x = get_conn_ctx(c);
-    struct ap_filter_ring **ref, *pendings;
-    ap_filter_t *next, *e;
-    int found = 0;
-
-    if (!f->bb) {
-        f->bb = get_spare(c, x->spare_brigades);
-        if (!f->bb) {
-            f->bb = apr_brigade_create(c->pool, c->bucket_alloc);
-        }
+    struct ap_filter_private *fp = f->priv, *e;
+    struct pending_ring **ref, *pendings;
+    ap_filter_t *next;
+
+    if (is_pending_filter(f)) {
+        return DECLINED;
+    }
+
+    if (!fp->bb) {
+        fp->bb = ap_acquire_brigade(c);
         if (f->r) {
+            /* Take care of request filters that don't remove themselves
+             * from the chain(s), when f->r is being destroyed.
+             */
             apr_pool_cleanup_register(f->r->pool, f,
                                       pending_filter_cleanup,
                                       apr_pool_cleanup_null);
         }
-    }
-    if (is_pending_filter(f)) {
-        return DECLINED;
+        else {
+            /* In fp->bb there may be buckets on fp->deferred_pool, so take
+             * care to always pre_cleanup the former before the latter.
+             */
+            apr_pool_pre_cleanup_register(c->pool, f,
+                                          pending_filter_cleanup);
+        }
     }
 
     if (f->frec->direction == AP_FILTER_INPUT) {
@@ -867,41 +912,39 @@ AP_DECLARE(int) ap_filter_prepare_brigade(ap_filter_t *f)
      * any, otherwise insert last.
      */
     if (pendings) {
-        for (next = f->next; next && !found; next = next->next) {
+        for (next = f->next; next; next = next->next) {
             for (e = APR_RING_FIRST(pendings);
-                 e != APR_RING_SENTINEL(pendings, ap_filter_t, pending);
+                 e != APR_RING_SENTINEL(pendings, ap_filter_private, pending);
                  e = APR_RING_NEXT(e, pending)) {
-                if (e == next) {
-                    APR_RING_INSERT_BEFORE(e, f, pending);
-                    found = 1;
-                    break;
+                if (e == next->priv) {
+                    APR_RING_INSERT_BEFORE(e, fp, pending);
+                    return OK;
                 }
             }
         }
     }
     else {
         pendings = *ref = apr_palloc(c->pool, sizeof(*pendings));
-        APR_RING_INIT(pendings, ap_filter_t, pending);
+        APR_RING_INIT(pendings, ap_filter_private, pending);
     }
-    if (!found) {
-        APR_RING_INSERT_TAIL(pendings, f, ap_filter_t, pending);
-    }
+    APR_RING_INSERT_TAIL(pendings, fp, ap_filter_private, pending);
     return OK;
 }
 
 AP_DECLARE(apr_status_t) ap_filter_setaside_brigade(ap_filter_t *f,
                                                     apr_bucket_brigade *bb)
 {
+    struct ap_filter_private *fp = f->priv;
+
     ap_log_cerror(APLOG_MARK, APLOG_TRACE6, 0, f->c,
                   "setaside %s brigade to %s brigade in '%s' output filter",
                   APR_BRIGADE_EMPTY(bb) ? "empty" : "full",
-                  (!f->bb || APR_BRIGADE_EMPTY(f->bb)) ? "empty" : "full",
+                  (!fp->bb || APR_BRIGADE_EMPTY(fp->bb)) ? "empty" : "full",
                   f->frec->name);
 
     if (!APR_BRIGADE_EMPTY(bb)) {
         /*
-         * Set aside the brigade bb within f->bb.
+         * Set aside the brigade bb within fp->bb.
          */
         ap_filter_prepare_brigade(f);
 
@@ -917,24 +960,25 @@ AP_DECLARE(apr_status_t) ap_filter_setaside_brigade(ap_filter_t *f,
                     }
                 }
             }
-            APR_BRIGADE_CONCAT(f->bb, bb);
+            APR_BRIGADE_CONCAT(fp->bb, bb);
         }
         else {
-            if (!f->deferred_pool) {
-                apr_pool_create(&f->deferred_pool, f->c->pool);
-                apr_pool_tag(f->deferred_pool, "deferred_pool");
+            if (!fp->deferred_pool) {
+                apr_pool_create(&fp->deferred_pool, f->c->pool);
+                apr_pool_tag(fp->deferred_pool, "deferred_pool");
             }
-            return ap_save_brigade(f, &f->bb, &bb, f->deferred_pool);
+            return ap_save_brigade(f, &fp->bb, &bb, fp->deferred_pool);
         }
 
     }
-    else if (f->deferred_pool) {
+    else if (fp->deferred_pool) {
         /*
          * There are no more requests in the pipeline. We can just clear the
          * pool.
          */
-        apr_brigade_cleanup(f->bb);
-        apr_pool_clear(f->deferred_pool);
+        AP_DEBUG_ASSERT(fp->bb);
+        apr_brigade_cleanup(fp->bb);
+        apr_pool_clear(fp->deferred_pool);
     }
     return APR_SUCCESS;
 }
@@ -946,16 +990,17 @@ AP_DECLARE(apr_status_t) ap_filter_reinstate_brigade(ap_filter_t *f,
     apr_bucket *bucket, *next;
     apr_size_t bytes_in_brigade, non_file_bytes_in_brigade;
     int eor_buckets_in_brigade, morphing_bucket_in_brigade;
+    struct ap_filter_private *fp = f->priv;
     core_server_config *conf;
  
     ap_log_cerror(APLOG_MARK, APLOG_TRACE6, 0, f->c,
                   "reinstate %s brigade to %s brigade in '%s' output filter",
-                  (!f->bb || APR_BRIGADE_EMPTY(f->bb) ? "empty" : "full"),
+                  (!fp->bb || APR_BRIGADE_EMPTY(fp->bb) ? "empty" : "full"),
                   (APR_BRIGADE_EMPTY(bb) ? "empty" : "full"),
                   f->frec->name);
 
-    if (f->bb) {
-        APR_BRIGADE_PREPEND(bb, f->bb);
+    if (fp->bb) {
+        APR_BRIGADE_PREPEND(bb, fp->bb);
     }
     if (!flush_upto) {
         /* Just prepend all. */
@@ -1115,7 +1160,8 @@ AP_DECLARE(int) ap_filter_should_yield(ap_filter_t *f)
      * from us.
      */
     while (f) {
-        if (f->bb && !APR_BRIGADE_EMPTY(f->bb)) {
+        struct ap_filter_private *fp = f->priv;
+        if (fp->bb && !APR_BRIGADE_EMPTY(fp->bb)) {
             return 1;
         }
         f = f->next;
@@ -1126,32 +1172,32 @@ AP_DECLARE(int) ap_filter_should_yield(ap_filter_t *f)
 AP_DECLARE_NONSTD(int) ap_filter_output_pending(conn_rec *c)
 {
     struct ap_filter_conn_ctx *x = c->filter_conn_ctx;
+    struct ap_filter_private *fp, *prev;
     apr_bucket_brigade *bb;
-    ap_filter_t *f, *prev;
     int rc = DECLINED;
 
     if (!x || !x->pending_output_filters) {
         goto cleanup;
     }
 
-    bb = ap_reuse_brigade_from_pool("ap_fop_bb", c->pool,
-                                    c->bucket_alloc);
-
     /* Flush outer most filters first for ap_filter_should_yield(f->next)
      * to be relevant in the previous ones (e.g. ap_request_core_filter()
      * won't pass its buckets if its next filters yield already).
      */
-    for (f = APR_RING_LAST(x->pending_output_filters);
-         f != APR_RING_SENTINEL(x->pending_output_filters,
-                                ap_filter_t, pending);
-         f = prev) {
+    bb = ap_acquire_brigade(c);
+    for (fp = APR_RING_LAST(x->pending_output_filters);
+         fp != APR_RING_SENTINEL(x->pending_output_filters,
+                                 ap_filter_private, pending);
+         fp = prev) {
         /* If a filter removes itself from the filters stack (when run), it
          * also orphans itself from the ring, so save "prev" here to avoid
          * an infinite loop in this case.
          */
-        prev = APR_RING_PREV(f, pending);
+        prev = APR_RING_PREV(fp, pending);
 
-        if (f->bb && !APR_BRIGADE_EMPTY(f->bb)) {
+        AP_DEBUG_ASSERT(fp->bb);
+        if (!APR_BRIGADE_EMPTY(fp->bb)) {
+            ap_filter_t *f = fp->f;
             apr_status_t rv;
 
             rv = ap_pass_brigade(f, bb);
@@ -1164,12 +1210,13 @@ AP_DECLARE_NONSTD(int) ap_filter_output_pending(conn_rec *c)
                 break;
             }
 
-            if (f->bb && !APR_BRIGADE_EMPTY(f->bb)) {
+            if (fp->bb && !APR_BRIGADE_EMPTY(fp->bb)) {
                 rc = OK;
                 break;
             }
         }
     }
+    ap_release_brigade(c, bb);
 
 cleanup:
     /* No more flushing, all filters have returned, recycle/unleak dead request
@@ -1183,26 +1230,26 @@ cleanup:
 AP_DECLARE_NONSTD(int) ap_filter_input_pending(conn_rec *c)
 {
     struct ap_filter_conn_ctx *x = c->filter_conn_ctx;
-    ap_filter_t *f;
+    struct ap_filter_private *fp;
 
     if (!x || !x->pending_input_filters) {
         return DECLINED;
     }
 
-    for (f = APR_RING_LAST(x->pending_input_filters);
-         f != APR_RING_SENTINEL(x->pending_input_filters,
-                                ap_filter_t, pending);
-         f = APR_RING_PREV(f, pending)) {
-        if (f->bb) {
-            apr_bucket *e = APR_BRIGADE_FIRST(f->bb);
+    for (fp = APR_RING_LAST(x->pending_input_filters);
+         fp != APR_RING_SENTINEL(x->pending_input_filters,
+                                 ap_filter_private, pending);
+         fp = APR_RING_PREV(fp, pending)) {
+        apr_bucket *e;
 
-            /* if there is a leading non-morphing bucket
-             * in place, then we have data pending
-             */
-            if (e != APR_BRIGADE_SENTINEL(f->bb)
-                    && e->length != (apr_size_t)(-1)) {
-                return OK;
-            }
+        /* if there is a leading non-morphing bucket
+         * in place, then we have data pending
+         */
+        AP_DEBUG_ASSERT(fp->bb);
+        e = APR_BRIGADE_FIRST(fp->bb);
+        if (e != APR_BRIGADE_SENTINEL(fp->bb)
+                && e->length != (apr_size_t)(-1)) {
+            return OK;
         }
     }