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)



Subject: url() does not handle multi-valued X-Forwarded-Host
Download (untitled) / with headers
text/plain 646b
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
Download CGI.pm.patch
text/x-diff 696b
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();
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>
Download (untitled) / with headers
text/plain 779b
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
From: ray [...] 1729.org.uk
Download (untitled) / with headers
text/plain 197b
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
Download CGI.pm.patch
text/x-diff 1.2k
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';
Download (untitled) / with headers
text/plain 240b
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.
Download (untitled) / with headers
text/plain 438b
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(+)


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.