You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by "William A. Rowe, Jr." <wr...@rowe-clan.net> on 2005/09/09 00:55:39 UTC
mod_proxy http request body code review request
As many of the proxy reviewers could not follow
http://svn.apache.org/viewcvs.cgi/httpd/httpd/branches/proxy-reqbody-2.0.x/modules/proxy/proxy_http.c?rev=230744&r1=230703&r2=230744
I've provided the following fork of that codebase, to try to repair the
damage from the vetoed 171205 commit, in a piece by piece analysis of
what's changed and why.
The new code is only marginally different (in expected ways, based on
our API changes) from the trunk code, which I'm certain that our beta
proponents have actively tested. The resulting difference from trunk is
svn diff \
http://svn.apache.org/repos/asf/httpd/httpd/branches/proxy-reqbody-2.0.x/modules/proxy/proxy_http.c
\
http://svn.apache.org/repos/asf/httpd/httpd/trunk/modules/proxy/mod_proxy_http.c
Please test, and vote on either the entire patch or nak specific commits
below with technical justification, so that we might release 2.0.55 and
avoid backing out 171205 once again. AIUI, JimJ and myself are +1 so
far, which are all the votes collected (I'd addressed several -1's to
address the technical concerns within the fixes below). To those who
take the time to review, Thank You!
Log of /httpd/httpd/branches/proxy-reqbody-2.0.x/modules/proxy/proxy_http.c
No default branch
Bookmark a link to HEAD: (view) (download) (as text)
Revision 230744 - (view) (download) (as text) - [select for diffs]
Modified Mon Aug 8 03:33:20 2005 UTC (4 weeks, 3 days ago) by wrowe
File length: 65895 byte(s)
Diff to previous 230741 (colored)
http://svn.apache.org/viewcvs.cgi/httpd/httpd/branches/proxy-reqbody-2.0.x/modules/proxy/proxy_http.c?rev=230744&r1=230741&r2=230744
The final, inconsequential patch which makes the diff for request
body processing between trunk/ and 2.1.x/ very easy to read.
This patch consists of nothing except whitespace changes, and the
change from b to bb for the local bucket brigade variable name,
to disambiguate it from a bucket.
Revision 230741 - (view) (download) (as text) - [select for diffs]
Modified Mon Aug 8 03:20:27 2005 UTC (4 weeks, 3 days ago) by wrowe
File length: 65742 byte(s)
Diff to previous 230738 (colored)
http://svn.apache.org/viewcvs.cgi/httpd/httpd/branches/proxy-reqbody-2.0.x/modules/proxy/proxy_http.c?rev=230741&r1=230738&r2=230741
Fix some very minor nits in ap_proxy_http_request() which make it
quite simple to diff to the current trunk/ code, ensuring the most
accurate review possible.
Consists only of whitespace and line spacing changes, and a change
from 'bb' to 'header_brigade' for this local argument variable's name,
so there are no functional changes here.
Revision 230738 - (view) (download) (as text) - [select for diffs]
Modified Mon Aug 8 03:10:10 2005 UTC (4 weeks, 3 days ago) by wrowe
File length: 65595 byte(s)
Diff to previous 230737 (colored)
http://svn.apache.org/viewcvs.cgi/httpd/httpd/branches/proxy-reqbody-2.0.x/modules/proxy/proxy_http.c?rev=230738&r1=230737&r2=230738
Sync to trunk/, add an extra measure of paranoia to the cl + te case.
Revision 230737 - (view) (download) (as text) - [select for diffs]
Modified Mon Aug 8 03:03:56 2005 UTC (4 weeks, 3 days ago) by wrowe
File length: 65510 byte(s)
Diff to previous 230736 (colored)
http://svn.apache.org/viewcvs.cgi/httpd/httpd/branches/proxy-reqbody-2.0.x/modules/proxy/proxy_http.c?rev=230737&r1=230736&r2=230737
Commit a comment from trunk/, this brings spool_reqbody_cl in sync
with trunk/.
Revision 230736 - (view) (download) (as text) - [select for diffs]
Modified Mon Aug 8 03:01:39 2005 UTC (4 weeks, 3 days ago) by wrowe
File length: 65446 byte(s)
Diff to previous 230729 (colored)
http://svn.apache.org/viewcvs.cgi/httpd/httpd/branches/proxy-reqbody-2.0.x/modules/proxy/proxy_http.c?rev=230736&r1=230729&r2=230736
Backport r230735, we need not look at the first bucket for EOS,
because the outer while loop protected us from that case.
Backport the header brigade changes as it's impossible to have
a body request waiting for a final send. Look at seen_eos to
flush us in the request body loop, and handle the only exception,
(header_brigade), outside of that loop.
This brings stream_reqbody_cl in sync with the trunk.
Revision 230729 - (view) (download) (as text) - [select for diffs]
Modified Mon Aug 8 01:44:46 2005 UTC (4 weeks, 3 days ago) by wrowe
File length: 65751 byte(s)
Diff to previous 230728 (colored)
http://svn.apache.org/viewcvs.cgi/httpd/httpd/branches/proxy-reqbody-2.0.x/modules/proxy/proxy_http.c?rev=230729&r1=230728&r2=230729
Backport the fix to handling Connection: close. The existing code
was impossible to follow; the new code sets up p_conn->close correctly,
and uses that evaluated value to inject the appropriate choice
immediately
before passing the request to the backend server.
Revision 230728 - (view) (download) (as text) - [select for diffs]
Modified Mon Aug 8 01:39:14 2005 UTC (4 weeks, 3 days ago) by wrowe
File length: 65459 byte(s)
Diff to previous 230727 (colored)
http://svn.apache.org/viewcvs.cgi/httpd/httpd/branches/proxy-reqbody-2.0.x/modules/proxy/proxy_http.c?rev=230728&r1=230727&r2=230728
Backport the corrected C-L streamed, v.s. spooled, v.s. T-E: chunked
selection logic from httpd trunk/. This now correctly chooses the
most portable mechanism (e.g. C-L) when we can see the entire body,
even for chunked bodies from the client, falls back on spool cl when
it's necessary, and uses chunked when we have faith in it.
Revision 230727 - (view) (download) (as text) - [select for diffs]
Modified Mon Aug 8 01:31:02 2005 UTC (4 weeks, 3 days ago) by wrowe
File length: 64590 byte(s)
Diff to previous 230726 (colored)
http://svn.apache.org/viewcvs.cgi/httpd/httpd/branches/proxy-reqbody-2.0.x/modules/proxy/proxy_http.c?rev=230727&r1=230726&r2=230727
Backport the rejection of non-'chunked' values in the transfer
encoding; we simply don't know what, exactly to do with them.
Backport the reporting of 'both C-L and T-E' when we encounter
this edge case, setting the connection up to close down once
we finish (perhaps we were victims of a request splitting attack).
Revision 230726 - (view) (download) (as text) - [select for diffs]
Modified Mon Aug 8 01:28:14 2005 UTC (4 weeks, 3 days ago) by wrowe
File length: 63526 byte(s)
Diff to previous 230725 (colored)
http://svn.apache.org/viewcvs.cgi/httpd/httpd/branches/proxy-reqbody-2.0.x/modules/proxy/proxy_http.c?rev=230726&r1=230725&r2=230726
Backport the fix to an edge case, it's now possible for a primary
request which has a body in spite of what was determined by the
header parsing; this would usually be due to an input filter between
the client request and mod_proxy. Add another consideration, and
force the C-L determination if we saw bytes in already.
Revision 230725 - (view) (download) (as text) - [select for diffs]
Modified Mon Aug 8 01:26:34 2005 UTC (4 weeks, 3 days ago) by wrowe
File length: 63460 byte(s)
Diff to previous 230724 (colored)
http://svn.apache.org/viewcvs.cgi/httpd/httpd/branches/proxy-reqbody-2.0.x/modules/proxy/proxy_http.c?rev=230725&r1=230724&r2=230725
Backport the fix to an edge case; it's now much more efficient to
entirely skip request body determinations for subrequests, stuff
in an EOS and we are off to the races, ready to create a body-less
proxied subrequest.
Revision 230724 - (view) (download) (as text) - [select for diffs]
Modified Mon Aug 8 01:22:17 2005 UTC (4 weeks, 3 days ago) by wrowe
File length: 63218 byte(s)
Diff to previous 230722 (colored)
http://svn.apache.org/viewcvs.cgi/httpd/httpd/branches/proxy-reqbody-2.0.x/modules/proxy/proxy_http.c?rev=230724&r1=230722&r2=230724
Backport the body pre-fetch code from trunk/, modifying the three
stream_te / stream_cl / spool_cl functions to presume an input_brigade
before fighting with fetching additional body content.
We will be using the bytes_read in a later commit to make a better
decision about choosing to send a C-L or chunked body to the backend.
Revision 230722 - (view) (download) (as text) - [select for diffs]
Modified Mon Aug 8 01:10:20 2005 UTC (4 weeks, 3 days ago) by wrowe
File length: 61915 byte(s)
Diff to previous 230720 (colored)
http://svn.apache.org/viewcvs.cgi/httpd/httpd/branches/proxy-reqbody-2.0.x/modules/proxy/proxy_http.c?rev=230722&r1=230720&r2=230722
Backport yet another security fix; if stream_cl exceeds the 'stated'
CL which proxy_request_body asked us to send, then we have to quit
forwarding any more bytes (we won't even pass the header if we
hadn't yet.)
Closes an HTTP Request splitting edge case.
Revision 230720 - (view) (download) (as text) - [select for diffs]
Modified Mon Aug 8 01:03:09 2005 UTC (4 weeks, 3 days ago) by wrowe
File length: 61005 byte(s)
Diff to previous 230719 (colored)
http://svn.apache.org/viewcvs.cgi/httpd/httpd/branches/proxy-reqbody-2.0.x/modules/proxy/proxy_http.c?rev=230720&r1=230719&r2=230720
Fix a buglett in backport 230718; get our selection of brigades correct.
(tis what happens when our code is this far out-of-sync with trunk,
sorry.)
Revision 230719 - (view) (download) (as text) - [select for diffs]
Modified Mon Aug 8 00:59:40 2005 UTC (4 weeks, 3 days ago) by wrowe
File length: 61013 byte(s)
Diff to previous 230716 (colored)
http://svn.apache.org/viewcvs.cgi/httpd/httpd/branches/proxy-reqbody-2.0.x/modules/proxy/proxy_http.c?rev=230719&r1=230716&r2=230719
Backport fix r230718; a soon-to-be impossible edge case; we are always
sending a body (even an 'empty' body) using stream_chunked, so we must
always send the trailing "0" end of body marker.
Revision 230716 - (view) (download) (as text) - [select for diffs]
Modified Mon Aug 8 00:30:27 2005 UTC (4 weeks, 3 days ago) by wrowe
File length: 61036 byte(s)
Diff to previous 230714 (colored)
http://svn.apache.org/viewcvs.cgi/httpd/httpd/branches/proxy-reqbody-2.0.x/modules/proxy/proxy_http.c?rev=230716&r1=230714&r2=230716
Finish up the header brigades up-front; we simply know ahead of time if
they are necessary or not. This changes one behavior; the stream_chunked
now always sends a transfer encoded body; even if it's nothing but an
empty body.
Also, Fix a bugglet in backporting the input_brigade argument; need the
bucket_alloc already.
Revision 230714 - (view) (download) (as text) - [select for diffs]
Modified Sun Aug 7 23:45:54 2005 UTC (4 weeks, 3 days ago) by wrowe
File length: 61086 byte(s)
Diff to previous 230712 (colored)
http://svn.apache.org/viewcvs.cgi/httpd/httpd/branches/proxy-reqbody-2.0.x/modules/proxy/proxy_http.c?rev=230714&r1=230712&r2=230714
Pre-commit a simple bit from r219224, pass an initialized input_brigade
off to the spool/stream functions. Effectively a no-op so far, but in
a couple of patches, the reason becomes obvious.
Revision 230712 - (view) (download) (as text) - [select for diffs]
Modified Sun Aug 7 23:32:36 2005 UTC (4 weeks, 3 days ago) by wrowe
File length: 60767 byte(s)
Diff to previous 230711 (colored)
http://svn.apache.org/viewcvs.cgi/httpd/httpd/branches/proxy-reqbody-2.0.x/modules/proxy/proxy_http.c?rev=230712&r1=230711&r2=230712
Backport a logging change to make things clearer; includes jorton's
r224721 fix.
Revision 230711 - (view) (download) (as text) - [select for diffs]
Modified Sun Aug 7 23:23:38 2005 UTC (4 weeks, 3 days ago) by wrowe
File length: 60629 byte(s)
Diff to previous 230709 (colored)
http://svn.apache.org/viewcvs.cgi/httpd/httpd/branches/proxy-reqbody-2.0.x/modules/proxy/proxy_http.c?rev=230711&r1=230709&r2=230711
Backport part of r216156 and very minor bits of r218978;
Collapse the handling of Content-Length / Transfer-Encoding header
values into the headers loop saving a bit of cpu and making the
results clearer, handle the (r->main) subrequest case a bit more
efficiently as well, cutting out some duplicate tests, and a mild
whitespace issue in the If-Unmodified-Since test line.
Fixes a subrequest bug where subrequests *still* would attempt to
read Transfer-Encoding: chunked request bodies.
Revision 230709 - (view) (download) (as text) - [select for diffs]
Modified Sun Aug 7 22:55:37 2005 UTC (4 weeks, 3 days ago) by wrowe
File length: 60932 byte(s)
Diff to previous 230708 (colored)
http://svn.apache.org/viewcvs.cgi/httpd/httpd/branches/proxy-reqbody-2.0.x/modules/proxy/proxy_http.c?rev=230709&r1=230708&r2=230709
Backport part of r216156;
send_request_body makes it very difficult to follow all the mistakes
in this code. Fold send_request_body into ap_proxy_http_request
so that proxy_http_request makes all of the elections.
Revision 230708 - (view) (download) (as text) - [select for diffs]
Modified Sun Aug 7 22:22:41 2005 UTC (4 weeks, 4 days ago) by wrowe
File length: 61447 byte(s)
Diff to previous 230706 (colored)
http://svn.apache.org/viewcvs.cgi/httpd/httpd/branches/proxy-reqbody-2.0.x/modules/proxy/proxy_http.c?rev=230708&r1=230706&r2=230708
End apr_natstrcasecmp, backporting part of r216111 (there may remain
other apr_natstrcasecmp abuse in other files).
Still unsure if apr_strnatcasecmp of the hostname is deliberate to
handle leading 0's in ip addresses, so leaving that single use case.
Revision 230706 - (view) (download) (as text) - [select for diffs]
Modified Sun Aug 7 22:00:54 2005 UTC (4 weeks, 4 days ago) by wrowe
File length: 61585 byte(s)
Diff to previous 230704 (colored)
http://svn.apache.org/viewcvs.cgi/httpd/httpd/branches/proxy-reqbody-2.0.x/modules/proxy/proxy_http.c?rev=230704&r1=230706&r2=230704
Now backport whitespace-only changes from r209836, making the rest of
the patches more legible, and the resulting code comparable to
httpd/trunk/modules/proxy/mod_proxy_http.c.
Revision 230704 - (view) (download) (as text) - [select for diffs]
Modified Sun Aug 7 21:57:56 2005 UTC (4 weeks, 4 days ago) by wrowe
File length: 61871 byte(s)
Diff to previous 230703 (colored)
http://svn.apache.org/viewcvs.cgi/httpd/httpd/branches/proxy-reqbody-2.0.x/modules/proxy/proxy_http.c?rev=230704&r1=230703&r2=230704
Prepare to backport r209836 whitespace changes; this is the one
minor code change to declare *buf once, obliterating the need for
a number of code blocks throughout this function.
Revision 230703 - (view) (download) (as text) - [select for diffs]
Modified Sun Aug 7 21:09:38 2005 UTC (4 weeks, 4 days ago) by wrowe
File length: 61970 byte(s)
Diff to previous 219059 (colored)
Create proxy-reqbody patch evaluation branch.
Re: mod_proxy http request body code review request
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Graham Leggett wrote:
> William A. Rowe, Jr. said:
>
>>Please test, and vote on either the entire patch or nak specific commits
>>below with technical justification, so that we might release 2.0.55 and
>>avoid backing out 171205 once again. AIUI, JimJ and myself are +1 so
>>far, which are all the votes collected (I'd addressed several -1's to
>>address the technical concerns within the fixes below). To those who
>>take the time to review, Thank You!
>
> Went through each patch in detail, and it all seems fine so far. Next step
> is to do some testing of the patch, which I'll do this weekend between
> getting on and off planes. To those who worked on getting this patch
> together, thank you :)
NP - and thank you for your ongoing review; looking forward to your
results so we can clear this showstopper - was all set to start pulling
off an httpd 2.0.55 candidate today, and I'll check back in later to see
if it can still come together.
Thanks to all, esp. Jeff, who are busilly reviewing and backporting our
final bug fixes for 2.0.55.
Bill
Re: mod_proxy http request body code review request
Posted by Graham Leggett <mi...@sharp.fm>.
William A. Rowe, Jr. said:
> Please test, and vote on either the entire patch or nak specific commits
> below with technical justification, so that we might release 2.0.55 and
> avoid backing out 171205 once again. AIUI, JimJ and myself are +1 so
> far, which are all the votes collected (I'd addressed several -1's to
> address the technical concerns within the fixes below). To those who
> take the time to review, Thank You!
Went through each patch in detail, and it all seems fine so far. Next step
is to do some testing of the patch, which I'll do this weekend between
getting on and off planes. To those who worked on getting this patch
together, thank you :)
Regards,
Graham
--
Re: mod_proxy http request body code review request
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Jeff Trawick wrote:
> On 9/8/05, William A. Rowe, Jr. <wr...@rowe-clan.net> wrote:
>
>>As many of the proxy reviewers could not follow
>>http://svn.apache.org/viewcvs.cgi/httpd/httpd/branches/proxy-reqbody-2.0.x/modules/proxy/proxy_http.c?rev=230744&r1=230703&r2=230744
>>
>>I've provided the following fork of that codebase, to try to repair the
>>damage from the vetoed 171205 commit, in a piece by piece analysis of
>>what's changed and why.
>
> Here's the "why" for normally using chunked encoding to origin server
> if client sent to our proxy using chunked encoding:
>
> If origin server doesn't support chunked, it wasn't going to work
> bypassing this proxy or using some other proxy implementations anyway;
> therefore unlikely that client code would be chunking to a server that
> doesn't support it (if this configuration only works when an uplevel
> Apache proxy is in the middle, admin can go a step further and use the
> sendcl setting to convert from chunked to cl)
>
> For the record, what is wrong with that?
Nothing by me, but do, go back over Roy's original response.
Note that we can proxy a 1.0 server for a 1.1 client, and that client is
welcome to pass us CL chunked, we'll do the homework.
> This is of interest because using chunked potentially results in lower
> resource use.
This has not changed. If the body is smaller than the readahead buffer,
we will send it with C-L, however if the body is larger, it prefers to
be sent on in the original chunked format, unless the user configures to
explicitly use sendcl. There's no significant increase in resource
consumption, AFAICT.
Does that make more sense?
Bill
Re: mod_proxy http request body code review request
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Further details (as I've promised for any query or objection)...
Jeff Trawick wrote:
>
> Here's the "why" for normally using chunked encoding to origin server
> if client sent to our proxy using chunked encoding:
>
> If origin server doesn't support chunked, it wasn't going to work
> bypassing this proxy or using some other proxy implementations anyway;
> therefore unlikely that client code would be chunking to a server that
> doesn't support it (if this configuration only works when an uplevel
> Apache proxy is in the middle, admin can go a step further and use the
> sendcl setting to convert from chunked to cl)
Let's take one quick look;
if (APR_BUCKET_IS_EOS(APR_BRIGADE_LAST(input_brigade))) {
/* The whole thing fit, so our decision is trivial, use
* the filtered bytes read from the client for the request
* body Content-Length.
*
* If we expected no body, and read no body, do not set
* the Content-Length.
*/
if (old_cl_val || old_te_val || bytes_read) {
old_cl_val = apr_off_t_toa(r->pool, bytes_read);
}
rb_method = RB_STREAM_CL;
}
this case above is by Roy's insistance that, once we preread the small
in-memory buffer, we elect C-L always. Every HTTP app must be able to
handle a C-L request. Note we ignore other T-E flavors here (other than
'chunked') because we rejected them outright earlier, and the httpd
server API doesn't have a mechanism to handle non-chunked T-E bodies.
else if (old_te_val) {
if (force10
|| (apr_table_get(r->subprocess_env, "proxy-sendcl")
&& !apr_table_get(r->subprocess_env,
"proxy-sendchunks"))) {
rb_method = RB_SPOOL_CL;
}
else {
rb_method = RB_STREAM_CHUNKED;
}
}
this case above is what I mentioned, that in almost any case, if we had
a T-E in the first place, and no demand (e.g. HTTP/1.0, or proxy-sendcl)
to use C-L, we will continue to use T-E. Note that if the user toggles
both proxy-sendcl and proxy-sendchunks, sendchunks will win.
> This is of interest because using chunked potentially results in lower
> resource use.
Could you provide an example of how a 16384 byte or smaller body could
ever be handled more quickly with T-E: chunked, rather than C-L: n
(<=16384)? I can think of many cases (mostly our own) which are more
efficient when responding with a T-E, but never a case that it's faster
for the client to handle T-E rather than C-L.
Bill
Re: mod_proxy http request body code review request
Posted by Jeff Trawick <tr...@gmail.com>.
On 9/8/05, William A. Rowe, Jr. <wr...@rowe-clan.net> wrote:
> As many of the proxy reviewers could not follow
> http://svn.apache.org/viewcvs.cgi/httpd/httpd/branches/proxy-reqbody-2.0.x/modules/proxy/proxy_http.c?rev=230744&r1=230703&r2=230744
>
> I've provided the following fork of that codebase, to try to repair the
> damage from the vetoed 171205 commit, in a piece by piece analysis of
> what's changed and why.
Here's the "why" for normally using chunked encoding to origin server
if client sent to our proxy using chunked encoding:
If origin server doesn't support chunked, it wasn't going to work
bypassing this proxy or using some other proxy implementations anyway;
therefore unlikely that client code would be chunking to a server that
doesn't support it (if this configuration only works when an uplevel
Apache proxy is in the middle, admin can go a step further and use the
sendcl setting to convert from chunked to cl)
For the record, what is wrong with that?
This is of interest because using chunked potentially results in lower
resource use.
Re: mod_proxy http request body code review request
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
William A. Rowe, Jr. wrote:
> Jeff Trawick wrote:
>
>> On 9/8/05, William A. Rowe, Jr. <wr...@rowe-clan.net> wrote:
>>
>>> I've provided the following fork of that codebase, to try to repair the
>>> damage from the vetoed 171205 commit, in a piece by piece analysis of
>>> what's changed and why.
>>
>>
>> any particular reason we lost the sendunchangedcl setting?
>>
>> + if (...
>> + (r->input_filters == r->proto_input_filters
>> + ...
>> + || apr_table_get(r->subprocess_env,
>> "proxy-sendunchangedcl"))) {
>> + rb_method = RB_STREAM_CL;
>> + }
>
> But there's a second justification; because the new patch pre-reads up
> to some fixed bytes of input, and handle the vast majority of POST
> bodies in memory with a trusted CL, the necessity is somewhat lessened.
And I missed the third point; if the *protocol* input filter chain has
not been modified (no input request filters injected) then we continue
to presume that the CL is unchanged. So really the only code that
triggers spool is the situation where we have injected an input filter,
and we cannot (will not) chunk the request body, and the body is larger
than the read-ahead buffer. So this is most definately the exception,
not the rule, and the 'advantages' of some proxy-sendunchangedcl envvar
just seem not to justify the probability of misconfiguration and the
unhopefull task of diagnosing the actual symptoms.
Bill
Re: mod_proxy http request body code review request
Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Jeff Trawick wrote:
> On 9/8/05, William A. Rowe, Jr. <wr...@rowe-clan.net> wrote:
>
>>I've provided the following fork of that codebase, to try to repair the
>>damage from the vetoed 171205 commit, in a piece by piece analysis of
>>what's changed and why.
>
> any particular reason we lost the sendunchangedcl setting?
>
> + if (...
> + (r->input_filters == r->proto_input_filters
> + ...
> + || apr_table_get(r->subprocess_env, "proxy-sendunchangedcl"))) {
> + rb_method = RB_STREAM_CL;
> + }
Yes; sendunchangedcl implies that the *Administrator* (as opposed to
module developers) have this information. In fact, for all intents
and purposes, Administrators do not. The issue comes down to the fact
that the Administrator is the last person who is going to have success
shooting this down when they misconfigure it. And once misconfigured,
you will end up with hangs (short bodies) or split request bodies.
The only way to determine that the length will be assuredly unchanged,
is to modify our API to record that the filter will, or will not, modify
the body length. And, if it will, and the filter can predetermine the
modification (say for example, one byte code page to unicode two-byte
input) then it should have the opportunity to reflect that in some new
'effective input CL' req_rec field.
But there's a second justification; because the new patch pre-reads up
to some fixed bytes of input, and handle the vast majority of POST
bodies in memory with a trusted CL, the necessity is somewhat lessened.
proxy-sendunchangedcl is one aspect of the original patch that yes, I
believed should be vetoed. There has to be a third-way here, and that's
to fix the API and handling CL's - both request and response body CL.
Bill
Re: mod_proxy http request body code review request
Posted by Jeff Trawick <tr...@gmail.com>.
On 9/8/05, William A. Rowe, Jr. <wr...@rowe-clan.net> wrote:
> As many of the proxy reviewers could not follow
> http://svn.apache.org/viewcvs.cgi/httpd/httpd/branches/proxy-reqbody-2.0.x/modules/proxy/proxy_http.c?rev=230744&r1=230703&r2=230744
>
> I've provided the following fork of that codebase, to try to repair the
> damage from the vetoed 171205 commit, in a piece by piece analysis of
> what's changed and why.
any particular reason we lost the sendunchangedcl setting?
+ if (...
+ (r->input_filters == r->proto_input_filters
+ ...
+ || apr_table_get(r->subprocess_env, "proxy-sendunchangedcl"))) {
+ rb_method = RB_STREAM_CL;
+ }