httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Greg Stein <gst...@lyra.org>
Subject Re: cvs commit: httpd-2.0/server/mpm/perchild perchild.c
Date Tue, 08 May 2001 12:11:50 GMT
On Sun, May 06, 2001 at 11:27:14PM -0000, rbb@apache.org 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.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Mime
View raw message