Greg Stein wrote:
>
> On Sun, May 06, 2001 at 11:27:14PM -0000, [EMAIL PROTECTED] wrote:
> > rbb 01/05/06 16:27:14
> >
> > Modified: include util_filter.h
> > modules/experimental mod_case_filter_in.c mod_charset_lite.c
> > mod_ext_filter.c
> > modules/http http_protocol.c http_request.c mod_core.h
> > modules/mappers mod_alias.c
> > modules/tls mod_tls.c
> > server core.c protocol.c util_filter.c
> > server/mpm/perchild perchild.c
> > Log:
> > Back out the recent change to ap_get_brigade, to make it use indirection
> > again. The problem is that the amount of data read from the network,
> > is not necessarily the amount of data returned from the filters. It is
> > possible for input filters to add bytes to the data read from the network.
> >
> > To fix the original bug, I just removed the line from ap_get_client_block
> > that decremented r->remaining, we allow the http_filter to do that for
> > us.
>
> This was premature, Ryan. I don't think you should have done this. My change
> exposed further problems. It didn't create them.
>
> The simple problem is that r->remaining is handled at the wrong level. It
> should be handled closer to the network, before a filter monkeys with the
> number of bytes available.
>
> Content-Length describes a network length. Therefore, it should be near the
> network. A filter that increases the number of bytes will appear "above"
> that, so the app will see more bytes. But C-L still operates down near the
> network.
>
> Second, there is no reason for the "apr_size_t *nbytes" indirection. The
> number of bytes read is in the brigade. It doesn't need to be returned. Even
> worse, some of the follow-on discussion seems to imply that you think
> "certain, magic filters" will be the only ones to adjust that value. I don't
> buy the magic.
>
> Lastly, the filters should not return more than is requested. If they do,
> they could end up reading past a request, which is Bad Mojo.
>
> Let's say my handler calls ap_get_client_block() with a 2k buffer.
>
> ap_get_client_block() calls ap_get_brigade(2k) to get 2k of data. That goes
> down through the filter chain. The bottom-most *request* input filter (not
> connection!) is processing the Content-Length. It says 200, so it ignores
> the 2k that was passed in and asks the next filter for 200 bytes. That gets
> returned and passed back up the filter stack. In the input chain, above the
> C-L processor is a gzip filter which uncompresses the brigade to 400 bytes.
> That is then returned to my app -- 400 bytes.
>
> C-L is handled properly, decompression is handled, and my app gets the data
> appropriately.
>
> So...
>
> 1) the indirection on the prototype should go away (which I did)
> 2) the C-L handling needs to move from ap_get_client_block() down to a
> low-level request input filter.
>
> The problem that you observed was because (2) wasn't done; not because of my
> patch. The correct solution would have been to fix (2), *not* to back out
> the work that I did.
This is exactly what I was trying to get at... it seems we end up with a
hybrid of the previous two attempts, though - each layer gets to
indicate to the layer below what it wants, but some layers may _also_
use protocol specific data to further limit what is returned - am I
right?
BTW, wouldn't a consequence of this be that a decompression filter (for
example) may have to buffer data?
In case it isn't obvious, I'm currently +1 on Greg's analysis. I really
don't like the protocol-specific magic going on, particularly since it
could fail when filters are combined in novel ways.
Cheers,
Ben.
--
http://www.apache-ssl.org/ben.html
"There is no limit to what a man can do or how far he can go if he
doesn't mind who gets the credit." - Robert Woodruff
