Commit a4b9355a authored by Yann Ylavic's avatar Yann Ylavic
Browse files

Follow up to r1840149: core input filter pending data. 

Since r1840149 ap_core_input_filter() can't use use f->[priv->]bb directly, so
ap_filter_input_pending() stopped accounting for its pending data.

But ap_core_input_filter() can't (and doesn't need to) setaside its socket
bucket, so ap_filter_setaside_brigade() is not an option. This commit adds
ap_filter_adopt_brigade() which simply moves the given buckets (brigade) into
f->priv->bb, and since this is not something to be done blindly (the buckets
need to have c->pool/bucket_alloc lifetime, which is the case in the core
filter) the function is not AP_DECLAREd/exported thus can be used in core only.

With ap_filter_adopt_brigade() and ap_filter_reinstate_brigade(), the core
input is now ap_filter_input_pending() friendly.

Also, ap_filter_recycle() is no more part of the API (AP_DECLARE removed too),
there really is no point to call it outside core code. MAJOR bumped once again
because of this.


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

include/ap_mmn.h

+2 −1
Changes for include/ap_mmn.h: 2 added lines, 1 removed line.
Original line number Diff line number Diff line
@@ -606,12 +606,13 @@ 
 *                         ap_acquire_brigade()/ap_release_brigade(), and

 *                         in ap_filter_t replace pending/bb/deferred_pool

 *                         fields by struct ap_filter_private *priv

 * 20180906.1 (2.5.1-dev)  Don't export ap_filter_recycle() anymore

 */



#define MODULE_MAGIC_COOKIE 0x41503235UL /* "AP25" */



#ifndef MODULE_MAGIC_NUMBER_MAJOR

#define MODULE_MAGIC_NUMBER_MAJOR 20180905

#define MODULE_MAGIC_NUMBER_MAJOR 20180906

#endif

#define MODULE_MAGIC_NUMBER_MINOR 1                 /* 0...n */

include/util_filter.h

+15 −2
Changes for include/util_filter.h: 15 added lines, 2 removed lines.
Original line number Diff line number Diff line
@@ -596,6 +596,16 @@ AP_DECLARE(int) ap_filter_prepare_brigade(ap_filter_t *f); 
AP_DECLARE(apr_status_t) ap_filter_setaside_brigade(ap_filter_t *f,

                                                    apr_bucket_brigade *bb);



/*

 * Adopt a bucket brigade as is (no setaside nor copy).

 * @param f The current filter

 * @param bb The bucket brigade adopted.  This brigade is always empty

 *          on return

 * @remark httpd internal, not exported, needed by

 *               ap_core_input_filter

 */

void ap_filter_adopt_brigade(ap_filter_t *f, apr_bucket_brigade *bb);



/**

 * Reinstate a brigade setaside earlier, and calculate the amount of data we

 * should write based on the presence of flush buckets, size limits on in
@@ -656,14 +666,17 @@ AP_DECLARE_NONSTD(int) ap_filter_output_pending(conn_rec *c); 
 */

AP_DECLARE_NONSTD(int) ap_filter_input_pending(conn_rec *c);



/**

/*

 * Recycle removed request filters so that they can be reused for filters

 * added later on the same connection. This typically should happen after

 * each request handling.

 *

 * @param c The connection.

 * @remark httpd internal, not exported, needed by

 *         ap_process_request_after_handler

 *         

 */

AP_DECLARE(void) ap_filter_recycle(conn_rec *c);

void ap_filter_recycle(conn_rec *c);



