[Catalyst-dev] [PATCH] Performance issue: remote hostname lookup

[Marc Mims]

[Originally submitted to the catalyst list, in error.  Patch still
applies cleanly to the current 5.80 head.  I would have provided tests,
but I'm not sure how to go about testing this.  Suggestions? -Marc]

Catalyst::Engine::HTTP does a hostname lookup for each request in order
to populate $ENV{REMOTE_HOST}.  This causes a huge delay when the lookup
fails, especially when using C::P::Static::Simple where the lookup is
done for a bunch of css and javascript links.

The following patch against Catalyst-Runtime/5.80 removes the hostname
lookup from Engine::HTTP allowing it to be deferred until $req->hostname
is called (if ever).

I preserved the functionality from the Engine::HTTP lookup that
used 'localhost' as the hostname if the lookup fails.  I wonder,
however, if it might be better to use the host address, instead, i.e.,

    gethostbyaddr( inet_aton( $self->address ), AF_INET ) || $self->address;

Or perhaps it's best to just let $req->hostname return undef if the
hostname lookup fails, but either would change the behavior of the
existing code.

Note the scary comment in _socket_data: "This mess is necessary to keep
IE from crashing the server."  This patch works fine for me with an IE
client, but someone familiar with the IE crashing problem may want to
test it in the environment where such crashes occurred.

	-Marc

diff --git a/lib/Catalyst/Engine/CGI.pm b/lib/Catalyst/Engine/CGI.pm
index 7cd01dd..2c2fc87 100644
--- a/lib/Catalyst/Engine/CGI.pm
+++ b/lib/Catalyst/Engine/CGI.pm
@@ -70,7 +70,7 @@ sub prepare_connection {
         $request->address($ip);
     }
 
-    $request->hostname( $ENV{REMOTE_HOST} );
+    $request->hostname( $ENV{REMOTE_HOST} ) if exists $ENV{REMOTE_HOST};
     $request->protocol( $ENV{SERVER_PROTOCOL} );
     $request->user( $ENV{REMOTE_USER} );
     $request->method( $ENV{REQUEST_METHOD} );
diff --git a/lib/Catalyst/Engine/HTTP.pm b/lib/Catalyst/Engine/HTTP.pm
index 0dfc8e3..99ed939 100644
--- a/lib/Catalyst/Engine/HTTP.pm
+++ b/lib/Catalyst/Engine/HTTP.pm
@@ -357,7 +357,6 @@ sub _handler {
             PATH_INFO       => $path         || '',
             QUERY_STRING    => $query_string || '',
             REMOTE_ADDR     => $sockdata->{peeraddr},
-            REMOTE_HOST     => $sockdata->{peername},
             REQUEST_METHOD  => $method || '',
             SERVER_NAME     => $sockdata->{localname},
             SERVER_PORT     => $port,
@@ -508,9 +507,6 @@ sub _socket_data {
 
     # This mess is necessary to keep IE from crashing the server
     my $data = {
-        peername  => $iaddr 
-            ? ( gethostbyaddr( $iaddr, AF_INET ) || 'localhost' )
-            : 'localhost',
         peeraddr  => $iaddr 
             ? ( inet_ntoa($iaddr) || '127.0.0.1' )
             : '127.0.0.1',
diff --git a/lib/Catalyst/Request.pm b/lib/Catalyst/Request.pm
index c85ca09..fd9f0b9 100644
--- a/lib/Catalyst/Request.pm
+++ b/lib/Catalyst/Request.pm
@@ -113,7 +113,7 @@ has hostname => (
   lazy      => 1,
   default   => sub {
     my ($self) = @_;
-    gethostbyaddr( inet_aton( $self->address ), AF_INET )
+    gethostbyaddr( inet_aton( $self->address ), AF_INET ) || 'localhost'
   },
 );