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
MIME-Version: 1.0
X-Mailer: MIME-tools 5.427 (Entity 5.427)
X-RT-Original-Encoding: utf-8
Content-Type: multipart/mixed; boundary="----------=_1265821173-14824-95"
Content-Length: 0
Content-Type: text/plain; charset="UTF-8"
Content-Disposition: inline
Content-Transfer-Encoding: binary
Content-Length: 646
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
MIME-Version: 1.0
Content-Type: text/x-patch; name="CGI.pm.patch"
X-Mailer: MIME-tools 5.427 (Entity 5.427)
Content-Disposition: inline; filename="CGI.pm.patch"
Content-Transfer-Encoding: binary
Content-Length: 696
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();
MIME-Version: 1.0
X-Spam-Status: No, hits=-0.0 required=8.0 tests=SPF_PASS
In-Reply-To: <rt-3.8.HEAD-14824-1265821178-696.54487-4-0 [...] rt.cpan.org>
X-Mailer: Claws Mail 3.7.5 (GTK+ 2.12.9; i486-pc-linux-gnu)
References: <RT-Ticket-54487 [...] rt.cpan.org> <rt-3.8.HEAD-14824-1265821178-696.54487-4-0 [...] rt.cpan.org>
Message-ID: <20100210120712.56dbc2e5 [...] summersault.com>
Content-Type: text/plain; charset="utf-8"
X-RT-Original-Encoding: utf-8
Received: from la.mx.develooper.com (x1.develooper.com [207.171.7.70]) by diesel.bestpractical.com (Postfix) with SMTP id E8F144D8165 for <bug-CGI.pm [...] rt.cpan.org>; Wed, 10 Feb 2010 12:07:59 -0500 (EST)
Received: (qmail 8275 invoked by uid 103); 10 Feb 2010 17:07:58 -0000
Received: from x16.dev (10.0.100.26) by x1.dev with QMQP; 10 Feb 2010 17:07:58 -0000
Received: from tanagra.summersault.com (HELO tanagra.summersault.com) (12.161.105.149) by 16.mx.develooper.com (qpsmtpd/0.80) with SMTP; Wed, 10 Feb 2010 09:07:23 -0800
Received: (qmail 20384 invoked from network); 10 Feb 2010 17:07:12 -0000
Received: from simba.summersault.com (HELO localhost) (192.168.97.182) by tanagra.summersault.com with SMTP; 10 Feb 2010 17:07:12 -0000
Delivered-To: cpan-bug+CGI.pm [...] diesel.bestpractical.com
Subject: Re: [rt.cpan.org #54487] url() does not handle multi-valued X-Forwarded-Host
Return-Path: <mark [...] summersault.com>
X-Spam-Check-BY: 16.mx.develooper.com
X-Original-To: bug-CGI.pm [...] rt.cpan.org
Date: Wed, 10 Feb 2010 12:07:12 -0500
X-Spam-Level: *
To: bug-CGI.pm [...] rt.cpan.org
Content-Transfer-Encoding: 7bit
From: Mark Stosberg <mark [...] summersault.com>
RT-Message-ID: <rt-3.8.HEAD-14812-1265821692-332.54487-0-0 [...] rt.cpan.org>
Content-Length: 779
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
MIME-Version: 1.0
In-Reply-To: <rt-3.8.HEAD-14812-1265821692-332.54487-0-0 [...] rt.cpan.org>
X-Mailer: MIME-tools 5.427 (Entity 5.427)
References: <RT-Ticket-54487 [...] rt.cpan.org> <rt-3.8.HEAD-14824-1265821178-696.54487-4-0 [...] rt.cpan.org> <20100210120712.56dbc2e5 [...] summersault.com> <rt-3.8.HEAD-14812-1265821692-332.54487-0-0 [...] rt.cpan.org>
Content-Type: multipart/mixed; boundary="----------=_1275987058-10890-105"
Message-ID: <rt-3.8.HEAD-10890-1275987058-1500.54487-0-0 [...] rt.cpan.org>
From: ray [...] 1729.org.uk
X-RT-Original-Encoding: utf-8
Content-Length: 0
Content-Disposition: inline
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: binary
X-RT-Original-Encoding: utf-8
Content-Length: 197
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.
MIME-Version: 1.0
Subject: CGI.pm.patch
X-Mailer: MIME-tools 5.427 (Entity 5.427)
Content-Type: text/x-patch; name="CGI.pm.patch"
Content-Disposition: inline; filename="CGI.pm.patch"
Content-Transfer-Encoding: binary
Content-Length: 1252
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';
X-RT-Interface: REST
MIME-Version: 1.0
X-Mailer: MIME-tools 5.504 (Entity 5.504)
RT-Message-ID: <rt-4.0.18-25657-1400760453-681.54487-0-0 [...] rt.cpan.org>
Content-Type: text/plain; charset="utf-8"
Content-Disposition: inline
Content-Transfer-Encoding: binary
Content-Length: 240
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.
MIME-Version: 1.0
X-Mailer: MIME-tools 5.504 (Entity 5.504)
Content-Disposition: inline
X-RT-Interface: Web
Content-Type: text/plain; charset="utf-8"
Message-ID: <rt-4.0.18-11005-1405084122-536.54487-0-0 [...] rt.cpan.org>
Content-Transfer-Encoding: binary
X-RT-Original-Encoding: utf-8
X-RT-Encrypt: 0
X-RT-Sign: 0
Content-Length: 438
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.