[Catalyst-dev] Proxy headers patch take two
Dave Rolsky
autarch at urth.org
Sun Jun 17 19:55:52 GMT 2007
Ok, I thought about my first patch and realized that adjusting for the
proxy after the other prepare_* methods were called was kind of gross.
Unfortnuately, the only alternative I could see has its own grossness, in
that each engine must call $self->_proxy_info($c) in various methods in
order to account for a proxy.
The attached patch implements the proxy-related bits fully for the
Catalyst-Runtime distro and Catalyst-Engine-Apache.
It also partially implements this for the Zeus and SCGI engines.
There are some outstanding issues:
* SCGI and Zeus prepare_path() needs to be adjusted in a similar way to
C::E::CGI and C::E::Apache
* The POE server was not altered at all, because I don't understand it ;)
* Is C::E::Server still relevant? It doesn't seem like it, but it may need
updating if so.
* I really hate how the proxy info is cached in the Catalyst object by the
Engine object. This is really unclean, but it's a general problem with
Catalyst, AFAICT.
* I also really hate how the path manipulation stuff is so grotty in
trying to handle doubled slashes.
I could imagine a different implementation that did something like provide
a per-engine RequestInfo class that could be used by Catalyst::Engine to
centralize the logic of the prepare_* methods. This RequestInfo class
would basically be an abstraction on top of CGI/ModPerl/Zeus/POE/whatever.
But I didn't want to go down that road unless there was a very clear
consensus on what that would look like, and whether it should happen at
all.
-dave
/*===================================================
VegGuide.Org www.BookIRead.com
Your guide to all that's veg. My book blog
===================================================*/
-------------- next part --------------
Index: Catalyst-Engine-Apache/t/live_engine_request_headers.t
===================================================================
--- Catalyst-Engine-Apache/t/live_engine_request_headers.t (revision 6466)
+++ Catalyst-Engine-Apache/t/live_engine_request_headers.t (working copy)
@@ -6,7 +6,7 @@
use FindBin;
use lib "$FindBin::Bin/lib";
-use Test::More tests => 17;
+use Test::More tests => 27;
use Catalyst::Test 'TestApp';
use Catalyst::Request;
@@ -17,12 +17,18 @@
{
my $creq;
- my $request = GET( 'http://localhost/dump/request',
- 'User-Agent' => 'MyAgen/1.0',
- 'X-Whats-Cool' => 'Catalyst',
- 'X-Multiple' => [ 1 .. 5 ],
- 'X-Forwarded-Host' => 'frontend.server.com',
- 'X-Forwarded-For' => '192.168.1.1, 1.2.3.4',
+ my $request = GET(
+ 'http://localhost/dump/request',
+ 'User-Agent' => 'MyAgen/1.0',
+ 'X-Whats-Cool' => 'Catalyst',
+ 'X-Multiple' => [ 1 .. 5 ],
+ 'X-Forwarded-Host' => 'frontend.server.com',
+ 'X-Forwarded-For' => '192.168.1.1, 1.2.3.4',
+ # Trailing slash is intentional - tests that we don't generate
+ # paths with doubled slashes
+ 'X-Forwarded-Path' => '/prefix/',
+ 'X-Forwarded-Port' => '12345',
+ 'X-Forwarded-Is-SSL' => 1,
);
ok( my $response = request($request), 'Request' );
@@ -50,11 +56,21 @@
SKIP:
{
if ( $ENV{CATALYST_SERVER} && $ENV{CATALYST_SERVER} !~ /127.0.0.1|localhost/ ) {
- skip "Using remote server", 2;
+ skip "Using remote server", 12;
}
- is( $creq->base->host, 'frontend.server.com', 'Catalyst::Request proxied base' );
- is( $creq->address, '1.2.3.4', 'Catalyst::Request proxied address' );
+ is( $creq->address, '1.2.3.4', 'X-Forwarded-For header => address()' );
+ is( $creq->hostname, 'frontend.server.com', 'X-Forwarded-Host header => hostname()' );
+ is( $creq->base->host, 'frontend.server.com', 'X-Forwarded-Host => base()->host()' );
+ is( $creq->uri->host, 'frontend.server.com', 'X-Forwarded-Host => uri()->host()' );
+ is( $creq->base->path, '/prefix/', 'X-Forwarded-Path => base()->path()' );
+ is( $creq->uri->path, '/prefix/dump/request', 'X-Forwarded-Path => uri()->path()' );
+ is( $creq->base->port, 12345, 'X-Forwarded-Port => base()->port()' );
+ is( $creq->uri->port, 12345, 'X-Forwarded-Port => uri()->port()' );
+ is( $creq->protocol, 'https', 'X-Forwarded-Is-Secure => protocol()' );
+ ok( $creq->secure, 'X-Forwarded-Is-Secure => secure()' );
+ is( $creq->base->scheme, 'https', 'X-Forwarded-Is-Secure => base()->scheme()' );
+ is( $creq->uri->scheme, 'https', 'X-Forwarded-Is-Secure => uri()->scheme()' );
}
SKIP:
Index: Catalyst-Engine-Apache/lib/Catalyst/Engine/Apache.pm
===================================================================
--- Catalyst-Engine-Apache/lib/Catalyst/Engine/Apache.pm (revision 6466)
+++ Catalyst-Engine-Apache/lib/Catalyst/Engine/Apache.pm (working copy)
@@ -26,25 +26,11 @@
sub prepare_connection {
my ( $self, $c ) = @_;
- $c->request->address( $self->apache->connection->remote_ip );
+ my $proxy = $self->_proxy_info($c);
- PROXY_CHECK:
- {
- my $headers = $self->apache->headers_in;
- unless ( $c->config->{using_frontend_proxy} ) {
- last PROXY_CHECK if $c->request->address ne '127.0.0.1';
- last PROXY_CHECK if $c->config->{ignore_frontend_proxy};
- }
- last PROXY_CHECK unless $headers->{'X-Forwarded-For'};
-
- # If we are running as a backend server, the user will always appear
- # as 127.0.0.1. Select the most recent upstream IP (last in the list)
- my ($ip) = $headers->{'X-Forwarded-For'} =~ /([^,\s]+)$/;
- $c->request->address( $ip );
- }
-
- $c->request->hostname( $self->apache->connection->remote_host );
- $c->request->protocol( $self->apache->protocol );
+ $c->request->address( $proxy->{address} || $self->apache->connection->remote_ip );
+ $c->request->hostname( $proxy->{host} || $self->apache->connection->remote_host );
+ $c->request->protocol( $proxy->{scheme} || $self->apache->protocol );
$c->request->user( $self->apache->user );
# when config options are set, check them here first
@@ -55,8 +41,17 @@
$c->request->secure(1) if defined $https and uc $https eq 'ON';
}
+ if ( $c->request->protocol eq 'https' ) {
+ $c->request->secure(1);
+ }
}
+sub _ip_address_without_proxy {
+ my ( $self, $c ) = @_;
+
+ return $self->apache->connection->remote_ip;
+}
+
sub prepare_query_parameters {
my ( $self, $c ) = @_;
@@ -78,31 +73,12 @@
sub prepare_path {
my ( $self, $c ) = @_;
+ my $proxy = $self->_proxy_info($c);
+
my $scheme = $c->request->secure ? 'https' : 'http';
- my $host = $self->apache->hostname || 'localhost';
- my $port = $self->apache->get_server_port;
+ my $host = $proxy->{host} || $self->apache->hostname || 'localhost';
+ my $port = $proxy->{port} || $self->apache->get_server_port;
- # If we are running as a backend proxy, get the true hostname
- PROXY_CHECK:
- {
- unless ( $c->config->{using_frontend_proxy} ) {
- last PROXY_CHECK if $host !~ /localhost|127.0.0.1/;
- last PROXY_CHECK if $c->config->{ignore_frontend_proxy};
- }
- last PROXY_CHECK unless $c->request->header( 'X-Forwarded-Host' );
-
- $host = $c->request->header( 'X-Forwarded-Host' );
-
- if ( $host =~ /^(.+):(\d+)$/ ) {
- $host = $1;
- $port = $2;
- } else {
- # backend could be on any port, so
- # assume frontend is on the default port
- $port = $c->request->secure ? 443 : 80;
- }
- }
-
my $base_path = '';
# Are we running in a non-root Location block?
@@ -110,6 +86,9 @@
if ( $location && $location ne '/' ) {
$base_path = $location;
}
+
+ $base_path = $proxy->{path} . $base_path
+ if $proxy->{path};
# Using URI directly is way too slow, so we construct the URLs manually
my $uri_class = "URI::$scheme";
@@ -130,7 +109,11 @@
my (undef, $path_query) = split / /, $self->apache->the_request, 3;
($path, $qs) = split /\?/, $path_query, 2;
}
-
+
+ $path = $proxy->{path} . $path
+ if $proxy->{path};
+ $path =~ s[/{2,}][/]g;
+
# Check if $base_path appears to be a regex (contains invalid characters),
# meaning we're in a LocationMatch block
if ( $base_path =~ m/[^$URI::uric]/o ) {
Index: Catalyst-Engine-Zeus/lib/Catalyst/Engine/Zeus/Base.pm
===================================================================
--- Catalyst-Engine-Zeus/lib/Catalyst/Engine/Zeus/Base.pm (revision 6466)
+++ Catalyst-Engine-Zeus/lib/Catalyst/Engine/Zeus/Base.pm (working copy)
@@ -114,16 +114,29 @@
sub prepare_connection {
my $c = shift;
- $c->request->address( $c->zeus->connection->remote_ip );
- $c->request->hostname( $c->zeus->connection->remote_host );
- $c->request->protocol( $c->zeus->protocol );
+
+ my $proxy = $self->_proxy_info($c);
+
+ $c->request->address( $proxy->{address} || $c->zeus->connection->remote_ip );
+ $c->request->hostname( $proxy->{host} || $c->zeus->connection->remote_host );
+ $c->request->protocol( $proxy->{scheme} || $c->zeus->protocol );
$c->request->user( $c->zeus->user );
if ( $ENV{HTTPS} || $c->zeus->get_server_port == 443 ) {
$c->request->secure(1);
}
+
+ if ( $c->request->protocol eq 'https' ) {
+ $c->request->secure(1);
+ }
}
+sub _ip_address_without_proxy {
+ my ( $self, $c ) = @_;
+
+ return $self->zeus->connection->remote_ip;
+}
+
=item $c->prepare_headers
=cut
Index: Catalyst-Runtime/t/live_engine_request_headers.t
===================================================================
--- Catalyst-Runtime/t/live_engine_request_headers.t (revision 6469)
+++ Catalyst-Runtime/t/live_engine_request_headers.t (working copy)
@@ -6,7 +6,7 @@
use FindBin;
use lib "$FindBin::Bin/lib";
-use Test::More tests => 17;
+use Test::More tests => 27;
use Catalyst::Test 'TestApp';
use Catalyst::Request;
@@ -17,12 +17,18 @@
{
my $creq;
- my $request = GET( 'http://localhost/dump/request',
- 'User-Agent' => 'MyAgen/1.0',
- 'X-Whats-Cool' => 'Catalyst',
- 'X-Multiple' => [ 1 .. 5 ],
- 'X-Forwarded-Host' => 'frontend.server.com',
- 'X-Forwarded-For' => '192.168.1.1, 1.2.3.4',
+ my $request = GET(
+ 'http://localhost/dump/request',
+ 'User-Agent' => 'MyAgen/1.0',
+ 'X-Whats-Cool' => 'Catalyst',
+ 'X-Multiple' => [ 1 .. 5 ],
+ 'X-Forwarded-Host' => 'frontend.server.com',
+ 'X-Forwarded-For' => '192.168.1.1, 1.2.3.4',
+ # Trailing slash is intentional - tests that we don't generate
+ # paths with doubled slashes
+ 'X-Forwarded-Path' => '/prefix/',
+ 'X-Forwarded-Port' => '12345',
+ 'X-Forwarded-Is-SSL' => 1,
);
ok( my $response = request($request), 'Request' );
@@ -50,11 +56,21 @@
SKIP:
{
if ( $ENV{CATALYST_SERVER} && $ENV{CATALYST_SERVER} !~ /127.0.0.1|localhost/ ) {
- skip "Using remote server", 2;
+ skip "Using remote server", 12;
}
- is( $creq->base->host, 'frontend.server.com', 'Catalyst::Request proxied base' );
- is( $creq->address, '1.2.3.4', 'Catalyst::Request proxied address' );
+ is( $creq->address, '1.2.3.4', 'X-Forwarded-For header => address()' );
+ is( $creq->hostname, 'frontend.server.com', 'X-Forwarded-Host header => hostname()' );
+ is( $creq->base->host, 'frontend.server.com', 'X-Forwarded-Host => base()->host()' );
+ is( $creq->uri->host, 'frontend.server.com', 'X-Forwarded-Host => uri()->host()' );
+ is( $creq->base->path, '/prefix/', 'X-Forwarded-Path => base()->path()' );
+ is( $creq->uri->path, '/prefix/dump/request', 'X-Forwarded-Path => uri()->path()' );
+ is( $creq->base->port, 12345, 'X-Forwarded-Port => base()->port()' );
+ is( $creq->uri->port, 12345, 'X-Forwarded-Port => uri()->port()' );
+ is( $creq->protocol, 'https', 'X-Forwarded-Is-Secure => protocol()' );
+ ok( $creq->secure, 'X-Forwarded-Is-Secure => secure()' );
+ is( $creq->base->scheme, 'https', 'X-Forwarded-Is-Secure => base()->scheme()' );
+ is( $creq->uri->scheme, 'https', 'X-Forwarded-Is-Secure => uri()->scheme()' );
}
SKIP:
Index: Catalyst-Runtime/lib/Catalyst.pm
===================================================================
--- Catalyst-Runtime/lib/Catalyst.pm (revision 6469)
+++ Catalyst-Runtime/lib/Catalyst.pm (working copy)
@@ -1583,9 +1583,9 @@
}
else {
$c->prepare_request(@arguments);
+ $c->prepare_headers;
$c->prepare_connection;
$c->prepare_query_parameters;
- $c->prepare_headers;
$c->prepare_cookies;
$c->prepare_path;
@@ -1713,6 +1713,14 @@
sub prepare_path { my $c = shift; $c->engine->prepare_path( $c, @_ ) }
+=head2 $c->adjust_request_for_proxy
+
+Adjusts the request to account for a frontend proxy.
+
+=cut
+
+sub adjust_request_for_proxy { my $c = shift; $c->engine->adjust_request_for_proxy( $c, @_ ) }
+
=head2 $c->prepare_query_parameters
Prepares query parameters.
@@ -2256,12 +2264,39 @@
the frontend and backend servers on the same machine. The following
changes are made to the request.
- $c->req->address is set to the user's real IP address, as read from
- the HTTP X-Forwarded-For header.
-
- The host value for $c->req->base and $c->req->uri is set to the real
- host, as read from the HTTP X-Forwarded-Host header.
+=over 4
+=item * X-Forwarded-For
+
+The IP address in C<< $c->req->address >> is set to the user's real IP
+address, as read from the X-Forwarded-For header.
+
+=item * X-Forwarded-Host
+
+The host value for C<< $c->req->base >> and C<< $c->req->uri >> is set
+to the real host, as read from the X-Forwarded-Host header. The value
+of C<< $c->req->hostname >> is also adjusted accordingly.
+
+=item * X-Forwarded-Port
+
+The port value for C<< $c->req->base >> and C<< $c->req->uri >> is set
+to the real port, as read from the X-Forwarded-Port header.
+
+=item * X-Forwarded-Path
+
+If this is set, the value of the X-Forwarded-Path header is
+I<prepended> to the path value of C<< $c->req->base >> and C<<
+$c->req->uri >>.
+
+=item * X-Forwarded-Is-SSL
+
+If this is set, the scheme value of C<< $c->req->base >> and C<<
+$c->req->uri >> is set to "https". Additional, C<< $c->req->protocol
+>> is also set to "https", and C<< $c->req->secure >> is set to a true
+value.
+
+=back
+
Obviously, your web server must support these headers for this to work.
In a more complex server farm environment where you may have your
Index: Catalyst-Runtime/lib/Catalyst/Engine.pm
===================================================================
--- Catalyst-Runtime/lib/Catalyst/Engine.pm (revision 6469)
+++ Catalyst-Runtime/lib/Catalyst/Engine.pm (working copy)
@@ -397,6 +397,104 @@
sub prepare_headers { }
+=head2 $self->_proxy_info($c)
+
+Checks for the presence of various headers from a frontend proxy, and
+returns a hash of information based on what it finds.
+
+This method is intended to be called by engines in their various
+C<prepare_XXX()> methods so that they can override values based on
+proxy headers.
+
+This method returns a hash which may have one or more of the following
+keys:
+
+=over 4
+
+=item * host
+
+=item * port
+
+=item * path
+
+=item * scheme
+
+The only value used for scheme is "https".
+
+=back
+
+If the config key "ignore_frontend_proxy" is true, no adjustments are
+made.
+
+If the config key "using_frontend_proxy" is I<not> true, then we do
+not make adjustments nuless the client's IP address is 127.0.0.1
+(localhost).
+
+=head3 Subclassing
+
+If you are creating a new Engine subclass, you may want to add a
+method named C<_ip_address_without_proxy()>. This method will be
+called when checking whether or not to respect proxy headers. It
+should return the "raw" IP address of the connection, without looking
+at the "X-Forwarded-For" header.
+
+This class provides an implementation of this method that simply
+returns C<$ENV{REMOTE_ADDR}>, but you may wish to override this
+implementation.
+
+=cut
+
+sub _proxy_info {
+ my ( $self, $c, $ip_address ) = @_;
+
+ return $c->{proxy_info}
+ if $c->{proxy_info};
+
+ unless ( $self->_check_for_proxy($c) ) {
+ return $c->{proxy_info} = {};
+ }
+
+ my %proxy;
+ if ( my $for = $c->request->header('X-Forwarded-For') ) {
+ ($proxy{address}) = $for =~ /([^,\s]+)$/
+ }
+
+ $proxy{host} = $c->request->header('X-Forwarded-Host');
+ $proxy{port} = $c->request->header('X-Forwarded-Port');
+
+ $proxy{path} = $c->request->header('X-Forwarded-Path');
+ $proxy{path} =~ s{/$}{}
+ if $proxy{path};
+
+ $proxy{scheme} = 'https'
+ if $c->request->header('X-Forwarded-Is-SSL');
+
+ $c->{proxy_info} = \%proxy;
+
+ return $c->{proxy_info};
+}
+
+sub _check_for_proxy {
+ my ( $self, $c, $ip_address ) = @_;
+
+ return 0 if $c->config->{ignore_frontend_proxy};
+
+ my $address = $self->_ip_address_without_proxy($c);
+
+ return 0 unless $c->config->{using_frontend_proxy}
+ || $address eq '127.0.0.1';
+
+ return 1;
+}
+
+# This method is provided mainly as a fallback for older versions of
+# engines that don't implement this method themselves. Given that most
+# web environments emulate the CGI environment to some extent,
+# checking $ENV{REMOTE_ADDR} has a decent chance of being correct.
+sub _ip_address_without_proxy {
+ return $ENV{REMOTE_ADDR};
+}
+
=head2 $self->prepare_parameters($c)
sets up parameters from query and post parameters.
Index: Catalyst-Runtime/lib/Catalyst/Engine/CGI.pm
===================================================================
--- Catalyst-Runtime/lib/Catalyst/Engine/CGI.pm (revision 6469)
+++ Catalyst-Runtime/lib/Catalyst/Engine/CGI.pm (working copy)
@@ -53,24 +53,11 @@
my ( $self, $c ) = @_;
local (*ENV) = $self->env || \%ENV;
- $c->request->address( $ENV{REMOTE_ADDR} );
+ my $proxy = $self->_proxy_info($c);
- PROXY_CHECK:
- {
- unless ( $c->config->{using_frontend_proxy} ) {
- last PROXY_CHECK if $ENV{REMOTE_ADDR} ne '127.0.0.1';
- last PROXY_CHECK if $c->config->{ignore_frontend_proxy};
- }
- last PROXY_CHECK unless $ENV{HTTP_X_FORWARDED_FOR};
-
- # If we are running as a backend server, the user will always appear
- # as 127.0.0.1. Select the most recent upstream IP (last in the list)
- my ($ip) = $ENV{HTTP_X_FORWARDED_FOR} =~ /([^,\s]+)$/;
- $c->request->address($ip);
- }
-
- $c->request->hostname( $ENV{REMOTE_HOST} );
- $c->request->protocol( $ENV{SERVER_PROTOCOL} );
+ $c->request->address( $proxy->{address} || $ENV{REMOTE_ADDR} );
+ $c->request->hostname( $proxy->{host} || $ENV{REMOTE_HOST} );
+ $c->request->protocol( $proxy->{scheme} || $ENV{SERVER_PROTOCOL} );
$c->request->user( $ENV{REMOTE_USER} );
$c->request->method( $ENV{REQUEST_METHOD} );
@@ -81,6 +68,10 @@
if ( $ENV{SERVER_PORT} == 443 ) {
$c->request->secure(1);
}
+
+ if ( $c->request->protocol eq 'https' ) {
+ $c->request->secure(1);
+ }
}
=head2 $self->prepare_headers($c)
@@ -107,9 +98,11 @@
my ( $self, $c ) = @_;
local (*ENV) = $self->env || \%ENV;
+ my $proxy = $self->_proxy_info($c);
+
my $scheme = $c->request->secure ? 'https' : 'http';
- my $host = $ENV{HTTP_HOST} || $ENV{SERVER_NAME};
- my $port = $ENV{SERVER_PORT} || 80;
+ my $host = $proxy->{host} || $ENV{HTTP_HOST} || $ENV{SERVER_NAME};
+ my $port = $proxy->{port} || $ENV{SERVER_PORT} || 80;
my $base_path;
if ( exists $ENV{REDIRECT_URL} ) {
$base_path = $ENV{REDIRECT_URL};
@@ -119,26 +112,14 @@
$base_path = $ENV{SCRIPT_NAME} || '/';
}
- # If we are running as a backend proxy, get the true hostname
- PROXY_CHECK:
- {
- unless ( $c->config->{using_frontend_proxy} ) {
- last PROXY_CHECK if $host !~ /localhost|127.0.0.1/;
- last PROXY_CHECK if $c->config->{ignore_frontend_proxy};
- }
- last PROXY_CHECK unless $ENV{HTTP_X_FORWARDED_HOST};
+ $base_path = $proxy->{path} . $base_path
+ if $proxy->{path};
- $host = $ENV{HTTP_X_FORWARDED_HOST};
-
- # backend could be on any port, so
- # assume frontend is on the default port
- $port = $c->request->secure ? 443 : 80;
- }
-
# set the request URI
my $path = $base_path . ( $ENV{PATH_INFO} || '' );
$path =~ s{^/+}{};
-
+ $path =~ s[/{2,}][/]g;
+
# Using URI directly is way too slow, so we construct the URLs manually
my $uri_class = "URI::$scheme";
Index: Catalyst-Engine-SCGI/lib/Catalyst/Engine/SCGI.pm
===================================================================
--- Catalyst-Engine-SCGI/lib/Catalyst/Engine/SCGI.pm (revision 6466)
+++ Catalyst-Engine-SCGI/lib/Catalyst/Engine/SCGI.pm (working copy)
@@ -105,22 +105,6 @@
$base_path = $ENV{SCRIPT_NAME} || '/';
}
- # If we are running as a backend proxy, get the true hostname
- PROXY_CHECK:
- {
- unless ( $c->config->{using_frontend_proxy} ) {
- last PROXY_CHECK if $host !~ /localhost|127.0.0.1/;
- last PROXY_CHECK if $c->config->{ignore_frontend_proxy};
- }
- last PROXY_CHECK unless $ENV{HTTP_X_FORWARDED_HOST};
-
- $host = $ENV{HTTP_X_FORWARDED_HOST};
-
- # backend could be on any port, so
- # assume frontend is on the default port
- $port = $c->request->secure ? 443 : 80;
- }
-
my $path = $base_path . ( $ENV{PATH_INFO} || '' );
$path =~ s{^/+}{};
More information about the Catalyst-dev
mailing list