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]

2002-05-30 Thread Graham Leggett

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
-- 
-
[EMAIL PROTECTED]There's a moon
over Bourbon Street
tonight...


smime.p7s
Description: S/MIME Cryptographic Signature


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]

2002-05-29 Thread Graham Leggett

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
-- 
-
[EMAIL PROTECTED]There's a moon
over Bourbon Street
tonight...


smime.p7s
Description: S/MIME Cryptographic Signature


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]

2002-05-29 Thread Thomas Eibner

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]

2002-05-29 Thread Thomas Eibner

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]

2002-05-29 Thread Graham Leggett

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
-- 
-
[EMAIL PROTECTED]There's a moon
over Bourbon Street
tonight...


smime.p7s
Description: S/MIME Cryptographic Signature


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]

2002-05-29 Thread Thomas Eibner

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.c21 Apr 2002 21:16:39 -  1.98
+++ proxy_http.c29 May 2002 17:04:38 -
 -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]

2002-05-29 Thread Graham Leggett

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
-- 
-
[EMAIL PROTECTED]There's a moon
over Bourbon Street
tonight...


smime.p7s
Description: S/MIME Cryptographic Signature


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]

2002-05-29 Thread Thomas Eibner

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]

2002-05-29 Thread Greg Marr

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.c21 Apr 2002 21:16:39 -  1.98
+++ proxy_http.c29 May 2002 17:04:38 -
@@ -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
[EMAIL PROTECTED]
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]

2002-05-29 Thread Thomas Eibner

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.c21 Apr 2002 21:16:39 -  1.98
 +++ proxy_http.c29 May 2002 17:04:38 -
  -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]

2002-05-29 Thread Thomas Eibner

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.c21 Apr 2002 21:16:39 -  1.98
  +++ proxy_http.c29 May 2002 17:04:38 -
   -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]

2002-05-29 Thread Martin Kraemer

On Wed, May 29, 2002 at 06:28:24PM +0200, Thomas Eibner wrote:
 From: Anthony Howe [EMAIL PROTECTED]
 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
-- 
[EMAIL PROTECTED] | Fujitsu Siemens
Fon: +49-89-636-46021, FAX: +49-89-636-47655 | 81730  Munich,  Germany



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]

2002-05-29 Thread Thomas Eibner

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/