From: Yann Ylavic Date: Wed, 5 Sep 2018 17:27:43 +0000 (+0000) Subject: util_filter: protect ap_filter_t private fields from external (ab)use. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=e70b8bfbcd790e99dca06b08066f1f25c5e85d1d;p=apache util_filter: protect ap_filter_t private fields from external (ab)use. 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 --- diff --git a/include/ap_mmn.h b/include/ap_mmn.h index 1c6d198f3a..7f6398d68d 100644 --- a/include/ap_mmn.h +++ b/include/ap_mmn.h @@ -602,14 +602,18 @@ * 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 diff --git a/include/httpd.h b/include/httpd.h index 977c042897..1f6a608200 100644 --- a/include/httpd.h +++ b/include/httpd.h @@ -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 diff --git a/include/util_filter.h b/include/util_filter.h index 7f0e856580..135ff35f42 100644 --- a/include/util_filter.h +++ b/include/util_filter.h @@ -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 diff --git a/modules/http/http_request.c b/modules/http/http_request.c index d993d5793b..039dd25a26 100644 --- a/modules/http/http_request.c +++ b/modules/http/http_request.c @@ -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); diff --git a/server/core_filters.c b/server/core_filters.c index a964286613..9704fcc095 100644 --- a/server/core_filters.c +++ b/server/core_filters.c @@ -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, ©_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; } diff --git a/server/request.c b/server/request.c index abfabb7cfd..b3c9ba8e25 100644 --- a/server/request.c +++ b/server/request.c @@ -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; diff --git a/server/util.c b/server/util.c index 0ef1a74968..b99bf4812d 100644 --- a/server/util.c +++ b/server/util.c @@ -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++) diff --git a/server/util_filter.c b/server/util_filter.c index 02f40f64c1..0beb6642de 100644 --- a/server/util_filter.c +++ b/server/util_filter.c @@ -51,22 +51,34 @@ #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; } }