/**

 * Flush function for apr_brigade_* calls.  This calls ap_pass_brigade

modules/http/http_request.c

+0 −3
Changes for modules/http/http_request.c: 0 added lines, 3 removed lines.
Original line number Diff line number Diff line
@@ -402,10 +402,7 @@ AP_DECLARE(void) ap_process_request_after_handler(request_rec *r) 
    (void)ap_check_pipeline(c, bb, DEFAULT_LIMIT_BLANK_LINES);



    ap_release_brigade(c, bb);



    if (!c->aborted) {

    ap_filter_recycle(c);

    }



    if (c->cs) {

        if (c->aborted) {

server/core_filters.c

+31 −22
Changes for server/core_filters.c: 31 added lines, 22 removed lines.
Original line number Diff line number Diff line
@@ -94,7 +94,7 @@ apr_status_t ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b, 
                                  ap_input_mode_t mode, apr_read_type_e block,

                                  apr_off_t readbytes)

{

    apr_status_t rv;

    apr_status_t rv = APR_SUCCESS;

    core_net_rec *net = f->ctx;

    core_ctx_t *ctx = net->in_ctx;

    const char *str;
@@ -124,9 +124,12 @@ apr_status_t ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b, 
        if (rv != APR_SUCCESS)

            return rv;

    }

    else if (APR_BRIGADE_EMPTY(ctx->bb)) {

    else {

        ap_filter_reinstate_brigade(f, ctx->bb, NULL);

        if (APR_BRIGADE_EMPTY(ctx->bb)) {

            return APR_EOF;

        }

    }



    /* ### This is bad. */

    BRIGADE_NORMALIZE(ctx->bb);
@@ -151,7 +154,7 @@ apr_status_t ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b, 
        if (APR_STATUS_IS_EAGAIN(rv) && block == APR_NONBLOCK_READ) {

            rv = APR_SUCCESS;

        }

        return rv;

        goto cleanup;

    }



    /* ### AP_MODE_PEEK is a horrific name for this mode because we also
@@ -171,15 +174,16 @@ 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(ctx->bb))

                return APR_EOF;

            if (APR_BRIGADE_EMPTY(ctx->bb)) {

                rv = APR_EOF;

                goto cleanup;

            }



            e = APR_BRIGADE_FIRST(ctx->bb);



            rv = apr_bucket_read(e, &str, &len, APR_NONBLOCK_READ);



            if (rv != APR_SUCCESS)

                return rv;

            if (rv != APR_SUCCESS) {

                goto cleanup;

            }



            c = str;

            while (c < str + len) {
@@ -188,7 +192,7 @@ apr_status_t ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b, 
                else if (*c == APR_ASCII_CR && *(c + 1) == APR_ASCII_LF)

                    c += 2;

                else

                    return APR_SUCCESS;

                    goto cleanup;

            }



            /* If we reach here, we were a bucket just full of CRLFs, so
@@ -196,7 +200,9 @@ apr_status_t ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b, 
            /* FIXME: Is this the right thing to do in the core? */

            apr_bucket_delete(e);

        }

        return APR_SUCCESS;



        /* UNREACHABLE */

        ap_assert(0);

    }



    /* If mode is EXHAUSTIVE, we want to just read everything until the end
@@ -222,7 +228,9 @@ apr_status_t ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b, 
         * must be EOS. */

        e = apr_bucket_eos_create(f->c->bucket_alloc);

        APR_BRIGADE_INSERT_TAIL(b, e);

        return APR_SUCCESS;



        rv = APR_SUCCESS;

        goto cleanup;

    }



    /* read up to the amount they specified. */
