Correct parse tags provided as a list

Discovered while testing a PR for rabbit-hole

2 files changed, 38 insertions, 13 deletions

diff --git a/deps/rabbit/src/rabbit_vhost.erl b/deps/rabbit/src/rabbit_vhost.erl
index 76aabd0fc8..0b8acff186 100644
--- a/deps/rabbit/src/rabbit_vhost.erl
+++ b/deps/rabbit/src/rabbit_vhost.erl
@@ -66,10 +66,20 @@ parse_tags("") ->
     [];
 parse_tags(<<"">>) ->
     [];
+parse_tags([]) ->
+    [];
 parse_tags(Val) when is_binary(Val) ->
-    parse_tags(rabbit_data_coercion:to_list(Val));
+    SVal = rabbit_data_coercion:to_list(Val),
+    [trim_tag(Tag) || Tag <- re:split(SVal, ",", [{return, list}])];
 parse_tags(Val) when is_list(Val) ->
-    [trim_tag(Tag) || Tag <- re:split(Val, ",", [{return, list}])].
+    case hd(Val) of
+      Bin when is_binary(Bin) ->
+        %% this is a list of binaries
+           [trim_tag(Tag) || Tag <- Val];
+      Int when is_integer(Int) ->
+          %% this is a string/charlist
+          [trim_tag(Tag) || Tag <- re:split(Val, ",", [{return, list}])]
+    end.
 
 -spec add(vhost:name(), rabbit_types:username()) -> rabbit_types:ok_or_error(any()).
 
@@ -92,7 +102,7 @@ do_add(Name, Description, Tags, ActingUser) ->
         undefined ->
             rabbit_log:info("Adding vhost '~s' without a description", [Name]);
         Value ->
-            rabbit_log:info("Adding vhost '~s' (description: '~s')", [Name, Value])
+            rabbit_log:info("Adding vhost '~s' (description: '~s', tags: ~p)", [Name, Value, Tags])
     end,
     VHost = rabbit_misc:execute_mnesia_transaction(
           fun () ->
@@ -178,15 +188,18 @@ put_vhost(Name, Description, Tags0, Trace, Username) ->
     end,
     Result = case exists(Name) of
         true  -> ok;
-        false -> add(Name, Description, parse_tags(Tags), Username),
-                 %% wait for up to 45 seconds for the vhost to initialise
-                 %% on all nodes
-                 case await_running_on_all_nodes(Name, 45000) of
-                     ok               ->
-                         maybe_grant_full_permissions(Name, Username);
-                     {error, timeout} ->
-                         {error, timeout}
-                 end
+        false ->
+            ParsedTags = parse_tags(Tags),
+            rabbit_log:debug("Parsed tags ~p to ~p", [Tags, ParsedTags]),
+            add(Name, Description, ParsedTags, Username),
+             %% wait for up to 45 seconds for the vhost to initialise
+             %% on all nodes
+             case await_running_on_all_nodes(Name, 45000) of
+                 ok               ->
+                     maybe_grant_full_permissions(Name, Username);
+                 {error, timeout} ->
+                     {error, timeout}
+             end
     end,
     case Trace of
         true      -> rabbit_trace:start(Name);
diff --git a/deps/rabbit/test/vhost_SUITE.erl b/deps/rabbit/test/vhost_SUITE.erl
index 39a56d1327..a86d90f50a 100644
--- a/deps/rabbit/test/vhost_SUITE.erl
+++ b/deps/rabbit/test/vhost_SUITE.erl
@@ -25,7 +25,8 @@ groups() ->
     ClusterSize1Tests = [
         single_node_vhost_deletion_forces_connection_closure,
         vhost_failure_forces_connection_closure,
-        vhost_creation_idempotency
+        vhost_creation_idempotency,
+        parse_tags
     ],
     ClusterSize2Tests = [
         cluster_vhost_deletion_forces_connection_closure,
@@ -318,6 +319,17 @@ vhost_creation_idempotency(Config) ->
         rabbit_ct_broker_helpers:delete_vhost(Config, VHost)
     end.
 
+parse_tags(Config) ->
+    rabbit_ct_broker_helpers:rpc(Config, 0, ?MODULE, parse_tags1, [Config]).
+
+parse_tags1(_Config) ->
+    ?assertEqual([], rabbit_vhost:parse_tags(<<"">>)),
+    ?assertEqual([], rabbit_vhost:parse_tags("")),
+    ?assertEqual([], rabbit_vhost:parse_tags([])),
+    ?assertEqual([a, b], rabbit_vhost:parse_tags(<<"a,b">>)),
+    ?assertEqual([a3, b3], rabbit_vhost:parse_tags("a3,b3")),
+    ?assertEqual([tag1, tag2], rabbit_vhost:parse_tags([<<"tag1">>, <<"tag2">>])).
+
 %% -------------------------------------------------------------------
 %% Helpers
 %% -------------------------------------------------------------------