Merge pull request #7946 from JimMoen/fix-exhook-api

fix exhook api

apps/emqx_exhook/src/emqx_exhook_api.erl

@@ -45,6 +45,10 @@
 -define(NOT_FOURD, 'NOT_FOUND').
 -define(BAD_REQUEST, 'BAD_REQUEST').
 -define(BAD_RPC, 'BAD_RPC').
+-define(ERR_BADARGS(REASON), begin
+    R0 = err_msg(REASON),
+    <<"Bad Arguments: ", R0/binary>>
+end).
 
 -dialyzer([
     {nowarn_function, [
@@ -246,6 +250,11 @@ exhooks(post, #{body := #{<<"name">> := Name} = Body}) ->
             {400, #{
                 code => <<"BAD_REQUEST">>,
                 message => <<"Already exists">>
+            }};
+        {error, Reason} ->
+            {400, #{
+                code => <<"BAD_REQUEST">>,
+                message => ?ERR_BADARGS(Reason)
             }}
     end.
 
@@ -265,10 +274,10 @@ action_with_name(put, #{bindings := #{name := Name}, body := Body}) ->
                 code => <<"NOT_FOUND">>,
                 message => <<"Server not found">>
             }};
-        {error, Error} ->
-            {500, #{
-                code => <<"BAD_RPC">>,
-                message => Error
+        {error, Reason} ->
+            {400, #{
+                code => <<"BAD_REQUEST">>,
+                message => ?ERR_BADARGS(Reason)
             }}
     end;
 action_with_name(delete, #{bindings := #{name := Name}}) ->
@@ -285,10 +294,10 @@ action_with_name(delete, #{bindings := #{name := Name}}) ->
                 code => <<"BAD_REQUEST">>,
                 message => <<"Server not found">>
             }};
-        {error, Error} ->
-            {500, #{
-                code => <<"BAD_RPC">>,
-                message => Error
+        {error, Reason} ->
+            {400, #{
+                code => <<"BAD_REQUEST">>,
+                message => ?ERR_BADARGS(Reason)
             }}
     end.
 
@@ -467,6 +476,26 @@ call_cluster(Fun) ->
 %%--------------------------------------------------------------------
 %% Internal Funcs
 %%--------------------------------------------------------------------
+err_msg({_, HoconErrors = [{Type, _} | _]}) when
+    Type == translation_error orelse Type == validation_error
+->
+    MessageFormat = [hocon_error(HoconError) || HoconError <- HoconErrors],
+    list_to_binary(MessageFormat);
+err_msg(Msg) ->
+    list_to_binary(io_lib:format("~0p", [Msg])).
+
+hocon_error({Type, Message0}) when
+    Type == translation_error orelse Type == validation_error
+->
+    case maps:get(reason, Message0, undefined) of
+        undefined ->
+            Message = maps:without([stacktrace], Message0),
+            emqx_logger_jsonfmt:best_effort_json(Message#{<<"type">> => Type}, []);
+        Reason when is_binary(Reason) ->
+            Reason;
+        Reason ->
+            list_to_binary(io_lib:format("~0p", [Reason]))
+    end.
 
 get_raw_config() ->
     RawConfig = emqx:get_raw_config([exhook, servers], []),

apps/emqx_exhook/src/emqx_exhook_schema.erl

@@ -55,7 +55,11 @@ fields(server) ->
         {name,
             sc(
                 binary(),
-                #{required => true, desc => ?DESC(name)}
+                #{
+                    required => true,
+                    validator => fun validate_name/1,
+                    desc => ?DESC(name)
+                }
             )},
         {enable,
             sc(
@@ -126,5 +130,23 @@ failed_action() ->
         }
     ).
 
+validate_name(Name) ->
+    NameRE = "^[A-Za-z0-9]+[A-Za-z0-9-_]*$",
+    NameLen = byte_size(Name),
+    case NameLen > 0 andalso NameLen =< 256 of
+        true ->
+            case re:run(Name, NameRE) of
+                {match, _} ->
+                    ok;
+                _Nomatch ->
+                    Reason = list_to_binary(
+                        io_lib:format("Bad ExHook Name ~p, expect ~p", [Name, NameRE])
+                    ),
+                    {error, Reason}
+            end;
+        false ->
+            {error, "Name Length must =< 256"}
+    end.
+
 server_config() ->
     fields(server).

apps/emqx_exhook/test/emqx_exhook_api_SUITE.erl

@@ -200,6 +200,23 @@ t_add_duplicate(Cfg) ->
 
     ?assertMatch([<<"default">>, <<"test1">>], emqx_exhook_mgr:running()).
 
+t_add_with_bad_name(Cfg) ->
+    Template = proplists:get_value(template, Cfg),
+    Instance = Template#{
+        name => <<"🤔">>,
+        url => "http://127.0.0.1:9001"
+    },
+
+    {error, _Reason} = request_api(
+        post,
+        api_path(["exhooks"]),
+        "",
+        auth_header_(),
+        Instance
+    ),
+
+    ?assertMatch([<<"default">>, <<"test1">>], emqx_exhook_mgr:running()).
+
 t_move_front(_) ->
     Result = request_api(
         post,