You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Thomas Eibner <th...@stderr.net> on 2002/05/29 18:28:24 UTC
[1.3] Proxy fixes and FWD: Re: [apache-modules] Setting bytes_sent in Request Record while generating all headers by myself in Apache 1.3]
Anyone looked at the remaining open bugs in 1.3 and might want to include
this patch (and bug)?
Would it also be possible to have mod_proxy for 1.3 set the same
X-Forwarded-* headers as the 2.0 proxy does? Need patches for this?
----- Forwarded message from Anthony Howe <ac...@snert.com> -----
Date: Wed, 29 May 2002 18:23:00 +0200
From: Anthony Howe <ac...@snert.com>
To: apache-modules@covalent.net
Reply-To: apache-modules@covalent.net
Subject: Re: [apache-modules] Setting bytes_sent in Request Record while generating
all headers by myself in Apache 1.3
User-Agent: Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.0rc3) Gecko/20020523
MIME-Version: 1.0
Thomas Eibner wrote:
> On Tue, May 28, 2002 at 01:21:59PM +0200, Anthony Howe wrote:
>
>>Are you using Apache 1.3? Are you using mod_proxy? I submitted a long
>>time ago a patch to Apache to fix a bug in mod_proxy, which fails to
>>update the bytes_sent field. Don't know if they ever included it, but I
>>have it still if required.
>
>
> Why not check up on wheter it actually was included/fixed and then submit
> a patch if it is still broken? Otherwise it will never make it into the
> 1.3 branch (which btw is having it's last release very soon now.)
Well from what I can tell from just looking at 1.3.24, they STILL
haven't included the patch.
I'm sure I already submitted the patch sometime ago, 14 Nov 2000, old
bug database number 6841, and its STILL open.
I figure if they can't address a one line patch when it was submitted
then, why should I keep pushing for something only to be ignored.
Obviously someone doesn't think my input is worth anything or that my
patch breaks something else. I figured I was doing my part by submitting
the bug in the first place.
Oh hum.
--------->8----------
Full text of PR number 6841:
Received: (qmail 3213 invoked by uid 501); 14 Nov 2000 14:17:09 -0000
Message-Id: <20...@locus.apache.org>
Date: 14 Nov 2000 14:17:09 -0000
From: Anthony Howe <ac...@snert.com>
Reply-To: achowe@snert.com
To: submit@bugz.apache.org
Subject: mod_proxy does not maintain the request_rec->bytes_sent field.
X-Send-Pr-Version: 3.110
>Number: 6841
>Category: mod_proxy
>Synopsis: mod_proxy does not maintain the
request_rec->bytes_sent field.
>Confidential: no
>Severity: non-critical
>Priority: medium
>Responsible: apache
>State: open
>Quarter:
>Keywords:
>Date-Required:
>Class: sw-bug
>Submitter-Id: apache
>Arrival-Date: Tue Nov 14 06:20:01 PST 2000
>Closed-Date:
>Last-Modified:
>Originator: achowe@snert.com
>Release: 1.3.14
>Organization:
apache
>Environment:
Linux mail.snert.net 2.0.34C52_SK #1 Tue Nov 30 18:14:40 PST 1999 mips
unknown
>Description:
A user reported a bug against mod_throttle claiming that mod_throttle
failed to
record the number of bytes sent when the request passed through mod_proxy.
Apon debugging and examination of the mod_proxy source, I found that
ap_proxy_send_fb() tracked and returned the number of bytes received/sent,
but that NO ONE made use of the return value to update the request_rec's
bytes_sent field.
Find enclosed a one line change to src/modules/proxy/proxy_util.c that
updates
the request_rec.
By making the change in ap_proxy_send_fb(), http and ftp response from the
remote server or from the cache will all correctly update the request_rec
so that other modules can make use of this information in the logging phase.
>How-To-Repeat:
>Fix:
*** proxy_util.c.orig Tue Nov 14 14:34:42 2000
--- proxy_util.c Tue Nov 14 14:49:25 2000
***************
*** 618,623 ****
--- 618,626 ----
ap_bflush(con->client);
ap_kill_timeout(r);
+
+ r->bytes_sent += total_bytes_rcvd;
+
return total_bytes_rcvd;
}
>Release-Note:
>Audit-Trail:
>Unformatted:
[In order for any reply to be added to the PR database, you need]
[to include <ap...@Apache.Org> in the Cc line and make sure the]
[subject line starts with the report component and number, with ]
[or without any 'Re:' prefixes (such as "general/1098:" or ]
["Re: general/1098:"). If the subject doesn't match this ]
[pattern, your message will be misfiled and ignored. The ]
["apbugs" address is not added to the Cc line of messages from ]
[the database automatically because of the potential for mail ]
[loops. If you do not include this Cc, your reply may be ig- ]
[nored unless you are responding to an explicit request from a ]
[developer. Reply only with text; DO NOT SEND ATTACHMENTS! ]
--------->8----------
--
Anthony C Howe +33 6 11 89 73 78
http://www.snert.com/ ICQ: 7116561 AIM: Sir Wumpus
"Microsoft (cough, sputter, spit, !@#$%) ..."
---------------------------------------------------------------------
To unsubscribe, e-mail: apache-modules-unsubscribe@covalent.net
For additional commands, e-mail: apache-modules-help@covalent.net
----- End forwarded message -----
--
Thomas Eibner <http://thomas.eibner.dk/> DnsZone <http://dnszone.org/>
mod_pointer <http://stderr.net/mod_pointer> <http://photos.eibner.dk/>
!(C)<http://copywrong.dk/> <http://apachegallery.dk/>
Putting the HEST in .COM <http://www.hestdesign.com/>
Re: [1.3] Proxy fixes and FWD: Re: [apache-modules] Setting bytes_sent in Request Record while generating all headers by myself in Apache 1.3]
Posted by Thomas Eibner <th...@stderr.net>.
On Wed, May 29, 2002 at 08:18:53PM +0200, Thomas Eibner wrote:
> On Wed, May 29, 2002 at 02:02:53PM -0400, Greg Marr wrote:
> > At 01:30 PM 05/29/2002, Thomas Eibner wrote:
> > >Index: proxy_http.c
> > >===================================================================
> > >RCS file: /home/cvspublic/apache-1.3/src/modules/proxy/proxy_http.c,v
> > >retrieving revision 1.98
> > >diff -u -r1.98 proxy_http.c
> > >--- proxy_http.c 21 Apr 2002 21:16:39 -0000 1.98
> > >+++ proxy_http.c 29 May 2002 17:04:38 -0000
> > >@@ -350,6 +350,20 @@
> > > * where the original request came from.
> > > */
> > > ap_table_mergen(req_hdrs, "X-Forwarded-For",
> > > r->connection->remote_ip);
> > >+ if (r->proxyreq == PROXY_PASS) {
> > >+ const char *buf;
> > >+ /* Add X-Forwarded-Host: so that upstream knows what the
> > >+ * original request hostname was.
> > >+ */
> > >+ if ((buf - ap_table_get(r->headers_in, "Host"))) {
> >
> > I think this should be "buf =" instead of "buf -".
>
> Yup, and:
>
> + ap_table_mergen(req_hdrs, "X-Forwarded-Server", r->server_hostname
>
> Should be r->server->server_hostname too.
>
> But it seems that getting the Host at this point is already to late.
> buf contains the remote hostname here already.
Which was fixed by the s/-/=/, thanks Greg.
--
Thomas Eibner <http://thomas.eibner.dk/> DnsZone <http://dnszone.org/>
mod_pointer <http://stderr.net/mod_pointer> <http://photos.eibner.dk/>
!(C)<http://copywrong.dk/> <http://apachegallery.dk/>
Putting the HEST in .COM <http://www.hestdesign.com/>
Re: [1.3] Proxy fixes and FWD: Re: [apache-modules] Setting bytes_sent in Request Record while generating all headers by myself in Apache 1.3]
Posted by Thomas Eibner <th...@stderr.net>.
On Wed, May 29, 2002 at 02:02:53PM -0400, Greg Marr wrote:
> At 01:30 PM 05/29/2002, Thomas Eibner wrote:
> >Index: proxy_http.c
> >===================================================================
> >RCS file: /home/cvspublic/apache-1.3/src/modules/proxy/proxy_http.c,v
> >retrieving revision 1.98
> >diff -u -r1.98 proxy_http.c
> >--- proxy_http.c 21 Apr 2002 21:16:39 -0000 1.98
> >+++ proxy_http.c 29 May 2002 17:04:38 -0000
> >@@ -350,6 +350,20 @@
> > * where the original request came from.
> > */
> > ap_table_mergen(req_hdrs, "X-Forwarded-For",
> > r->connection->remote_ip);
> >+ if (r->proxyreq == PROXY_PASS) {
> >+ const char *buf;
> >+ /* Add X-Forwarded-Host: so that upstream knows what the
> >+ * original request hostname was.
> >+ */
> >+ if ((buf - ap_table_get(r->headers_in, "Host"))) {
>
> I think this should be "buf =" instead of "buf -".
Yup, and:
+ ap_table_mergen(req_hdrs, "X-Forwarded-Server", r->server_hostname
Should be r->server->server_hostname too.
But it seems that getting the Host at this point is already to late.
buf contains the remote hostname here already.
--
Thomas Eibner <http://thomas.eibner.dk/> DnsZone <http://dnszone.org/>
mod_pointer <http://stderr.net/mod_pointer> <http://photos.eibner.dk/>
!(C)<http://copywrong.dk/> <http://apachegallery.dk/>
Putting the HEST in .COM <http://www.hestdesign.com/>
Re: [1.3] Proxy fixes and FWD: Re: [apache-modules] Setting
bytes_sent in Request Record while generating all headers by myself in
Apache 1.3]
Posted by Greg Marr <gr...@alum.wpi.edu>.
At 01:30 PM 05/29/2002, Thomas Eibner wrote:
>Index: proxy_http.c
>===================================================================
>RCS file: /home/cvspublic/apache-1.3/src/modules/proxy/proxy_http.c,v
>retrieving revision 1.98
>diff -u -r1.98 proxy_http.c
>--- proxy_http.c 21 Apr 2002 21:16:39 -0000 1.98
>+++ proxy_http.c 29 May 2002 17:04:38 -0000
>@@ -350,6 +350,20 @@
> * where the original request came from.
> */
> ap_table_mergen(req_hdrs, "X-Forwarded-For",
> r->connection->remote_ip);
>+ if (r->proxyreq == PROXY_PASS) {
>+ const char *buf;
>+ /* Add X-Forwarded-Host: so that upstream knows what the
>+ * original request hostname was.
>+ */
>+ if ((buf - ap_table_get(r->headers_in, "Host"))) {
I think this should be "buf =" instead of "buf -".
--
Greg Marr
gregm@alum.wpi.edu
"We thought you were dead."
"I was, but I'm better now." - Sheridan, "The Summoning"
Re: [1.3] Proxy fixes and FWD: Re: [apache-modules] Setting bytes_sent in Request Record while generating all headers by myself in Apache 1.3]
Posted by Thomas Eibner <th...@stderr.net>.
On Wed, May 29, 2002 at 07:44:16PM +0200, Graham Leggett wrote:
> Thomas Eibner wrote:
>
> > Inline patch here, but I'm wondering if you want the X-Forwarded-For
> > header to be stuck inside the conditional too?
>
> I think it should be... will sort this out later tonight or first thing
> tomorrow, have to leave the internet cafe now to fetch someone.
Good, there was a small mistake in the patch anyway. I'll setup a
test for it later so I can see if it actually works (doh).
--
Thomas Eibner <http://thomas.eibner.dk/> DnsZone <http://dnszone.org/>
mod_pointer <http://stderr.net/mod_pointer> <http://photos.eibner.dk/>
!(C)<http://copywrong.dk/> <http://apachegallery.dk/>
Putting the HEST in .COM <http://www.hestdesign.com/>
Re: [1.3] Proxy fixes and FWD: Re: [apache-modules] Setting bytes_sent
in Request Record while generating all headers by myself in Apache 1.3]
Posted by Graham Leggett <mi...@sharp.fm>.
Thomas Eibner wrote:
> Inline patch here, but I'm wondering if you want the X-Forwarded-For
> header to be stuck inside the conditional too?
I think it should be... will sort this out later tonight or first thing
tomorrow, have to leave the internet cafe now to fetch someone.
Regards,
Graham
--
-----------------------------------------
minfrin@sharp.fm "There's a moon
over Bourbon Street
tonight..."
Re: [1.3] Proxy fixes and FWD: Re: [apache-modules] Setting bytes_sent in Request Record while generating all headers by myself in Apache 1.3]
Posted by Thomas Eibner <th...@stderr.net>.
On Wed, May 29, 2002 at 07:20:17PM +0200, Graham Leggett wrote:
> Thomas Eibner wrote:
>
> > Ah yes, X-Forwarded-For is there, but not the two others there is in
> > 2.0 (X-Forwarded-Server and X-Forwared-Host) I read in the source that
> > someone thinks it needs to go into the Via header instead. And as I can
> > read from the source, X-Forwarded-For is only sent when it's a reverse
> > proxy request in 2.0, and always sent in 1.3.
>
> Oh yes... I remember now (memory rusty). If you can get a patch in
> before tomorrow, should be cool :)
Inline patch here, but I'm wondering if you want the X-Forwarded-For
header to be stuck inside the conditional too?
Index: proxy_http.c
===================================================================
RCS file: /home/cvspublic/apache-1.3/src/modules/proxy/proxy_http.c,v
retrieving revision 1.98
diff -u -r1.98 proxy_http.c
--- proxy_http.c 21 Apr 2002 21:16:39 -0000 1.98
+++ proxy_http.c 29 May 2002 17:04:38 -0000
@@ -350,6 +350,20 @@
* where the original request came from.
*/
ap_table_mergen(req_hdrs, "X-Forwarded-For", r->connection->remote_ip);
+ if (r->proxyreq == PROXY_PASS) {
+ const char *buf;
+ /* Add X-Forwarded-Host: so that upstream knows what the
+ * original request hostname was.
+ */
+ if ((buf - ap_table_get(r->headers_in, "Host"))) {
+ ap_table_mergen(req_hdrs, "X-Forwarded-Host", buf);
+ }
+ /* Add X-Forwarded-Server: so that upstream knows what the
+ * name of this proxy server is (if there are more than one)
+ * XXX: This duplicates Via: - do we strictly need it?
+ */
+ ap_table_mergen(req_hdrs, "X-Forwarded-Server", r->server_hostname);
+ }
/* we don't yet support keepalives - but we will soon, I promise! */
ap_table_set(req_hdrs, "Connection", "close");
--
Thomas Eibner <http://thomas.eibner.dk/> DnsZone <http://dnszone.org/>
mod_pointer <http://stderr.net/mod_pointer> <http://photos.eibner.dk/>
!(C)<http://copywrong.dk/> <http://apachegallery.dk/>
Putting the HEST in .COM <http://www.hestdesign.com/>
Re: [1.3] Proxy fixes and FWD: Re: [apache-modules] Setting bytes_sent
in Request Record while generating all headers by myself in Apache 1.3]
Posted by Graham Leggett <mi...@sharp.fm>.
Thomas Eibner wrote:
> Ah yes, X-Forwarded-For is there, but not the two others there is in
> 2.0 (X-Forwarded-Server and X-Forwared-Host) I read in the source that
> someone thinks it needs to go into the Via header instead. And as I can
> read from the source, X-Forwarded-For is only sent when it's a reverse
> proxy request in 2.0, and always sent in 1.3.
Oh yes... I remember now (memory rusty). If you can get a patch in
before tomorrow, should be cool :)
Regards,
Graham
--
-----------------------------------------
minfrin@sharp.fm "There's a moon
over Bourbon Street
tonight..."
Re: [1.3] Proxy fixes and FWD: Re: [apache-modules] Setting bytes_sent in Request Record while generating all headers by myself in Apache 1.3]
Posted by Thomas Eibner <th...@stderr.net>.
On Wed, May 29, 2002 at 07:10:25PM +0200, Graham Leggett wrote:
> Thomas Eibner wrote:
>
> > Looking at apache-1.3 in cvs VS httpd-2.0 there seems to be a few
> > changes, and the X-Forwarded-* headers are some of them. I have a
> > patch ready if needed (with what I believe are your comments in
> > it).
>
> Just checked - the X-Forwarded-For is definitely there in v1.3. Dunno
> about the send_bytes fix. Can someone check for me?
Ah yes, X-Forwarded-For is there, but not the two others there is in
2.0 (X-Forwarded-Server and X-Forwared-Host) I read in the source that
someone thinks it needs to go into the Via header instead. And as I can
read from the source, X-Forwarded-For is only sent when it's a reverse
proxy request in 2.0, and always sent in 1.3.
--
Thomas Eibner <http://thomas.eibner.dk/> DnsZone <http://dnszone.org/>
mod_pointer <http://stderr.net/mod_pointer> <http://photos.eibner.dk/>
!(C)<http://copywrong.dk/> <http://apachegallery.dk/>
Putting the HEST in .COM <http://www.hestdesign.com/>
Re: [1.3] Proxy fixes and FWD: Re: [apache-modules] Setting bytes_sent
in Request Record while generating all headers by myself in Apache 1.3]
Posted by Graham Leggett <mi...@sharp.fm>.
Thomas Eibner wrote:
> Looking at apache-1.3 in cvs VS httpd-2.0 there seems to be a few
> changes, and the X-Forwarded-* headers are some of them. I have a
> patch ready if needed (with what I believe are your comments in
> it).
Just checked - the X-Forwarded-For is definitely there in v1.3. Dunno
about the send_bytes fix. Can someone check for me?
Regards,
Graham
--
-----------------------------------------
minfrin@sharp.fm "There's a moon
over Bourbon Street
tonight..."
Re: [1.3] Proxy fixes and FWD: Re: [apache-modules] Setting bytes_sent in Request Record while generating all headers by myself in Apache 1.3]
Posted by Thomas Eibner <th...@stderr.net>.
On Wed, May 29, 2002 at 06:47:20PM +0200, Graham Leggett wrote:
> Thomas Eibner wrote:
>
> > Anyone looked at the remaining open bugs in 1.3 and might want to include
> > this patch (and bug)?
>
> Only if someone can verify that this patch actually does anything. The
> proxy has been largely rewritten since then, so this bug might not still
> be outstanding.
>
> There is a release coming out in another day or two, I am stuck offline
> - can someone take a look at this fix and see if it works...?
>
> > Would it also be possible to have mod_proxy for 1.3 set the same
> > X-Forwarded-* headers as the 2.0 proxy does? Need patches for this?
>
> I thought v1.3.24 already did this - don't remember if I added it to
> v1.3 as well, I am sure I did.
Looking at apache-1.3 in cvs VS httpd-2.0 there seems to be a few
changes, and the X-Forwarded-* headers are some of them. I have a
patch ready if needed (with what I believe are your comments in
it).
--
Thomas Eibner <http://thomas.eibner.dk/> DnsZone <http://dnszone.org/>
mod_pointer <http://stderr.net/mod_pointer> <http://photos.eibner.dk/>
!(C)<http://copywrong.dk/> <http://apachegallery.dk/>
Putting the HEST in .COM <http://www.hestdesign.com/>
Re: [1.3] Proxy fixes and FWD: Re: [apache-modules] Setting bytes_sent
in Request Record while generating all headers by myself in Apache 1.3]
Posted by Graham Leggett <mi...@sharp.fm>.
Thomas Eibner wrote:
> Anyone looked at the remaining open bugs in 1.3 and might want to include
> this patch (and bug)?
Only if someone can verify that this patch actually does anything. The
proxy has been largely rewritten since then, so this bug might not still
be outstanding.
There is a release coming out in another day or two, I am stuck offline
- can someone take a look at this fix and see if it works...?
> Would it also be possible to have mod_proxy for 1.3 set the same
> X-Forwarded-* headers as the 2.0 proxy does? Need patches for this?
I thought v1.3.24 already did this - don't remember if I added it to
v1.3 as well, I am sure I did.
Regards,
Graham
--
-----------------------------------------
minfrin@sharp.fm "There's a moon
over Bourbon Street
tonight..."
Re: [1.3] Proxy fixes and FWD: Re: [apache-modules] Setting bytes_sent
in Request Record while generating all headers by myself in Apache 1.3]
Posted by Graham Leggett <mi...@sharp.fm>.
Martin Kraemer wrote:
> About the X-Forwarded-* stuff: It's non-standard anyway (you can add
> any X-whatever header and still be RFC2616 compliant) so I'd rather
> not see it in 1.3 now (maybe in the next release ;-)
>
> I've seen proxies like squid hang because of nonstandard headers
> (proxy-connection: keep-alive) so I am not very fond of polluting the
> header namespace with more non-standard cruft.
These headers have been present for a long time in the httpd-2.0
mod_proxy, and are also added as standard by Squid, so they do
definitely work. As there is not likely to be another release unless new
bugs are found, I would rather add it now and see it included, rather
than add it later and have it never released.
I'll commit it now (as I am in an internet cafe and cannot commit it
later), however if there are any -1's please revert the commit before
the release.
Regards,
Graham
--
-----------------------------------------
minfrin@sharp.fm "There's a moon
over Bourbon Street
tonight..."
Re: [1.3] Proxy fixes and FWD: Re: [apache-modules] Setting bytes_sent in Request Record while generating all headers by myself in Apache 1.3]
Posted by Thomas Eibner <th...@stderr.net>.
On Wed, May 29, 2002 at 10:44:16PM +0200, Martin Kraemer wrote:
> Yes, the patch was correct (IMHO) and yes, I committed this one.
>
> About the X-Forwarded-* stuff: It's non-standard anyway (you can add
> any X-whatever header and still be RFC2616 compliant) so I'd rather
> not see it in 1.3 now (maybe in the next release ;-)
My only reasoning for wanting it in was that it's the way it works in
mod_proxy in 2.0.
> I've seen proxies like squid hang because of nonstandard headers
> (proxy-connection: keep-alive) so I am not very fond of polluting the
> header namespace with more non-standard cruft.
Not good.
--
Thomas Eibner <http://thomas.eibner.dk/> DnsZone <http://dnszone.org/>
mod_pointer <http://stderr.net/mod_pointer> <http://photos.eibner.dk/>
!(C)<http://copywrong.dk/> <http://apachegallery.dk/>
Putting the HEST in .COM <http://www.hestdesign.com/>
Re: [1.3] Proxy fixes and FWD: Re: [apache-modules] Setting bytes_sent in Request Record while generating all headers by myself in Apache 1.3]
Posted by Martin Kraemer <Ma...@Fujitsu-Siemens.com>.
On Wed, May 29, 2002 at 06:28:24PM +0200, Thomas Eibner wrote:
> From: Anthony Howe <ac...@snert.com>
> Subject: Re: [apache-modules] Setting bytes_sent in Request Record while generating
> all headers by myself in Apache 1.3
>
> >Number: 6841
> ap_kill_timeout(r);
> +
> + r->bytes_sent += total_bytes_rcvd;
> +
Yes, the patch was correct (IMHO) and yes, I committed this one.
About the X-Forwarded-* stuff: It's non-standard anyway (you can add
any X-whatever header and still be RFC2616 compliant) so I'd rather
not see it in 1.3 now (maybe in the next release ;-)
I've seen proxies like squid hang because of nonstandard headers
(proxy-connection: keep-alive) so I am not very fond of polluting the
header namespace with more non-standard cruft.
Martin
--
<Ma...@Fujitsu-Siemens.com> | Fujitsu Siemens
Fon: +49-89-636-46021, FAX: +49-89-636-47655 | 81730 Munich, Germany