diff --git a/CHANGES-4.3.md b/CHANGES-4.3.md index 0e320dbb4..10636eb6b 100644 --- a/CHANGES-4.3.md +++ b/CHANGES-4.3.md @@ -23,6 +23,8 @@ File format: `SELECT topic =~ 't' as r FROM "$events/client_connected"`. The topic is a null value as there's no such field in event `$events/client_connected`, so it should return false if match it to a topic. +- Added a test to prevent a last will testament message to be + published when a client is denied connection. [#8894](https://github.com/emqx/emqx/pull/8894) ## v4.3.19 diff --git a/apps/emqx_auth_mnesia/test/emqx_auth_mnesia_SUITE.erl b/apps/emqx_auth_mnesia/test/emqx_auth_mnesia_SUITE.erl index 0253110aa..f7071bc17 100644 --- a/apps/emqx_auth_mnesia/test/emqx_auth_mnesia_SUITE.erl +++ b/apps/emqx_auth_mnesia/test/emqx_auth_mnesia_SUITE.erl @@ -21,6 +21,9 @@ -include("emqx_auth_mnesia.hrl"). -include_lib("eunit/include/eunit.hrl"). -include_lib("common_test/include/ct.hrl"). +-include_lib("snabbkaffe/include/snabbkaffe.hrl"). + +-include_lib("emqx/include/emqx_mqtt.hrl"). -import(emqx_ct_http, [ request_api/3 , request_api/5 @@ -74,6 +77,20 @@ set_default(ClientId, UserName, Pwd, HashType) -> application:set_env(emqx_auth_mnesia, username_list, [{UserName, Pwd}]), application:set_env(emqx_auth_mnesia, password_hash, HashType), ok. + +init_per_testcase(t_will_message_connection_denied, Config) -> + emqx_zone:set_env(external, allow_anonymous, false), + Config; +init_per_testcase(_TestCase, Config) -> + Config. + +end_per_testcase(t_will_message_connection_denied, _Config) -> + emqx_zone:unset_env(external, allow_anonymous), + application:stop(emqx_auth_mnesia), + ok; +end_per_testcase(_TestCase, _Config) -> + ok. + %%------------------------------------------------------------------------------ %% Testcases %%------------------------------------------------------------------------------ @@ -390,6 +407,48 @@ t_password_hash(_) -> application:stop(emqx_auth_mnesia), ok = application:start(emqx_auth_mnesia). +t_will_message_connection_denied(Config) when is_list(Config) -> + ClientId = Username = <<"subscriber">>, + Password = <<"p">>, + application:stop(emqx_auth_mnesia), + ok = emqx_ct_helpers:start_apps([emqx_auth_mnesia]), + ok = emqx_auth_mnesia_cli:add_user({clientid, ClientId}, Password), + + {ok, Subscriber} = emqtt:start_link([ + {clientid, ClientId}, + {password, Password} + ]), + {ok, _} = emqtt:connect(Subscriber), + {ok, _, [?RC_SUCCESS]} = emqtt:subscribe(Subscriber, <<"lwt">>), + + process_flag(trap_exit, true), + + {ok, Publisher} = emqtt:start_link([ + {clientid, <<"publisher">>}, + {will_topic, <<"lwt">>}, + {will_payload, <<"should not be published">>} + ]), + snabbkaffe:start_trace(), + ?wait_async_action( + {error, _} = emqtt:connect(Publisher), + #{?snk_kind := channel_terminated} + ), + snabbkaffe:stop(), + + timer:sleep(1000), + + receive + {publish, #{ + topic := <<"lwt">>, + payload := <<"should not be published">> + }} -> + ct:fail("should not publish will message") + after 0 -> + ok + end, + + ok. + %%------------------------------------------------------------------------------ %% Helpers %%------------------------------------------------------------------------------ diff --git a/src/emqx_channel.erl b/src/emqx_channel.erl index c49299c58..a7565533b 100644 --- a/src/emqx_channel.erl +++ b/src/emqx_channel.erl @@ -1156,20 +1156,34 @@ interval(will_timer, #channel{will_msg = WillMsg}) -> %%-------------------------------------------------------------------- -spec(terminate(any(), channel()) -> ok). -terminate(_, #channel{conn_state = idle}) -> ok; +terminate(_Reason, #channel{conn_state = idle} = _Channel) -> + ?tp(channel_terminated, #{channel => _Channel, reason => _Reason}), + ok; terminate(normal, Channel) -> run_terminate_hook(normal, Channel); -terminate({shutdown, Reason}, Channel) - when Reason =:= kicked; Reason =:= discarded; Reason =:= takeovered -> - run_terminate_hook(Reason, Channel); terminate(Reason, Channel = #channel{will_msg = WillMsg}) -> - (WillMsg =/= undefined) andalso publish_will_msg(WillMsg), + should_publish_will_message(Reason, Channel) + andalso publish_will_msg(WillMsg), run_terminate_hook(Reason, Channel). -run_terminate_hook(_Reason, #channel{session = undefined}) -> ok; -run_terminate_hook(Reason, #channel{clientinfo = ClientInfo, session = Session}) -> +run_terminate_hook(_Reason, #channel{session = undefined} = _Channel) -> + ?tp(channel_terminated, #{channel => _Channel, reason => _Reason}), + ok; +run_terminate_hook(Reason, #channel{clientinfo = ClientInfo, session = Session} = _Channel) -> + ?tp(channel_terminated, #{channel => _Channel, reason => Reason}), emqx_session:terminate(ClientInfo, Reason, Session). +should_publish_will_message(TerminateReason, Channel) -> + not lists:member(TerminateReason, [ {shutdown, kicked} + , {shutdown, discarded} + , {shutdown, takeovered} + , {shutdown, not_authorized} + ]) + andalso not lists:member(info(conn_state, Channel), [ idle + , connecting + ]) + andalso info(will_msg, Channel) =/= undefined. + %%-------------------------------------------------------------------- %% Internal functions %%--------------------------------------------------------------------