From ef5687d465bd52253b5cdf413d88b5d4b47ac9c1 Mon Sep 17 00:00:00 2001 From: Stefan Strigler Date: Tue, 25 Oct 2022 16:24:20 +0200 Subject: [PATCH 1/2] fix(emqx_gateway_api): don't crash on unknown status --- .../emqx_dashboard/src/emqx_dashboard.app.src | 2 +- .../src/emqx_dashboard_swagger.erl | 16 ++++++++++- .../test/emqx_swagger_parameter_SUITE.erl | 16 ++++++++++- apps/emqx_gateway/src/emqx_gateway.app.src | 2 +- apps/emqx_gateway/src/emqx_gateway_api.erl | 28 +++++++++++++------ .../test/emqx_gateway_api_SUITE.erl | 12 ++++++-- changes/v5.0.10-en.md | 2 ++ changes/v5.0.10-zh.md | 2 ++ 8 files changed, 66 insertions(+), 14 deletions(-) diff --git a/apps/emqx_dashboard/src/emqx_dashboard.app.src b/apps/emqx_dashboard/src/emqx_dashboard.app.src index 9d5f85c7b..fb290c551 100644 --- a/apps/emqx_dashboard/src/emqx_dashboard.app.src +++ b/apps/emqx_dashboard/src/emqx_dashboard.app.src @@ -2,7 +2,7 @@ {application, emqx_dashboard, [ {description, "EMQX Web Dashboard"}, % strict semver, bump manually! - {vsn, "5.0.6"}, + {vsn, "5.0.7"}, {modules, []}, {registered, [emqx_dashboard_sup]}, {applications, [kernel, stdlib, mnesia, minirest, emqx]}, diff --git a/apps/emqx_dashboard/src/emqx_dashboard_swagger.erl b/apps/emqx_dashboard/src/emqx_dashboard_swagger.erl index 5674d273c..3904803ac 100644 --- a/apps/emqx_dashboard/src/emqx_dashboard_swagger.erl +++ b/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}. diff --git a/apps/emqx_dashboard/test/emqx_swagger_parameter_SUITE.erl b/apps/emqx_dashboard/test/emqx_swagger_parameter_SUITE.erl index 912459b9d..5dd76acf6 100644 --- a/apps/emqx_dashboard/test/emqx_swagger_parameter_SUITE.erl +++ b/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") -> diff --git a/apps/emqx_gateway/src/emqx_gateway.app.src b/apps/emqx_gateway/src/emqx_gateway.app.src index 98dbd6909..491d0242a 100644 --- a/apps/emqx_gateway/src/emqx_gateway.app.src +++ b/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]}, diff --git a/apps/emqx_gateway/src/emqx_gateway_api.erl b/apps/emqx_gateway/src/emqx_gateway_api.erl index 00d3d4b17..6ae1135b0 100644 --- a/apps/emqx_gateway/src/emqx_gateway_api.erl +++ b/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 = - case maps:get(<<"status">>, Params, undefined) of - undefined -> all; - S0 -> binary_to_existing_atom(S0, utf8) - end, - {200, emqx_gateway_http:gateways(Status)}; + Status = maps:get(<<"status">>, Params, <<"all">>), + case lists:member(Status, [<<"all">> | ?KNOWN_GATEWAY_STATUSES]) of + true -> + {200, emqx_gateway_http:gateways(binary_to_existing_atom(Status, utf8))}; + false -> + 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">> } )} ]. diff --git a/apps/emqx_gateway/test/emqx_gateway_api_SUITE.erl b/apps/emqx_gateway/test/emqx_gateway_api_SUITE.erl index 8532a3a74..29c4fefe7 100644 --- a/apps/emqx_gateway/test/emqx_gateway_api_SUITE.erl +++ b/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"), - assert_bad_request(BadReq), + {200, UnloadedGateways} = request(get, "/gateways?status=unloaded"), + 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( diff --git a/changes/v5.0.10-en.md b/changes/v5.0.10-en.md index c70ec9977..943ebadba 100644 --- a/changes/v5.0.10-en.md +++ b/changes/v5.0.10-en.md @@ -11,3 +11,5 @@ ## Bug fixes - 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 [#7794](https://github.com/emqx/emqx/pull/9225). diff --git a/changes/v5.0.10-zh.md b/changes/v5.0.10-zh.md index 26b2737dd..8411a2fb8 100644 --- a/changes/v5.0.10-zh.md +++ b/changes/v5.0.10-zh.md @@ -11,3 +11,5 @@ ## Bug fixes - 优化认认证配置中 `mechanism` 字段缺失情况下的错误日志 [#8924](https://github.com/emqx/emqx/pull/8924)。 + +- 修复未知 `status` 参数导致 `/gateway` API 发生 HTTP 500 错误的问题 [#7794](https://github.com/emqx/emqx/pull/9225) [#7794](https://github.com/emqx/emqx/pull/9225). From 2d548acabd9d783bfc55de84f2c0a4edf43badda Mon Sep 17 00:00:00 2001 From: Stefan Strigler Date: Fri, 28 Oct 2022 08:30:44 +0200 Subject: [PATCH 2/2] style: fix newline --- changes/v5.0.10-en.md | 2 +- changes/v5.0.10-zh.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/changes/v5.0.10-en.md b/changes/v5.0.10-en.md index 8b25e9418..4bd1ffb6e 100644 --- a/changes/v5.0.10-en.md +++ b/changes/v5.0.10-en.md @@ -23,4 +23,4 @@ - 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. \ No newline at end of file + Before the fix, it always returned `200` even if the EMQX application was not running. Now it returns `503` in that case. diff --git a/changes/v5.0.10-zh.md b/changes/v5.0.10-zh.md index 77e285907..fe9511655 100644 --- a/changes/v5.0.10-zh.md +++ b/changes/v5.0.10-zh.md @@ -22,4 +22,4 @@ - 修复未知 `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`。 \ No newline at end of file + 在此修复前，它总是返回 HTTP 状态码 `200`，即使 EMQX 没有完成启动或正在重启。 现在它在这些情况下会返回状态码 `503`。