Merge pull request #9258 from sstrigler/master

Re-UP fix(emqx_gateway_api): don't crash on unknown status

apps/emqx_dashboard/src/emqx_dashboard_swagger.erl

@@ -367,11 +367,13 @@ parameters(Params, Module) ->
                         Required = hocon_schema:field_schema(Type, required),
                         Default = hocon_schema:field_schema(Type, default),
                         HoconType = hocon_schema:field_schema(Type, type),
+                        SchemaExtras = hocon_extract_map([enum, default], Type),
                         Meta = init_meta(Default),
                         {ParamType, Refs} = hocon_schema_to_spec(HoconType, Module),
+                        Schema = maps:merge(maps:merge(ParamType, Meta), SchemaExtras),
                         Spec0 = init_prop(
                             [required | ?DEFAULT_FIELDS],
-                            #{schema => maps:merge(ParamType, Meta), name => Name, in => In},
+                            #{schema => Schema, name => Name, in => In},
                             Type
                         ),
                         Spec1 = trans_required(Spec0, Required, In),
@@ -384,6 +386,18 @@ parameters(Params, Module) ->
         ),
     {lists:reverse(SpecList), AllRefs}.
 
+hocon_extract_map(Keys, Type) ->
+    lists:foldl(
+        fun(K, M) ->
+            case hocon_schema:field_schema(Type, K) of
+                undefined -> M;
+                V -> M#{K => V}
+            end
+        end,
+        #{},
+        Keys
+    ).
+
 init_meta(undefined) -> #{};
 init_meta(Default) -> #{default => Default}.
 

apps/emqx_dashboard/test/emqx_swagger_parameter_SUITE.erl

@@ -6,7 +6,7 @@
 -export([paths/0, api_spec/0, schema/1, fields/1]).
 -export([init_per_suite/1, end_per_suite/1]).
 -export([t_in_path/1, t_in_query/1, t_in_mix/1, t_without_in/1, t_ref/1, t_public_ref/1]).
--export([t_require/1, t_nullable/1, t_method/1, t_api_spec/1]).
+-export([t_require/1, t_query_enum/1, t_nullable/1, t_method/1, t_api_spec/1]).
 -export([t_in_path_trans/1, t_in_query_trans/1, t_in_mix_trans/1, t_ref_trans/1]).
 -export([t_in_path_trans_error/1, t_in_query_trans_error/1, t_in_mix_trans_error/1]).
 -export([all/0, suite/0, groups/0]).
@@ -30,6 +30,7 @@ groups() ->
             t_in_mix,
             t_without_in,
             t_require,
+            t_query_enum,
             t_nullable,
             t_method,
             t_public_ref
@@ -226,6 +227,17 @@ t_require(_Config) ->
     validate("/required/false", ExpectSpec),
     ok.
 
