OK, I prefer this. Patch the dns lookups into connection_string, this makes names come out correctly for channels, stomp, mqtt and means we can remove some special casing from mgmt. Remove the extra process. Add a mechanism to disable the whole thing in case people have broken configs.

3 files changed, 37 insertions, 25 deletions

diff --git a/ebin/rabbit_app.in b/ebin/rabbit_app.in
index 9b1ff8bd..0431ee8a 100644
--- a/ebin/rabbit_app.in
+++ b/ebin/rabbit_app.in
@@ -43,6 +43,7 @@
          {trace_vhosts, []},
          {log_levels, [{connection, info}]},
          {ssl_cert_login_from, distinguished_name},
+         {reverse_dns_lookups, true},
          {tcp_listen_options, [binary,
                                {packet,        raw},
                                {reuseaddr,     true},
diff --git a/src/rabbit_net.erl b/src/rabbit_net.erl
index 038154c3..23738d3e 100644
--- a/src/rabbit_net.erl
+++ b/src/rabbit_net.erl
@@ -20,7 +20,7 @@
 -export([is_ssl/1, ssl_info/1, controlling_process/2, getstat/2,
          recv/1, async_recv/3, port_command/2, getopts/2, setopts/2, send/2,
          close/1, fast_close/1, sockname/1, peername/1, peercert/1,
-         tune_buffer_size/1, connection_string/2]).
+         tune_buffer_size/1, connection_string/2, rdns/2]).
 
 %%---------------------------------------------------------------------------
 
@@ -72,6 +72,9 @@
 -spec(tune_buffer_size/1 :: (socket()) -> ok_or_any_error()).
 -spec(connection_string/2 ::
         (socket(), 'inbound' | 'outbound') -> ok_val_or_error(string())).
+-spec(rdns/2 ::
+        (socket(), 'inbound' | 'outbound') -> {string() | 'unknown',
+                                               string() | 'unknown'}).
 
 -endif.
 
@@ -193,17 +196,38 @@ tune_buffer_size(Sock) ->
     end.
 
 connection_string(Sock, Direction) ->
-    {From, To} = case Direction of
-                     inbound  -> {fun peername/1, fun sockname/1};
-                     outbound -> {fun sockname/1, fun peername/1}
-                 end,
+    {From, To} = sock_funs(Direction),
     case {From(Sock), To(Sock)} of
         {{ok, {FromAddress, FromPort}}, {ok, {ToAddress, ToPort}}} ->
-            {ok, rabbit_misc:format("~s:~p -> ~s:~p",
-                                    [rabbit_misc:ntoab(FromAddress), FromPort,
-                                     rabbit_misc:ntoab(ToAddress),   ToPort])};
+            {ok, rabbit_misc:format(
+                   "~s:~p -> ~s:~p",
+                   [maybe_rdns(FromAddress, Sock, From), FromPort,
+                    maybe_rdns(ToAddress, Sock, To),     ToPort])};
         {{error, _Reason} = Error, _} ->
             Error;
         {_, {error, _Reason} = Error} ->
             Error
     end.
+
+rdns(Sock, Direction) ->
+    {From, To} = sock_funs(Direction),
+    {rdns_lookup(Sock, From), rdns_lookup(Sock, To)}.
+
+maybe_rdns(Addr, Sock, Fun) ->
+    case rdns_lookup(Sock, Fun) of
+        unknown -> rabbit_misc:ntoab(Addr);
+        Host    -> Host
+    end.
+
+rdns_lookup(Sock, Fun) ->
+    {ok, Lookup} = application:get_env(rabbit, reverse_dns_lookups),
+    case Lookup of
+        true -> case Fun(Sock) of
+                    {ok, {IP, _Port}} -> rabbit_networking:tcp_host(IP);
+                    _                 -> unknown
+                end;
+        _    -> unknown
+    end.
+
+sock_funs(inbound)  -> {fun peername/1, fun sockname/1};
+sock_funs(outbound) -> {fun sockname/1, fun peername/1}.
diff --git a/src/rabbit_reader.erl b/src/rabbit_reader.erl
index 3cffd1ae..c3bd77ab 100644
--- a/src/rabbit_reader.erl
+++ b/src/rabbit_reader.erl
@@ -198,8 +198,8 @@ start_connection(Parent, ChannelSupSupPid, Collector, StartHeartbeatFun, Deb,
     ConnStr = name(Sock),
     log(info, "accepting AMQP connection ~p (~s)~n", [self(), ConnStr]),
     ClientSock = socket_op(Sock, SockTransform),
-    erlang:send_after(?HANDSHAKE_TIMEOUT * 1000, self(),
-                      handshake_timeout),
+    erlang:send_after(?HANDSHAKE_TIMEOUT * 1000, self(), handshake_timeout),
+    {Host, PeerHost} = rabbit_net:rdns(Sock, inbound),
     State = #v1{parent              = Parent,
                 sock                = ClientSock,
                 connection          = #connection{
@@ -225,9 +225,8 @@ start_connection(Parent, ChannelSupSupPid, Collector, StartHeartbeatFun, Deb,
                 conserve_resources  = false,
                 last_blocked_by     = none,
                 last_blocked_at     = never,
-                peer_host           = unknown},
-    Self = self(),
-    spawn_link(fun() -> do_reverse_dns(ClientSock, Self) end),
+                host                = Host,
+                peer_host           = PeerHost},
     try
         ok = inet_op(fun () -> rabbit_net:tune_buffer_size(ClientSock) end),
         recvloop(Deb, switch_callback(rabbit_event:init_stats_timer(
@@ -253,18 +252,6 @@ start_connection(Parent, ChannelSupSupPid, Collector, StartHeartbeatFun, Deb,
     end,
     done.
 
-do_reverse_dns(Sock, Reader) ->
-    Host = do_reverse_dns0(Sock, fun rabbit_net:sockname/1),
-    PeerHost = do_reverse_dns0(Sock, fun rabbit_net:peername/1),
-    unlink(Reader),
-    Reader ! {rdns, Host, PeerHost}.
-
-do_reverse_dns0(Sock, Fun) ->
-    case Fun(Sock) of
-        {ok, {IP, _Port}} -> rabbit_networking:tcp_host(IP);
-        _                 -> undefined
-    end.
-
 recvloop(Deb, State = #v1{pending_recv = true}) ->
     mainloop(Deb, State);
 recvloop(Deb, State = #v1{connection_state = blocked}) ->