Merge pull request #3202 from rabbitmq/validate-amqp0-9-1-queue-name-length-in-management-ui

Validate amqp0.9.1 queue name length in management UI

(cherry picked from commit 4ba8d05)

Author: michaelklishin

.gitignore

@@ -57,6 +57,7 @@
 /plugins.lock
 /sbin/
 /sbin.lock
+erl_crash.dump
 .envrc
 *.plt
 *.lock

deps/rabbit/src/rabbit_parameter_validation.erl

@@ -7,7 +7,8 @@
 
 -module(rabbit_parameter_validation).
 
--export([number/2, integer/2, binary/2, boolean/2, list/2, regex/2, proplist/3, enum/1]).
+-export([number/2, integer/2, binary/2, amqp091_queue_name/2,
+        boolean/2, list/2, regex/2, proplist/3, enum/1]).
 
 number(_Name, Term) when is_number(Term) ->
     ok;
@@ -27,6 +28,16 @@ binary(_Name, Term) when is_binary(Term) ->
 binary(Name, Term) ->
     {error, "~s should be binary, actually was ~p", [Name, Term]}.
 
+amqp091_queue_name(Name, S) when is_binary(S) ->
+    case size(S) of
+        Len when Len =< 255 -> ok;
+        _                   -> {error, "~s should be less than 255 bytes, actually was ~p", [Name, size(S)]}
+    end;
+
+amqp091_queue_name(Name, Term) ->
+    {error, "~s should be binary, actually was ~p", [Name, Term]}.
+
+
 boolean(_Name, Term) when is_boolean(Term) ->
     ok;
 boolean(Name, Term) ->

deps/rabbitmq_management/src/rabbit_mgmt_wm_queue.erl

@@ -60,10 +60,16 @@ to_json(ReqData, Context) ->
 
 accept_content(ReqData, Context) ->
     Name = rabbit_mgmt_util:id(queue, ReqData),
-    rabbit_mgmt_util:direct_request(
-      'queue.declare',
-      fun rabbit_mgmt_format:format_accept_content/1,
-      [{queue, Name}], "Declare queue error: ~s", ReqData, Context).
+    %% NOTE: ?FRAMING currently defined as 0.9.1 hence validating length
+    case rabbit_parameter_validation:amqp091_queue_name(queue, Name) of
+        ok ->
+            rabbit_mgmt_util:direct_request(
+            'queue.declare',
+            fun rabbit_mgmt_format:format_accept_content/1,
+            [{queue, Name}], "Declare queue error: ~s", ReqData, Context);
+        {error, F, A} ->
+            rabbit_mgmt_util:bad_request(iolist_to_binary(io_lib:format(F ++ "~n", A)), ReqData, Context)
+    end.
 
 delete_resource(ReqData, Context) ->
     %% We need to retrieve manually if-unused and if-empty, as the HTTP API uses '-'

deps/rabbitmq_shovel/src/rabbit_shovel_parameters.erl

@@ -152,7 +152,7 @@ amqp091_dest_validation(_Def, User) ->
     [{<<"dest-uri">>,        validate_uri_fun(User), mandatory},
      {<<"dest-exchange">>,   fun rabbit_parameter_validation:binary/2,optional},
      {<<"dest-exchange-key">>,fun rabbit_parameter_validation:binary/2,optional},
-     {<<"dest-queue">>,      fun rabbit_parameter_validation:binary/2,optional},
+     {<<"dest-queue">>,      fun rabbit_parameter_validation:amqp091_queue_name/2,optional},
      {<<"dest-queue-args">>, fun validate_queue_args/2, optional},
      {<<"add-forward-headers">>, fun rabbit_parameter_validation:boolean/2,optional},
      {<<"add-timestamp-header">>, fun rabbit_parameter_validation:boolean/2,optional},

deps/rabbitmq_shovel_management/test/http_SUITE.erl

@@ -166,6 +166,27 @@ shovels(Config) ->
                            'dest-queue' => <<"test2">>}}, ?CREATED)
      || V <- ["%2f", "v"]],
 
+     [http_put(Config, "/parameters/shovel/" ++ V ++ "/my-dynamic",
+              #{value => #{'src-protocol' => <<"amqp091">>,
+                           'src-uri'    => <<"amqp://">>,
+                           'src-queue'  => <<"test">>,
+                           'dest-protocol' => <<"amqp091">>,
+                           'dest-uri'   => <<"amqp://">>,
+                           'dest-queue' => list_to_binary(
+                                           "test2qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq"
+                                           "test2qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq"
+                                           "test2qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq"
+                                           "test2qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq"
+                                           "test2qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq"
+                                           "test2qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq"
+                                           "test2qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq"
+                                           "test2qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq"
+                                           "test2qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq"
+                                           "test2qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq"
+                                           "test2qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq")}},
+               ?BAD_REQUEST)
+     || V <- ["%2f", "v"]],
+
     ?assertMatch([?StaticPattern, ?Dynamic1Pattern, ?Dynamic2Pattern],
 		 http_get(Config, "/shovels",     "guest", "guest", ?OK)),
     ?assertMatch([?Dynamic1Pattern],