+t_query_enum(_Config) ->
+    ExpectSpec = [
+        #{
+            in => query,
+            name => userid,
+            schema => #{type => string, enum => [<<"a">>], default => <<"a">>}
+        }
+    ],
+    validate("/query/enum", ExpectSpec),
+    ok.
+
 t_nullable(_Config) ->
     NullableFalse = [
         #{
@@ -528,6 +540,8 @@ schema("/test/without/in") ->
     };
 schema("/required/false") ->
     to_schema([{'userid', mk(binary(), #{in => query, required => false})}]);
+schema("/query/enum") ->
+    to_schema([{'userid', mk(binary(), #{in => query, enum => [<<"a">>], default => <<"a">>})}]);
 schema("/nullable/false") ->
     to_schema([{'userid', mk(binary(), #{in => query, required => true})}]);
 schema("/nullable/true") ->

apps/emqx_gateway/src/emqx_gateway.app.src

@@ -1,7 +1,7 @@
 %% -*- mode: erlang -*-
 {application, emqx_gateway, [
     {description, "The Gateway management application"},
-    {vsn, "0.1.6"},
+    {vsn, "0.1.7"},
     {registered, []},
     {mod, {emqx_gateway_app, []}},
     {applications, [kernel, stdlib, grpc, emqx, emqx_authn]},

apps/emqx_gateway/src/emqx_gateway_api.erl

@@ -53,6 +53,8 @@
     gateway_insta/2
 ]).
 
+-define(KNOWN_GATEWAY_STATUSES, [<<"running">>, <<"stopped">>, <<"unloaded">>]).
+
 %%--------------------------------------------------------------------
 %% minirest behaviour callbacks
 %%--------------------------------------------------------------------
@@ -71,12 +73,22 @@ paths() ->
 
 gateway(get, Request) ->
     Params = maps:get(query_string, Request, #{}),
-    Status =
+    Status = maps:get(<<"status">>, Params, <<"all">>),
-        case maps:get(<<"status">>, Params, undefined) of
+    case lists:member(Status, [<<"all">> | ?KNOWN_GATEWAY_STATUSES]) of
-            undefined -> all;
+        true ->
-            S0 -> binary_to_existing_atom(S0, utf8)
+            {200, emqx_gateway_http:gateways(binary_to_existing_atom(Status, utf8))};
-        end,
+        false ->
-    {200, emqx_gateway_http:gateways(Status)};
+            return_http_error(
+                400,
+                [
+                    "Unknown gateway status in query: ",
+                    Status,
+                    "\n",
+                    "Values allowed: ",
+                    lists:join(", ", ?KNOWN_GATEWAY_STATUSES)
+                ]
+            )
+    end;
 gateway(post, Request) ->
     Body = maps:get(body, Request, #{}),
     try
@@ -241,16 +253,16 @@ params_gateway_name_in_path() ->
     ].
 
 params_gateway_status_in_qs() ->
-    %% FIXME: enum in swagger ??
     [
         {status,
             mk(
                 binary(),
                 #{
                     in => query,
+                    enum => ?KNOWN_GATEWAY_STATUSES,
                     required => false,
                     desc => ?DESC(gateway_status_in_qs),
-                    example => <<"">>
+                    example => <<"running">>
                 }
             )}
     ].

apps/emqx_gateway/test/emqx_gateway_api_SUITE.erl

@@ -62,8 +62,16 @@ end_per_suite(Conf) ->
 t_gateway(_) ->
     {200, Gateways} = request(get, "/gateways"),
     lists:foreach(fun assert_gw_unloaded/1, Gateways),
-    {400, BadReq} = request(get, "/gateways/uname_gateway"),
+    {200, UnloadedGateways} = request(get, "/gateways?status=unloaded"),
-    assert_bad_request(BadReq),
+    lists:foreach(fun assert_gw_unloaded/1, UnloadedGateways),
+    {200, NoRunningGateways} = request(get, "/gateways?status=running"),
+    ?assertEqual([], NoRunningGateways),
+    {400, BadReqUnknownGw} = request(get, "/gateways/unknown_gateway"),
+    assert_bad_request(BadReqUnknownGw),
+    {400, BadReqInvalidStatus} = request(get, "/gateways?status=invalid_status"),
+    assert_bad_request(BadReqInvalidStatus),
+    {400, BadReqUCStatus} = request(get, "/gateways?status=UNLOADED"),
+    assert_bad_request(BadReqUCStatus),
     {201, _} = request(post, "/gateways", #{name => <<"stomp">>}),
     {200, StompGw1} = request(get, "/gateways/stomp"),
     assert_feilds_apperence(

changes/v5.0.10-en.md

@@ -20,6 +20,8 @@
 
 - Fix error log message when `mechanism` is missing in authentication config [#8924](https://github.com/emqx/emqx/pull/8924).
 
+- Fix HTTP 500 issue when unknown `status` parameter is used in `/gateway` API call [#9225](https://github.com/emqx/emqx/pull/9225).
+
 - Fixed the HTTP response status code for the `/status` endpoint [#9211](https://github.com/emqx/emqx/pull/9211).
   Before the fix, it always returned `200` even if the EMQX application was not running. Now it returns `503` in that case.
 

changes/v5.0.10-zh.md

@@ -19,6 +19,8 @@
 
 - 优化认认证配置中 `mechanism` 字段缺失情况下的错误日志 [#8924](https://github.com/emqx/emqx/pull/8924)。
 
+- 修复未知 `status` 参数导致 `/gateway` API 发生 HTTP 500 错误的问题 [#9225](https://github.com/emqx/emqx/pull/9225)。
+
 - 修正了 `/status` 端点的响应状态代码 [#9211](https://github.com/emqx/emqx/pull/9211)。
   在此修复前，它总是返回 HTTP 状态码 `200`，即使 EMQX 没有完成启动或正在重启。 现在它在这些情况下会返回状态码 `503`。