From a91f975dc2a77d8fc9a37095208cfcba3c284954 Mon Sep 17 00:00:00 2001
From: zhongwencool <zhongwencool@gmail.com>
Date: Mon, 22 Nov 2021 18:08:48 +0800
Subject: [PATCH] fix: make sure keepalive only 0~65535 (#6232)

---
 apps/emqx_management/src/emqx_mgmt.erl             |  6 ++++--
 apps/emqx_management/src/emqx_mgmt_api_clients.erl |  1 +
 apps/emqx_management/test/emqx_mgmt_api_SUITE.erl  |  9 +++++++++
 src/emqx_keepalive.erl                             | 14 +++++++++++++-
 4 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/apps/emqx_management/src/emqx_mgmt.erl b/apps/emqx_management/src/emqx_mgmt.erl
index 93f24cd37..61c32613b 100644
--- a/apps/emqx_management/src/emqx_mgmt.erl
+++ b/apps/emqx_management/src/emqx_mgmt.erl
@@ -274,8 +274,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, ?ERROR2, <<"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 5aacc7895..a25cc9ff5 100644
--- a/apps/emqx_management/src/emqx_mgmt_api_clients.erl
+++ b/apps/emqx_management/src/emqx_mgmt_api_clients.erl
@@ -261,6 +261,7 @@ set_keepalive(#{clientid := ClientId}, Params) ->
             case emqx_mgmt:set_keepalive(emqx_mgmt_util:urldecode(ClientId), Interval) of
                 ok -> minirest:return();
                 {error, not_found} -> minirest:return({error, ?ERROR12, not_found});
+                {error, Code, Reason} -> minirest:return({error, Code, Reason});
                 {error, Reason} -> minirest:return({error, ?ERROR1, Reason})
             end
     end.
diff --git a/apps/emqx_management/test/emqx_mgmt_api_SUITE.erl b/apps/emqx_management/test/emqx_mgmt_api_SUITE.erl
index f3bc42f3a..a3eea2ab3 100644
--- a/apps/emqx_management/test/emqx_mgmt_api_SUITE.erl
+++ b/apps/emqx_management/test/emqx_mgmt_api_SUITE.erl
@@ -657,6 +657,15 @@ t_keepalive(_Config) ->
     {ok, _} = emqtt:connect(C1),
     {ok, Ok} = request_api(put, Path, "interval=5", AuthHeader, [#{}]),
     ?assertEqual("{\"code\":0}", Ok),
+    [Pid] = emqx_cm:lookup_channels(list_to_binary(ClientId)),
+    #{conninfo := #{keepalive := Keepalive}} = emqx_connection:info(Pid),
+    ?assertEqual(5, Keepalive),
+    {ok, Error1} = request_api(put, Path, "interval=-1", AuthHeader, [#{}]),
+    {ok, Error2} = request_api(put, Path, "interval=65536", AuthHeader, [#{}]),
+    ErrMsg = #{<<"code">> => 102,
+               <<"message">> => <<"mqtt3.1.1 specification: keepalive must between 0~65535">>},
+    ?assertEqual(ErrMsg, jiffy:decode(Error1, [return_maps])),
+    ?assertEqual(Error1, Error2),
     emqtt:disconnect(C1),
     application:stop(emqx_dashboard),
     ok.
diff --git a/src/emqx_keepalive.erl b/src/emqx_keepalive.erl
index 216999acc..bef0714f9 100644
--- a/src/emqx_keepalive.erl
+++ b/src/emqx_keepalive.erl
@@ -73,7 +73,19 @@ 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}.