httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Graham Leggett <minf...@sharp.fm>
Subject Re: svn commit: r592951 - in /httpd/httpd/trunk: ./ docs/manual/mod/ include/ modules/filters/ modules/http/ server/
Date Sat, 20 Sep 2008 17:14:54 GMT
Paul Querna wrote:

> +1, this revision r592951, and the derivate r647263 need to be 
> rethought.  IMO is that this doesn't belong in the reqeust_rec.
> 
> I'm only realizing this went in 10 months after the first commit, and 5 
> months after it was rewritten in r647263, a sign of my inactivity in 
> httpd land, so I'm not gonna 'veto' it, but damn I really don't like 
> having this stuff on the reqeust_rec.

Having had the time to think this through, to be honest I cannot think 
of a different place for it to go other than request_rec.

At the core of the problem is a flaw that has been repeated over and 
over inside lots of modules within the server, and which this code 
partially fixes.

The flaw is that by design, the core will allow a request to spawn a 
subrequest, but the core offers no functionality for the subrequest to 
read a request body.

So you get the same repeated code, over and over again in various 
modules, incarnations of this:

/* what we have now */
if ( ! we_are_a_subrequest) {
    read_request_body();
}
else {
    ignore_body_somehow();
}

Now whether it is relevant for a subrequest to read a request body isn't 
important, what's important is that the subrequest isn't even allowed to 
try, and the subrequest has to go out of its way to make sure it doesn't 
read from the network twice. The subrequest shouldn't have to care about 
this stuff.

With the kept_body code, two new possibilities are available that have 
not been available before:

- Where the admin wants to set aside the body to be re-read by 
subrequests, the kept_body filter makes this possible, by allowing a 
kept body to be reread by a subrequest safely. The subrequest now 
becomes a fully fledged subrequest, able to access everything the 
request can.

- Where the admin doesn't want to set aside the body, the kept_body 
filter can be inserted anyway, with an empty brigade. This "caps" the 
input filter stack, guaranteeing that the network can never be read 
twice, regardless of how many times it tries to read from the network.

These two possibilities together mean that any module is now free to try 
and read a request body if it wants to, and the kept_body filter 
guarantees that the underlying network is never read from twice.

This in turn means that all the different variations of "detect 
subrequest and try to to read the body a second time" that are scattered 
around the code become obsolete and can be removed, resulting in a much 
simpler contract for module code, and a significantly cleaner server:

/* a cleaner alternative */
read_request_body();

In order for the partial fix to become a full fix, the kept_body filter 
should be unconditionally added to all subrequests (with an empty 
brigade in the simplest case), and all the instances of the "if ( ! 
we_are_a_subrequest)" code should be collapsed and removed, and modules 
should be free to try and read the body as many times as they like, safe 
in the knowledge that the kept_body filter guarantees the network will 
only ever be read from once.

Regards,
Graham
--

Mime
View raw message