improved connection refusal logic / error messagebug25383

plus some tests to go with that

And a tweak to the "become 1.0" API

2 files changed, 39 insertions, 20 deletions

diff --git a/src/rabbit_reader.erl b/src/rabbit_reader.erl
index ae832749..30ea6a5b 100644
--- a/src/rabbit_reader.erl
+++ b/src/rabbit_reader.erl
@@ -699,13 +699,8 @@ handle_input(handshake, <<"AMQP", 1, 1, 9, 1>>, State) ->
     start_connection({8, 0, 0}, rabbit_framing_amqp_0_8, State);
 
 %% ... and finally, the 1.0 spec is crystal clear!  Note that the
-%% TLS uses a different protocol number, and would go here.
-handle_input(handshake, <<"AMQP", 0, 1, 0, 0>>, State) ->
-    become_1_0(amqp, {0, 1, 0, 0}, State);
-
-%% 3 stands for "SASL"
-handle_input(handshake, <<"AMQP", 3, 1, 0, 0>>, State) ->
-    become_1_0(sasl, {3, 1, 0, 0}, State);
+handle_input(handshake, <<"AMQP", Id, 1, 0, 0>>, State) ->
+    become_1_0(Id, State);
 
 handle_input(handshake, <<"AMQP", A, B, C, D>>, #v1{sock = Sock}) ->
     refuse_connection(Sock, {bad_version, {A, B, C, D}});
@@ -736,10 +731,13 @@ start_connection({ProtocolMajor, ProtocolMinor, _ProtocolRevision},
                              connection_state = starting},
                     frame_header, 7).
 
-refuse_connection(Sock, Exception) ->
-    ok = inet_op(fun () -> rabbit_net:send(Sock, <<"AMQP",0,0,9,1>>) end),
+refuse_connection(Sock, Exception, {A, B, C, D}) ->
+    ok = inet_op(fun () -> rabbit_net:send(Sock, <<"AMQP",A,B,C,D>>) end),
     throw(Exception).
 
+refuse_connection(Sock, Exception) ->
+    refuse_connection(Sock, Exception, {0, 0, 9, 1}).
+
 ensure_stats_timer(State = #v1{connection_state = running}) ->
     rabbit_event:ensure_stats_timer(State, #v1.stats_timer, emit_stats);
 ensure_stats_timer(State) ->
@@ -1008,15 +1006,19 @@ emit_stats(State) ->
 
 %% 1.0 stub
 -ifdef(use_specs).
--spec(become_1_0/3 :: ('amqp' | 'sasl',
-                       {non_neg_integer(), non_neg_integer(),
-                        non_neg_integer(), non_neg_integer()},
-                       #v1{}) -> no_return()).
+-spec(become_1_0/2 :: (non_neg_integer(), #v1{}) -> no_return()).
 -endif.
-become_1_0(Mode, Version, State = #v1{sock = Sock}) ->
+become_1_0(Id, State = #v1{sock = Sock}) ->
     case code:is_loaded(rabbit_amqp1_0_reader) of
-        false -> refuse_connection(Sock, {bad_version, Version});
-        _     -> throw({become, {rabbit_amqp1_0_reader, become,
+        false -> refuse_connection(Sock, amqp1_0_plugin_not_enabled);
+        _     -> Mode = case Id of
+                            0 -> amqp;
+                            2 -> sasl;
+                            _ -> refuse_connection(
+                                   Sock, {unsupported_amqp1_0_protocol_id, Id},
+                                   {2, 1, 0, 0})
+                        end,
+                 throw({become, {rabbit_amqp1_0_reader, init,
                                  [Mode, pack_for_1_0(State)]}})
     end.
 
diff --git a/src/rabbit_tests.erl b/src/rabbit_tests.erl
index b0ff5af9..a2442f17 100644
--- a/src/rabbit_tests.erl
+++ b/src/rabbit_tests.erl
@@ -61,6 +61,7 @@ all_tests() ->
     passed = test_runtime_parameters(),
     passed = test_policy_validation(),
     passed = test_server_status(),
+    passed = test_amqp_connection_refusal(),
     passed = test_confirms(),
     passed =
         do_if_secondary_node(
@@ -1119,10 +1120,7 @@ test_server_status() ->
             rabbit_misc:r(<<"/">>, queue, <<"foo">>)),
 
     %% list connections
-    [#listener{host = H, port = P} | _] =
-        [L || L = #listener{node = N} <- rabbit_networking:active_listeners(),
-              N =:= node()],
-
+    {H, P} = find_listener(),
     {ok, C} = gen_tcp:connect(H, P, []),
     gen_tcp:send(C, <<"AMQP", 0, 0, 9, 1>>),
     timer:sleep(100),
@@ -1161,6 +1159,25 @@ test_server_status() ->
 
     passed.
 
+test_amqp_connection_refusal() ->
+    [passed = test_amqp_connection_refusal(V) ||
+        V <- [<<"AMQP",9,9,9,9>>, <<"AMQP",0,1,0,0>>, <<"XXXX",0,0,9,1>>]],
+    passed.
+
+test_amqp_connection_refusal(Header) ->
+    {H, P} = find_listener(),
+    {ok, C} = gen_tcp:connect(H, P, [binary, {active, false}]),
+    ok = gen_tcp:send(C, Header),
+    {ok, <<"AMQP",0,0,9,1>>} = gen_tcp:recv(C, 8, 100),
+    ok = gen_tcp:close(C),
+    passed.
+
+find_listener() ->
+    [#listener{host = H, port = P} | _] =
+        [L || L = #listener{node = N} <- rabbit_networking:active_listeners(),
+              N =:= node()],
+    {H, P}.
+
 test_writer() -> test_writer(none).
 
 test_writer(Pid) ->