From f45298d14a5872768b933117b7b829e26b0b127d Mon Sep 17 00:00:00 2001 From: JianBo He Date: Mon, 8 Jan 2024 15:03:10 +0800 Subject: [PATCH 1/4] chore: improve the error message if the parameter invalid --- apps/emqx_management/src/emqx_mgmt_api.erl | 21 ++++++++++++------- .../src/emqx_mgmt_api_clients.erl | 8 +++++++ .../test/emqx_mgmt_api_clients_SUITE.erl | 17 +++++++++++++++ 3 files changed, 38 insertions(+), 8 deletions(-) diff --git a/apps/emqx_management/src/emqx_mgmt_api.erl b/apps/emqx_management/src/emqx_mgmt_api.erl index b14d1d3165..4db37a7dcb 100644 --- a/apps/emqx_management/src/emqx_mgmt_api.erl +++ b/apps/emqx_management/src/emqx_mgmt_api.erl @@ -206,14 +206,19 @@ cluster_query(Tab, QString, QSchema, MsFun, FmtFun, Options) -> false -> {error, page_limit_invalid}; Meta -> - {_CodCnt, NQString} = parse_qstring(QString, QSchema), - Nodes = emqx:running_nodes(), - ResultAcc = init_query_result(), - QueryState = init_query_state(Tab, NQString, MsFun, Meta, Options), - NResultAcc = do_cluster_query( - Nodes, QueryState, ResultAcc - ), - format_query_result(FmtFun, Meta, NResultAcc) + try + {_CodCnt, NQString} = parse_qstring(QString, QSchema), + Nodes = emqx:running_nodes(), + ResultAcc = init_query_result(), + QueryState = init_query_state(Tab, NQString, MsFun, Meta, Options), + NResultAcc = do_cluster_query( + Nodes, QueryState, ResultAcc + ), + format_query_result(FmtFun, Meta, NResultAcc) + catch + throw:{bad_value_type, {Key, ExpectedType, AcutalValue}} -> + {error, invalid_query_string_param, {Key, ExpectedType, AcutalValue}} + end end. %% @private diff --git a/apps/emqx_management/src/emqx_mgmt_api_clients.erl b/apps/emqx_management/src/emqx_mgmt_api_clients.erl index e847d7ab9f..ab9dc4937f 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_clients.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_clients.erl @@ -699,6 +699,14 @@ list_clients(QString) -> case Result of {error, page_limit_invalid} -> {400, #{code => <<"INVALID_PARAMETER">>, message => <<"page_limit_invalid">>}}; + {error, invalid_query_string_param, {Key, ExpectedType, AcutalValue}} -> + Message = list_to_binary( + io_lib:format( + "the ~s parameter expected type is ~s, but the got value is ~s", + [Key, ExpectedType, emqx_utils_conv:str(AcutalValue)] + ) + ), + {400, #{code => <<"INVALID_PARAMETER">>, message => Message}}; {error, Node, {badrpc, R}} -> Message = list_to_binary(io_lib:format("bad rpc call ~p, Reason ~p", [Node, R])), {500, #{code => <<"NODE_DOWN">>, message => Message}}; diff --git a/apps/emqx_management/test/emqx_mgmt_api_clients_SUITE.erl b/apps/emqx_management/test/emqx_mgmt_api_clients_SUITE.erl index 32fbfdee5f..cf4fc019d4 100644 --- a/apps/emqx_management/test/emqx_mgmt_api_clients_SUITE.erl +++ b/apps/emqx_management/test/emqx_mgmt_api_clients_SUITE.erl @@ -167,6 +167,23 @@ t_clients(_) -> AfterKickoutResponse1 = emqx_mgmt_api_test_util:request_api(get, Client1Path), ?assertEqual({error, {"HTTP/1.1", 404, "Not Found"}}, AfterKickoutResponse1). +t_clients_bad_value_type(_) -> + %% get /clients + AuthHeader = [emqx_common_test_http:default_auth_header()], + ClientsPath = emqx_mgmt_api_test_util:api_path(["clients"]), + QsString = cow_qs:qs([{<<"ip_address">>, <<"127.0.0.1:8080">>}]), + {ok, 400, Resp} = emqx_mgmt_api_test_util:request_api( + get, ClientsPath, QsString, AuthHeader, [], #{compatible_mode => true} + ), + ?assertMatch( + #{ + <<"code">> := <<"INVALID_PARAMETER">>, + <<"message">> := + <<"the ip_address parameter expected type is ip, but the got value is 127.0.0.1:8080">> + }, + emqx_utils_json:decode(Resp, [return_maps]) + ). + t_authz_cache(_) -> ClientId = <<"client_authz">>, From b420e116a887e1f5915589d6c6f213dde70c577e Mon Sep 17 00:00:00 2001 From: JianBo He Date: Mon, 8 Jan 2024 15:10:20 +0800 Subject: [PATCH 2/4] chore: update changes --- changes/ce/fix-12269.en.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 changes/ce/fix-12269.en.md diff --git a/changes/ce/fix-12269.en.md b/changes/ce/fix-12269.en.md new file mode 100644 index 0000000000..970bdfbe86 --- /dev/null +++ b/changes/ce/fix-12269.en.md @@ -0,0 +1 @@ +Returns 400 and more detailed error messages instead of 500 when query string validation fails in the `/clients` interface. From da0307faa96d133f59a9d95c73bd1cdcf917ce50 Mon Sep 17 00:00:00 2001 From: JianBo He Date: Tue, 9 Jan 2024 10:41:01 +0800 Subject: [PATCH 3/4] chore: update apps/emqx_management/src/emqx_mgmt_api_clients.erl Co-authored-by: William Yang --- apps/emqx_management/src/emqx_mgmt_api_clients.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/emqx_management/src/emqx_mgmt_api_clients.erl b/apps/emqx_management/src/emqx_mgmt_api_clients.erl index ab9dc4937f..357e5bc5cd 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_clients.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_clients.erl @@ -702,7 +702,7 @@ list_clients(QString) -> {error, invalid_query_string_param, {Key, ExpectedType, AcutalValue}} -> Message = list_to_binary( io_lib:format( - "the ~s parameter expected type is ~s, but the got value is ~s", + "the ~s parameter expected type is ~s, but the value is ~s", [Key, ExpectedType, emqx_utils_conv:str(AcutalValue)] ) ), From 6ab8e31db099d42e50ef83d843f0f7c9d71a84e9 Mon Sep 17 00:00:00 2001 From: JianBo He Date: Wed, 10 Jan 2024 09:53:22 +0800 Subject: [PATCH 4/4] test: fix failed tests --- apps/emqx_management/test/emqx_mgmt_api_clients_SUITE.erl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/emqx_management/test/emqx_mgmt_api_clients_SUITE.erl b/apps/emqx_management/test/emqx_mgmt_api_clients_SUITE.erl index cf4fc019d4..c53299b5a6 100644 --- a/apps/emqx_management/test/emqx_mgmt_api_clients_SUITE.erl +++ b/apps/emqx_management/test/emqx_mgmt_api_clients_SUITE.erl @@ -179,7 +179,7 @@ t_clients_bad_value_type(_) -> #{ <<"code">> := <<"INVALID_PARAMETER">>, <<"message">> := - <<"the ip_address parameter expected type is ip, but the got value is 127.0.0.1:8080">> + <<"the ip_address parameter expected type is ip, but the value is 127.0.0.1:8080">> }, emqx_utils_json:decode(Resp, [return_maps]) ).