Skip Menu |

Preferred bug tracker

Please visit the preferred bug tracker to report your issue.

This queue is for tickets about the CGI CPAN distribution.

Report information
The Basics
Id: 54487
Status: resolved
Priority: 0/
Queue: CGI
People
Owner: Nobody in particular
Requestors: ray [...] 1729.org.uk
Cc:
AdminCc:
Bug Information
Severity: Important
Broken in: (no value)
Fixed in: (no value)

History Show all quoted text
  Wed Feb 10 11:59:38 2010 ray [...] 1729.org.uk - Ticket created
Subject: url() does not handle multi-valued X-Forwarded-Host
The url() method in CGI.pm examines the X-Forwarded-Host header to determine the vhost name, but does not cater for this header containing a comma-separated list (which can happen if the request has passed through multiple reverse proxies). The apache documentation <http://httpd.apache.org/docs/2.2/mod/mod_proxy.html> says: "Be careful when using these headers on the origin server, since they will contain more than one (comma-separated) value if the original request already contained one of these headers." The Catalyst code caters for this by taking the last value in the list. The attached patch makes CGI.pm follow the same behaviour.
Subject: CGI.pm.patch
diff -uNr CGI.pm-3.49.ORI/lib/CGI.pm CGI.pm-3.49/lib/CGI.pm --- CGI.pm-3.49.ORI/lib/CGI.pm 2010-01-29 14:41:54.000000000 +0000 +++ CGI.pm-3.49/lib/CGI.pm 2010-02-10 16:55:24.718751000 +0000 @@ -2856,6 +2856,8 @@ my $protocol = $self->protocol(); $url = "$protocol://"; my $vh = http('x_forwarded_host') || http('host') || ''; + $vh =~ s/^.*,\s*//; # x_forwarded_host may be a comma-separated list (e.g. when the request has + # passed through multiple reverse proxies. Take the last one. $vh =~ s/\:\d+$//; # some clients add the port number (incorrectly). Get rid of it. $url .= $vh || server_name();
  Wed Feb 10 12:08:12 2010 mark [...] summersault.com - Correspondence added
Subject: Re: [rt.cpan.org #54487] url() does not handle multi-valued X-Forwarded-Host
Date: Wed, 10 Feb 2010 12:07:12 -0500
To: bug-CGI.pm [...] rt.cpan.org
From: Mark Stosberg <mark [...] summersault.com>
Show quoted text
> The url() method in CGI.pm examines the X-Forwarded-Host header to > determine the vhost name, but does not cater for this header containing > a comma-separated list (which can happen if the request has passed > through multiple reverse proxies). > > The apache documentation > <http://httpd.apache.org/docs/2.2/mod/mod_proxy.html> says: > > "Be careful when using these headers on the origin server, since they > will contain more than one (comma-separated) value if the original > request already contained one of these headers." > > The Catalyst code caters for this by taking the last value in the list. > The attached patch makes CGI.pm follow the same behaviour.
Thanks for the report, Ray. Would you be willing to add an automated test to go with it? Mark
  Wed Feb 10 12:08:13 2010 The RT System itself - Status changed from 'new' to 'open'
  Tue Jun 08 04:50:58 2010 ray [...] 1729.org.uk - Correspondence added
From: ray [...] 1729.org.uk
On Wed Feb 10 12:08:12 2010, mark@summersault.com wrote: Show quoted text
> > Would you be willing to add an automated test > to go with it?
Yes, of course. Please see attached patch which adds a test. Ray.
Subject: CGI.pm.patch
diff -uNr CGI.pm-3.49.ORI/lib/CGI.pm CGI.pm-3.49/lib/CGI.pm --- CGI.pm-3.49.ORI/lib/CGI.pm 2010-01-29 14:41:54.000000000 +0000 +++ CGI.pm-3.49/lib/CGI.pm 2010-06-08 09:45:45.262080000 +0100 @@ -2856,6 +2856,8 @@ my $protocol = $self->protocol(); $url = "$protocol://"; my $vh = http('x_forwarded_host') || http('host') || ''; + $vh =~ s/^.*,\s*//; # x_forwarded_host may be a comma-separated list (e.g. when the request has + # passed through multiple reverse proxies. Take the last one. $vh =~ s/\:\d+$//; # some clients add the port number (incorrectly). Get rid of it. $url .= $vh || server_name(); diff -uNr CGI.pm-3.49.ORI/t/url.t CGI.pm-3.49/t/url.t --- CGI.pm-3.49.ORI/t/url.t 2010-01-29 14:41:54.000000000 +0000 +++ CGI.pm-3.49/t/url.t 2010-06-08 09:42:50.744084000 +0100 @@ -1,7 +1,7 @@ use strict; use warnings; -use Test::More tests => 4; # last test to print +use Test::More tests => 5; # last test to print use CGI qw/ :all /; @@ -21,3 +21,6 @@ is url() => 'http://proxy', 'url() with default port'; +$ENV{HTTP_X_FORWARDED_HOST} = 'proxy1:80, proxy2:8484'; + +is url() => 'http://proxy2:8484', 'url() with multiple proxies';
  Thu May 22 08:07:33 2014 LEEJO [...] cpan.org - Correspondence added
This issue has been copied to: https://github.com/leejo/CGI.pm/issues/70 please take all future correspondence there. This ticket will remain open but please do not reply here. This ticket will be closed when the github issue is dealt with.
  Fri May 23 14:28:05 2014 The RT System itself - Queue changed from CGI.pm to CGI
  Fri Jul 11 09:08:42 2014 LEEJO [...] cpan.org - Correspondence added
commit 786165e1ed07e42b2590608ec117a0dcb366d39c Author: Lee Johnson <lee@givengain.ch> Date: Fri Jul 11 15:07:09 2014 +0200 resolve #70 [rt.cpan.org #54487] - X-Forwarded-Host header with multiple comma separated values will now use the final value in the list, as per other web framework implementations. add test case. Changes | 5 +++++ lib/CGI.pm | 2 ++ t/url.t | 4 ++++ 3 files changed, 11 insertions(+)
  Fri Jul 11 09:08:44 2014 LEEJO [...] cpan.org - Status changed from 'open' to 'resolved'