@@ -233,14 +241,13 @@ apr_status_t ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b, 


        e = APR_BRIGADE_FIRST(ctx->bb);

        rv = apr_bucket_read(e, &str, &len, block);



        if (rv != APR_SUCCESS) {

            if (APR_STATUS_IS_EAGAIN(rv) && block == APR_NONBLOCK_READ) {

            /* getting EAGAIN for a blocking read is an error; for a

                /* getting EAGAIN for a blocking read is an error; not for a

                 * non-blocking read, return an empty brigade. */

            return APR_SUCCESS;

                rv = APR_SUCCESS;

            }

        else if (rv != APR_SUCCESS) {

            return rv;

            goto cleanup;

        }

        else if (block == APR_BLOCK_READ && len == 0) {

            /* We wanted to read some bytes in blocking mode.  We read
@@ -257,14 +264,13 @@ apr_status_t ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b, 
                e = apr_bucket_eos_create(f->c->bucket_alloc);

                APR_BRIGADE_INSERT_TAIL(b, e);

            }

            return APR_SUCCESS;

            goto cleanup;

        }



        /* Have we read as much data as we wanted (be greedy)? */

        if (len < readbytes) {

            apr_size_t bucket_len;



            rv = APR_SUCCESS;

            /* We already registered the data in e in len */

            e = APR_BUCKET_NEXT(e);

            while ((len < readbytes) && (rv == APR_SUCCESS)
@@ -298,7 +304,7 @@ apr_status_t ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b, 


        rv = apr_brigade_partition(ctx->bb, readbytes, &e);

        if (rv != APR_SUCCESS) {

            return rv;

            goto cleanup;

        }



        /* Must do move before CONCAT */
@@ -316,7 +322,7 @@ apr_status_t ap_core_input_filter(ap_filter_t *f, apr_bucket_brigade *b, 
            {

                rv = apr_bucket_copy(e, &copy_bucket);

                if (rv != APR_SUCCESS) {

                    return rv;

                    goto cleanup;

                }

                APR_BRIGADE_INSERT_TAIL(b, copy_bucket);

            }
@@ -325,7 +331,10 @@ 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->bb */

        APR_BRIGADE_CONCAT(ctx->bb, ctx->tmpbb);

    }

    return APR_SUCCESS;



cleanup:

    ap_filter_adopt_brigade(f, ctx->bb);

    return rv;

}



static apr_status_t send_brigade_nonblocking(apr_socket_t *s,

server/util_filter.c

+20 −2
Changes for server/util_filter.c: 20 added lines, 2 removed lines.
Original line number Diff line number Diff line
@@ -321,7 +321,8 @@ static struct ap_filter_conn_ctx *get_conn_ctx(conn_rec *c) 
    return x;

}



static void make_spare_ring(struct spare_ring **ring, apr_pool_t *p)

static APR_INLINE

void make_spare_ring(struct spare_ring **ring, apr_pool_t *p)

{

    if (!*ring) {

        *ring = apr_palloc(p, sizeof(**ring));
@@ -403,7 +404,7 @@ static apr_status_t request_filter_cleanup(void *arg) 
    return APR_SUCCESS;

}



AP_DECLARE(void) ap_filter_recycle(conn_rec *c)

void ap_filter_recycle(conn_rec *c)

{

    struct ap_filter_conn_ctx *x = c->filter_conn_ctx;
@@ -983,6 +984,22 @@ AP_DECLARE(apr_status_t) ap_filter_setaside_brigade(ap_filter_t *f, 
    return APR_SUCCESS;

}



void ap_filter_adopt_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,

                  "adopt %s brigade to %s brigade in '%s' output filter",

                  APR_BRIGADE_EMPTY(bb) ? "empty" : "full",

                  (!fp->bb || APR_BRIGADE_EMPTY(fp->bb)) ? "empty" : "full",

                  f->frec->name);



    if (!APR_BRIGADE_EMPTY(bb)) {

        ap_filter_prepare_brigade(f);

        APR_BRIGADE_CONCAT(fp->bb, bb);

    }

}



AP_DECLARE(apr_status_t) ap_filter_reinstate_brigade(ap_filter_t *f,

                                                     apr_bucket_brigade *bb,

                                                     apr_bucket **flush_upto)
@@ -1308,6 +1325,7 @@ AP_DECLARE_NONSTD(apr_status_t) ap_fprintf(ap_filter_t *f, 
    va_end(args);

    return rv;

}



AP_DECLARE(void) ap_filter_protocol(ap_filter_t *f, unsigned int flags)

{

    f->frec->proto_flags = flags ;