httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ben Laurie <>
Subject Re: cvs commit: httpd-2.0/server/mpm/perchild perchild.c
Date Tue, 08 May 2001 13:48:17 GMT
Greg Stein wrote:
> On Sun, May 06, 2001 at 11:27:14PM -0000, 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

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.




"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

View raw message