Mailing-List: contact new-httpd-help@apache.org; run by ezmlm
Precedence: bulk
Reply-To: new-httpd@apache.org
Date: Sat, 5 May 2001 08:53:13 -0700 (PDT)
From: <rbb@covalent.net>
To: <new-httpd@apache.org>
Cc: <httpd-2.0-cvs@apache.org>
Subject: Re: cvs commit: httpd-2.0/server/mpm/perchild perchild.c
In-Reply-To: <20010505111801.48448.qmail@apache.org>
Message-ID: <Pine.LNX.4.30.0105050842210.31445-100000@koj.rkbloom.net>
MIME-Version: 1.0
Content-Type: TEXT/PLAIN; charset=US-ASCII


>   Fix a bug in the input handling. ap_http_filter() was modifying *readbytes
>   which corresponded to r->remaining (in ap_get_client_block). However,
>   ap_get_client_block was *also* adjusting r->remaining. Net result was that
>   PUT (and probably POST) was broken. (at least on large inputs)
>
>   To fix it, I simply removed the indirection on "readbytes" for input
>   filters. There is no reason for them to return data (the brigade length is
>   the return length). This also simplifies a number of calls where people
>   needed to do &zero just to pass zero.
>
>   I also added a number of comments about operations and where things could be
>   improved, or are (semi) broken.

This has broken input filtering.  I know this looks like a good change,
but it won't work.  This also explains why we originally used the
c->remain field, instead of an argument to ap_get_brigade.

The problem is that r->remaining and readbytes represent how much data was
sent over the network.  But, a filter can modify the data that was sent,
so that there is more than the HTTP layer thought.  Look at this
scenario:

r->remaining = 256
ap_get_brigade(..., 256)
read returns 126 bytes from the network
return 126, but a filter doubles that number, by converting to a
double-byte charset
now r->remaining = 0, but there is still 128 bytes on the network.

This is why the http_filter has to set r->remaining.  It is also why many
of the comments that you have added are incorrect.  We have to be able to
return more bytes than requested, because a filter may add more data than
the HTTP layer knows about.  Since we must know when we have hit the end
of the data from the network, we use an EOS bucket to signify that
condition.

This was all covered on list when input filtering was first designed and
implemented.

>   -        /* Walk through the body, accounting for bytes, and removing an eos bucket if
>   -         * ap_http_filter() delivered the entire chunk.
>   +
>   +        /* Walk through the body, accounting for bytes, and removing an eos
>   +         * bucket if ap_http_filter() delivered the entire chunk.
>   +         *
>   +         * ### this shouldn't be necessary. 1) ap_http_filter shouldn't be
>   +         * ### adding EOS buckets. 2) it shouldn't return more bytes than
>   +         * ### we requested, therefore the total len can be found with a
>   +         * ### simple call to apr_brigade_length(). no further munging
>   +         * ### would be needed.
>             */
>            b = APR_BRIGADE_FIRST(bb);
>            while (b != APR_BRIGADE_SENTINEL(bb) && !APR_BUCKET_IS_EOS(b)) {
>   @@ -503,7 +518,7 @@
>        apr_bucket_brigade *b;
>    } http_ctx_t;
>
>   -apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b, ap_input_mode_t mode, apr_size_t *readbytes)
>   +apr_status_t ap_http_filter(ap_filter_t *f, apr_bucket_brigade *b, ap_input_mode_t mode, apr_size_t readbytes)
>    {
>        apr_bucket *e;
>        char *buff;
>   @@ -560,8 +575,17 @@
>                return rv;
>            }
>        }
>   +
>   +    /* readbytes == 0 is "read a single line". otherwise, read a block. */
>   +    if (readbytes) {
>   +
>   +        /* ### the code below, which moves bytes from one brigade to the
>   +           ### other is probably bogus. presuming the next filter down was
>   +           ### working properly, it should not have returned more than
>   +           ### READBYTES bytes, and we wouldn't have to do any work. further,
>   +           ### we could probably just use brigade_partition() in here.
>   +        */


_______________________________________________________________________________
Ryan Bloom                        	rbb@apache.org
406 29th St.
San Francisco, CA 94131
-------------------------------------------------------------------------------