From fdff3c5a53b110b18af61b45f63597e04105803d Mon Sep 17 00:00:00 2001 From: zhongwencool Date: Fri, 19 Nov 2021 17:57:41 +0800 Subject: [PATCH 1/2] fix: keepalive ct failed --- apps/emqx_management/test/emqx_mgmt_clients_api_SUITE.erl | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/apps/emqx_management/test/emqx_mgmt_clients_api_SUITE.erl b/apps/emqx_management/test/emqx_mgmt_clients_api_SUITE.erl index b961ed391..0b830dc79 100644 --- a/apps/emqx_management/test/emqx_mgmt_clients_api_SUITE.erl +++ b/apps/emqx_management/test/emqx_mgmt_clients_api_SUITE.erl @@ -175,8 +175,7 @@ t_keepalive(_Config) -> {ok, Ok} = emqx_mgmt_api_test_util:request_api(put, Path, Query, AuthHeader, <<"">>), ?assertEqual("", Ok), [Pid] = emqx_cm:lookup_channels(list_to_binary(ClientId)), - State = sys:get_state(Pid), - ct:pal("~p~n", [State]), - ?assertEqual(11000, element(2, element(5, element(9, State)))), + #{conninfo := #{keepalive := Keepalive}} = emqx_connection:info(Pid), + ?assertEqual(11, Keepalive), emqtt:disconnect(C1), ok. From 870af6df4174a7138eb98d78999f752da460fc9f Mon Sep 17 00:00:00 2001 From: zhongwencool Date: Mon, 22 Nov 2021 16:05:39 +0800 Subject: [PATCH 2/2] fix: Limit interval between 0~65535 --- apps/emqx/src/emqx_keepalive.erl | 13 ++++++++++++- apps/emqx_management/src/emqx_mgmt.erl | 6 ++++-- apps/emqx_management/src/emqx_mgmt_api_clients.erl | 4 +++- 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/apps/emqx/src/emqx_keepalive.erl b/apps/emqx/src/emqx_keepalive.erl index 7ec424d1d..9768ff876 100644 --- a/apps/emqx/src/emqx_keepalive.erl +++ b/apps/emqx/src/emqx_keepalive.erl @@ -74,7 +74,18 @@ check(NewVal, KeepAlive = #keepalive{statval = OldVal, true -> {error, timeout} end. +%% from mqtt-v3.1.1 specific +%% A Keep Alive value of zero (0) has the effect of turning off the keep alive mechanism. +%% This means that, in this case, the Server is not required +%% to disconnect the Client on the grounds of inactivity. +%% Note that a Server is permitted to disconnect a Client that it determines +%% to be inactive or non-responsive at any time, +%% regardless of the Keep Alive value provided by that Client. +%% Non normative comment +%%The actual value of the Keep Alive is application specific; +%% typically this is a few minutes. +%% The maximum value is (65535s) 18 hours 12 minutes and 15 seconds. %% @doc Update keepalive's interval -spec(set(interval, non_neg_integer(), keepalive()) -> keepalive()). -set(interval, Interval, KeepAlive) -> +set(interval, Interval, KeepAlive) when Interval >= 0 andalso Interval =< 65535000 -> KeepAlive#keepalive{interval = Interval}. diff --git a/apps/emqx_management/src/emqx_mgmt.erl b/apps/emqx_management/src/emqx_mgmt.erl index d45b5ad77..826a2135f 100644 --- a/apps/emqx_management/src/emqx_mgmt.erl +++ b/apps/emqx_management/src/emqx_mgmt.erl @@ -336,8 +336,10 @@ set_ratelimit_policy(ClientId, Policy) -> set_quota_policy(ClientId, Policy) -> call_client(ClientId, {quota, Policy}). -set_keepalive(ClientId, Interval) -> - call_client(ClientId, {keepalive, Interval}). +set_keepalive(ClientId, Interval)when Interval >= 0 andalso Interval =< 65535 -> + call_client(ClientId, {keepalive, Interval}); +set_keepalive(_ClientId, _Interval) -> + {error, <<"mqtt3.1.1 specification: keepalive must between 0~65535">>}. %% @private call_client(ClientId, Req) -> diff --git a/apps/emqx_management/src/emqx_mgmt_api_clients.erl b/apps/emqx_management/src/emqx_mgmt_api_clients.erl index ff429a9a6..e10902752 100644 --- a/apps/emqx_management/src/emqx_mgmt_api_clients.erl +++ b/apps/emqx_management/src/emqx_mgmt_api_clients.erl @@ -457,6 +457,7 @@ keepalive_api() -> ], responses => #{ <<"404">> => emqx_mgmt_util:error_schema(<<"Client id not found">>), + <<"400">> => emqx_mgmt_util:error_schema(<<"">>, 'PARAMS_ERROR'), <<"200">> => emqx_mgmt_util:schema(<<"ok">>)}}}, {"/clients/:clientid/keepalive", Metadata, set_keepalive}. %%%============================================================================================== @@ -509,7 +510,8 @@ set_keepalive(put, #{bindings := #{clientid := ClientID}, query_string := Query} Interval = binary_to_integer(Interval0), case emqx_mgmt:set_keepalive(emqx_mgmt_util:urldecode(ClientID), Interval) of ok -> {200}; - {error, not_found} ->{404, ?CLIENT_ID_NOT_FOUND} + {error, not_found} ->{404, ?CLIENT_ID_NOT_FOUND}; + {error, Reason} -> {400, #{code => 'PARAMS_ERROR', message => Reason}} end end.