Commit fcfce9d9 authored by Justin Erenkrantz's avatar Justin Erenkrantz
Browse files

Fix the case where if there is no ErrorDocument specified for an error, 

the error would be sent to the client *twice* because both the filter
and the core would trigger error responses.

The problem is that the filters have already handled some errors (say 413)
and due to the ap_get_client_block API, the error was morphed into 400.
Therefore, ap_discard_request_body must use brigades directly rather than
the ap_get_client_block API so that any potential errors are not dropped.

The special value AP_FILTER_ERROR indicates that the lower level has
already dealt with this problem (ap_die() will realize this).  Otherwise,
we'll error with HTTP_BAD_REQUEST and ap_die() will take it from there.

This also prevents needless memory copies when we are just going to
discard it anyway.

Thanks to Cliff Woolley who found this wacky problem.


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

modules/http/http_protocol.c

+38 −15
Changes for modules/http/http_protocol.c: 38 added lines, 15 removed lines.
Original line number Diff line number Diff line
@@ -1851,7 +1851,8 @@ AP_DECLARE(long) ap_get_client_block(request_rec *r, char *buffer, 
 */

AP_DECLARE(int) ap_discard_request_body(request_rec *r)

{

    int rv;

    apr_bucket_brigade *bb;

    int rv, seen_eos;



    /* Sometimes we'll get in a state where the input handling has

     * detected an error where we want to drop the connection, so if
@@ -1864,33 +1865,55 @@ AP_DECLARE(int) ap_discard_request_body(request_rec *r) 
        return OK;

    }



    if (r->read_length == 0) {  /* if not read already */

        if ((rv = ap_setup_client_block(r, REQUEST_CHUNKED_DECHUNK))) {

    bb = apr_brigade_create(r->pool, r->connection->bucket_alloc);

    seen_eos = 0;

    do {

        apr_bucket *bucket;



        rv = ap_get_brigade(r->input_filters, bb, AP_MODE_READBYTES,

                            APR_BLOCK_READ, HUGE_STRING_LEN);



        if (rv != APR_SUCCESS) {

            /* FIXME: If we ever have a mapping from filters (apr_status_t)

             * to HTTP error codes, this would be a good place for them.

             * 

             * If we received the special case AP_FILTER_ERROR, it means

             * that the filters have already handled this error.

             * Otherwise, we should assume we have a bad request.

             */

            if (rv == AP_FILTER_ERROR) {

                return rv;

            }

            else {

                return HTTP_BAD_REQUEST;

            }

        }



    /* In order to avoid sending 100 Continue when we already know the

     * final response status, and yet not kill the connection if there is

     * no request body to be read, we need to duplicate the test from

     * ap_should_client_block() here negated rather than call it directly.

     */

    if ((r->read_length == 0) && (r->read_chunked || (r->remaining > 0))) {

        char dumpbuf[HUGE_STRING_LEN];

        APR_BRIGADE_FOREACH(bucket, bb) {

            const char *data;

            apr_size_t len;



        if (r->expecting_100) {

            r->connection->keepalive = -1;

            return OK;

            if (APR_BUCKET_IS_EOS(bucket)) {

                seen_eos = 1;

                break;

            }



        while ((rv = ap_get_client_block(r, dumpbuf, HUGE_STRING_LEN)) > 0) {

            /* These are metadata buckets. */

            if (bucket->length == 0) {

                continue;

            }



        if (rv < 0) {

            /* We MUST read because in case we have an unknown-length

             * bucket or one that morphs, we want to exhaust it.

             */

            rv = apr_bucket_read(bucket, &data, &len, APR_BLOCK_READ);

            if (rv != APR_SUCCESS) {

                return HTTP_BAD_REQUEST;

            }

        }

        apr_brigade_cleanup(bb);

    } while (!seen_eos);



    return OK;

}