From 966b2affc2dbdd83184174cbe6282b39b8f1d470 Mon Sep 17 00:00:00 2001 From: JianBo He Date: Thu, 6 Jul 2023 10:53:07 +0800 Subject: [PATCH 1/2] chore: allow handle */* or multiple values in Accept headers --- .../src/emqx_management.app.src | 2 +- .../src/emqx_mgmt_api_configs.erl | 29 +++++++++++++++++-- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/apps/emqx_management/src/emqx_management.app.src b/apps/emqx_management/src/emqx_management.app.src index 0e2c2646e..d6286f454 100644 --- a/apps/emqx_management/src/emqx_management.app.src +++ b/apps/emqx_management/src/emqx_management.app.src @@ -2,7 +2,7 @@ {application, emqx_management, [ {description, "EMQX Management API and CLI"}, % strict semver, bump manually! - {vsn, "5.0.25"}, + {vsn, "5.0.26"}, {modules, []}, {registered, [emqx_management_sup]}, {applications, [kernel, stdlib, emqx_plugins, minirest, emqx, emqx_ctl]}, diff --git a/apps/emqx_management/src/emqx_mgmt_api_configs.erl b/apps/emqx_management/src/emqx_mgmt_api_configs.erl index 6991bb11c..80818b41a 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_configs.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_configs.erl @@ -337,9 +337,10 @@ config_reset(post, _Params, Req) -> configs(get, #{query_string := QueryStr, headers := Headers}, _Req) -> %% Should deprecated json v1 since 5.2.0 - case maps:get(<<"accept">>, Headers, <<"text/plain">>) of - <<"application/json">> -> get_configs_v1(QueryStr); - <<"text/plain">> -> get_configs_v2(QueryStr) + case find_suitable_accept(Headers, [<<"text/plain">>, <<"application/json">>]) of + {ok, <<"application/json">>} -> get_configs_v1(QueryStr); + {ok, <<"text/plain">>} -> get_configs_v2(QueryStr); + {error, _} = Error -> {400, #{code => 'INVALID_ACCEPT', message => ?ERR_MSG(Error)}} end; configs(put, #{body := Conf, query_string := #{<<"mode">> := Mode}}, _Req) -> case emqx_conf_cli:load_config(Conf, Mode) of @@ -348,6 +349,28 @@ configs(put, #{body := Conf, query_string := #{<<"mode">> := Mode}}, _Req) -> {error, Errors} -> {400, #{code => 'UPDATE_FAILED', message => ?ERR_MSG(Errors)}} end. +find_suitable_accept(Headers, Perferences) -> + AcceptVal = maps:get(<<"accept">>, Headers, <<"*/*">>), + %% Multiple types, weighted with the quality value syntax: + %% Accept: text/html, application/xhtml+xml, application/xml;q=0.9, image/webp, */*;q=0.8 + Accepts = lists:map( + fun(S) -> + [T | _] = binary:split(string:trim(S), <<";">>), + T + end, + re:split(AcceptVal, ",") + ), + case lists:member(<<"*/*">>, Accepts) of + true -> + {ok, lists:first(Perferences)}; + fales -> + Found = lists:filter(fun(Accept) -> lists:member(Accept, Accepts) end, Perferences), + case Found of + [] -> {error, no_suitalbe_accept}; + _ -> lists:first(Found) + end + end. + get_configs_v1(QueryStr) -> Node = maps:get(<<"node">>, QueryStr, node()), case From 71d1f805300c5200e38f36fb7697e32210c79148 Mon Sep 17 00:00:00 2001 From: JianBo He Date: Thu, 6 Jul 2023 11:30:38 +0800 Subject: [PATCH 2/2] test(api): cover the accept logic for `/configs` API --- .../src/emqx_mgmt_api_configs.erl | 9 +++--- .../test/emqx_mgmt_api_configs_SUITE.erl | 30 +++++++++++++++++++ 2 files changed, 35 insertions(+), 4 deletions(-) diff --git a/apps/emqx_management/src/emqx_mgmt_api_configs.erl b/apps/emqx_management/src/emqx_mgmt_api_configs.erl index 80818b41a..f2e336d0f 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_configs.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_configs.erl @@ -122,6 +122,7 @@ schema("/configs") -> }} ] }, + 400 => emqx_dashboard_swagger:error_codes(['INVALID_ACCEPT']), 404 => emqx_dashboard_swagger:error_codes(['NOT_FOUND']), 500 => emqx_dashboard_swagger:error_codes(['BAD_NODE']) } @@ -349,7 +350,7 @@ configs(put, #{body := Conf, query_string := #{<<"mode">> := Mode}}, _Req) -> {error, Errors} -> {400, #{code => 'UPDATE_FAILED', message => ?ERR_MSG(Errors)}} end. -find_suitable_accept(Headers, Perferences) -> +find_suitable_accept(Headers, Perferences) when is_list(Perferences), length(Perferences) > 0 -> AcceptVal = maps:get(<<"accept">>, Headers, <<"*/*">>), %% Multiple types, weighted with the quality value syntax: %% Accept: text/html, application/xhtml+xml, application/xml;q=0.9, image/webp, */*;q=0.8 @@ -362,12 +363,12 @@ find_suitable_accept(Headers, Perferences) -> ), case lists:member(<<"*/*">>, Accepts) of true -> - {ok, lists:first(Perferences)}; - fales -> + {ok, lists:nth(1, Perferences)}; + false -> Found = lists:filter(fun(Accept) -> lists:member(Accept, Accepts) end, Perferences), case Found of [] -> {error, no_suitalbe_accept}; - _ -> lists:first(Found) + _ -> {ok, lists:nth(1, Found)} end end. diff --git a/apps/emqx_management/test/emqx_mgmt_api_configs_SUITE.erl b/apps/emqx_management/test/emqx_mgmt_api_configs_SUITE.erl index 0e54a3e22..43554c9ff 100644 --- a/apps/emqx_management/test/emqx_mgmt_api_configs_SUITE.erl +++ b/apps/emqx_management/test/emqx_mgmt_api_configs_SUITE.erl @@ -323,6 +323,36 @@ t_configs_key(_Config) -> ?assertEqual(<<"error">>, read_conf([<<"log">>, <<"console">>, <<"level">>])), ok. +t_get_configs_in_different_accept(_Config) -> + [Key | _] = lists:sort(emqx_conf_cli:keys()), + URI = emqx_mgmt_api_test_util:api_path(["configs?key=" ++ Key]), + Auth = emqx_mgmt_api_test_util:auth_header_(), + Request = fun(Accept) -> + Headers = [{"accept", Accept}, Auth], + case + emqx_mgmt_api_test_util:request_api(get, URI, [], Headers, [], #{return_all => true}) + of + {ok, {{_, Code, _}, RespHeaders, Body}} -> + Type = proplists:get_value("content-type", RespHeaders), + {Code, Type, Body}; + {error, {{_, Code, _}, RespHeaders, Body}} -> + Type = proplists:get_value("content-type", RespHeaders), + {Code, Type, Body} + end + end, + + %% returns text/palin if text/plain is acceptable + ?assertMatch({200, "text/plain", _}, Request(<<"text/plain">>)), + ?assertMatch({200, "text/plain", _}, Request(<<"*/*">>)), + ?assertMatch( + {200, "text/plain", _}, + Request(<<"application/json, application/xml;q=0.9, image/webp, */*;q=0.8">>) + ), + %% returns application/json if it only support it + ?assertMatch({200, "application/json", _}, Request(<<"application/json">>)), + %% returns error if it set to other type + ?assertMatch({400, "application/json", _}, Request(<<"application/xml">>)). + %% Helpers get_config(Name) ->