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 --
smime.p7s
Description: S/MIME Cryptographic Signature
