Bug 43183 - Env proxy-sendchunked is coded as proxy-sendchunks (typo)
Summary: Env proxy-sendchunked is coded as proxy-sendchunks (typo)
Status: RESOLVED FIXED
Alias: None
Product: Apache httpd-2
Classification: Unclassified
Component: mod_proxy (show other bugs)
Version: 2.5-HEAD
Hardware: All All
: P2 trivial (vote)
Target Milestone: ---
Assignee: Apache HTTPD Bugs Mailing List
URL:
Keywords: PatchAvailable
Depends on:
Blocks:
 
Reported: 2007-08-21 16:04 UTC by Brian Rectanus
Modified: 2007-09-03 14:07 UTC (History)
0 users



Attachments
patch typo (967 bytes, patch)
2007-08-21 16:05 UTC, Brian Rectanus
Details | Diff
Check for both -s and -ed variants in 2.2 (1.08 KB, patch)
2007-08-24 05:50 UTC, Vincent Bray
Details | Diff
Check for -s and -ed variants in 2.0 (1.07 KB, patch)
2007-08-24 05:52 UTC, Vincent Bray
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brian Rectanus 2007-08-21 16:04:03 UTC
Both docs in trunk/docs/manual/mod/mod_proxy.xml and comments in
trunk/modules/proxy/mod_proxy_http.c state to use:

SetEnv proxy-sendchunked 1

to force chunked T-E to the backend, however, the code tests for
"proxy-sendchunks" ("s" not "ed").  Code needs changed to match docs.

Also needs backports.
Comment 1 Brian Rectanus 2007-08-21 16:05:09 UTC
Created attachment 20686 [details]
patch typo
Comment 2 Vincent Bray 2007-08-22 00:14:24 UTC
The attached patch changes the code rather than the docs. Could dev@ confirm that the code rather than 
the docs are wrong?

If not I'll fix up the docs.
Comment 3 Ruediger Pluem 2007-08-22 12:15:04 UTC
Please fix the docs. Thus we avoid regressions with configurations in the wild
that found out this situation and adjusted their config to the code.
Comment 4 Brian Rectanus 2007-08-22 13:28:04 UTC
What about all those that have read the docs, have their configs set as such? 
Those people continue to have broken setups if you fix the docs.  Changing the
code will at least allow people using the proxy-sendchunked env var to start
working in the next release.  I think the chances of someone noticing this in
the code and adjusting their configs is much less than someone following the
docs and never verifying that it was working.  Look how long this bug has been
there as proof of this.
Comment 5 Brian Rectanus 2007-08-22 13:28:58 UTC
If you do decide to fix the docs, please fix the code comments as well.
Comment 6 Vincent Bray 2007-08-22 13:39:16 UTC
Grepping the (trunk) sources gives:

./docs/manual/env.html.en:329:   <h3><a name="proxy" id="proxy">force-proxy-request-1.0, proxy-nokeepalive, proxy-
sendchunked, proxy-sendcl</a></h3>
./docs/manual/env.html.ja.euc-jp:321:   <h3><a name="proxy" id="proxy">force-proxy-request-1.0, proxy-nokeepalive, 
proxy-sendchunked, proxy-sendcl</a></h3>
./docs/manual/env.xml:369:   <section id="proxy"><title>force-proxy-request-1.0, proxy-nokeepalive, proxy-sendchunked, 
proxy-sendcl</title>
./docs/manual/env.xml.ja:356:   <section id="proxy"><title>force-proxy-request-1.0, proxy-nokeepalive, proxy-
sendchunked, proxy-sendcl</title>
./docs/manual/mod/mod_proxy.html.en:289:    <code>proxy-sendchunked</code> minimizes resource usage by using
./docs/manual/mod/mod_proxy.xml:260:    <code>proxy-sendchunked</code> minimizes resource usage by using
./modules/proxy/mod_proxy_http.c:910:     *   not setenv proxy-sendchunked or has set setenv proxy-sendcl
./modules/proxy/mod_proxy_http.c:913:     *   setenv proxy-sendcl, and not setenv proxy-sendchunked
./modules/proxy/mod_proxy_http.c:915:     * If both proxy-sendcl and proxy-sendchunked are set, the
./modules/proxy/mod_proxy_http.c:921:     * To reduce server resource use,   setenv proxy-sendchunked

So it's clear that there's some confusion over the variable name. In fact, there's more references to 'proxy-sendchunked' in the 
code comments than there are in the docs.

I was about to commit a sed-style fix to the trunk docs, but I'm holding off in case send-chunked (which seems to me a better 
phrase) wins out over send-chunks (which at least has comical value to the English :)
Comment 7 Jeff Trawick 2007-08-23 16:18:41 UTC
My suggestion:
Double-check that all docs say "chunked" instead of "chunks".
For code in 2.0.x and 2.2.x "stable" branches, check for "chunked" like the docs
say now, as well as "chunks" which somebody who read the code might use.
For code in trunk, check only "chunked".
Comment 8 Vincent Bray 2007-08-24 05:50:47 UTC
Created attachment 20699 [details]
Check for both -s and -ed variants in 2.2

Check for both sendchunks and sendchunked in 2.2 following from Jeff Trawick's
suggestion. Brian's original patch for trunk is also valid. No changes required
for the 2.2 docs, or any comments in the C code.
Comment 9 Vincent Bray 2007-08-24 05:52:59 UTC
Created attachment 20700 [details]
Check for -s and -ed variants in 2.0

Again following from Jeff Trawick's suggestion, this time for 2.0. The 2.0 docs
don't mention either variant in env.xml, so I guess there's no changes to make
there.

Please review the logic of both patches carefully, as I'm not a C coder and not
particularly confident that it's right.
Comment 10 Brian Rectanus 2007-08-29 20:00:30 UTC
Sorry, been busy ;)

The patches in comments #8 and #9 look fine.  I have not tested, though (no
time) as I applied my patch to our 2.2.x tree.

Thanks Vincent!

-B
Comment 11 Ruediger Pluem 2007-09-03 14:07:46 UTC
Backported to 2.2.x as r572421 (http://svn.apache.org/viewvc?rev=572421&view=rev).