From 969ddc08b934a5d893bb4a7bb2e4b22c6967c49a Mon Sep 17 00:00:00 2001 From: jacekwegr Date: Thu, 19 Oct 2023 15:06:32 +0200 Subject: [PATCH] Change mod_ping to handle ping timeout with sm --- big_tests/tests/sm_SUITE.erl | 54 +++++++++++++++++++++++++++++++++++- src/mod_ping.erl | 27 +++++++++++++++++- 2 files changed, 79 insertions(+), 2 deletions(-) diff --git a/big_tests/tests/sm_SUITE.erl b/big_tests/tests/sm_SUITE.erl index 01c4487c86..45c4ffdf31 100644 --- a/big_tests/tests/sm_SUITE.erl +++ b/big_tests/tests/sm_SUITE.erl @@ -40,6 +40,8 @@ -define(LONG_TIMEOUT, 3600). -define(SHORT_TIMEOUT, 1). -define(SMALL_SM_BUFFER, 3). +-define(PING_REQUEST_TIMEOUT, 1). +-define(PING_INTERVAL, 3). %%-------------------------------------------------------------------- %% Suite configuration @@ -57,7 +59,8 @@ groups() -> {parallel_manual_ack_freq_1, [parallel], parallel_manual_ack_freq_1_cases()}, {manual_ack_freq_2, [], manual_ack_freq_2_cases()}, {stale_h, [], stale_h_cases()}, - {parallel_unacknowledged_message_hook, [parallel], parallel_unacknowledged_message_hook_cases()} + {parallel_unacknowledged_message_hook, [parallel], parallel_unacknowledged_message_hook_cases()}, + {ping_timeout, [], ping_timeout_cases()} ]. parallel_cases() -> @@ -124,6 +127,9 @@ parallel_unacknowledged_message_hook_cases() -> unacknowledged_message_hook_resume, unacknowledged_message_hook_filter]. +ping_timeout_cases() -> + [ping_timeout]. + %%-------------------------------------------------------------------- %% Init & teardown %%-------------------------------------------------------------------- @@ -151,6 +157,13 @@ init_per_group(stream_mgmt_disabled, Config) -> dynamic_modules:stop(host_type(), ?MOD_SM), rpc(mim(), mnesia, delete_table, [sm_session]), Config; +init_per_group(ping_timeout = Group, Config) -> + dynamic_modules:ensure_modules(host_type(), required_modules(Config, group, Group)), + start_mod_ping(#{send_pings => true, + ping_interval => ?PING_INTERVAL, + ping_req_timeout => ?PING_REQUEST_TIMEOUT, + timeout_action => kill}), + Config; init_per_group(Group, Config) -> dynamic_modules:ensure_modules(host_type(), required_modules(Config, group, Group)), Config. @@ -210,6 +223,9 @@ required_sm_opts(group, parallel_unacknowledged_message_hook) -> #{ack_freq => 1}; required_sm_opts(group, manual_ack_freq_long_session_timeout) -> #{ack_freq => 1, buffer_max => 1000}; +required_sm_opts(group, ping_timeout) -> + #{ack_freq => 1, + resume_timeout => ?SHORT_TIMEOUT}; required_sm_opts(testcase, resume_expired_session_returns_correct_h) -> #{ack_freq => 1, resume_timeout => ?SHORT_TIMEOUT, @@ -240,6 +256,10 @@ register_some_smid_h(Config) -> TestSmids = lists:map(fun register_smid/1, lists:seq(1, 3)), [{smid_test, TestSmids} | Config]. +start_mod_ping(Opts) -> + HostType = domain_helper:host_type(mim), + dynamic_modules:start(HostType, mod_ping, config_parser_helper:mod_config(mod_ping, Opts)). + %%-------------------------------------------------------------------- %% Tests %%-------------------------------------------------------------------- @@ -591,6 +611,38 @@ resend_unacked_after_resume_timeout(Config) -> escalus_connection:stop(Bob), escalus_connection:stop(NewAlice). +ping_timeout(Config) -> + % ct:sleep(timer:seconds(10)), + + logger_ct_backend:start(), + logger_ct_backend:capture(error), + + %% connect Alice and simulate ping timeout by dropping connection + Alice = connect_fresh(Config, alice, sr_presence), + + escalus_client:wait_for_stanza(Alice), + ct:sleep(?PING_REQUEST_TIMEOUT + ?PING_INTERVAL + timer:seconds(1)), + + %% attempt to resume the session after the connection drop + NewAlice = sm_helper:kill_and_connect_with_resume_session_without_waiting_for_result(Alice), + + %% after resume_timeout, we expect the session to be closed + escalus_connection:get_stanza(NewAlice, failed_resumption), + + %% bind a new session and expect unacknowledged messages to be resent + escalus_session:session(escalus_session:bind(NewAlice)), + send_initial_presence(NewAlice), + + % check if the error occurred + FilterFun = fun(_, Msg) -> + re:run(Msg, "parse_iq_id_failed", [global]) /= nomatch + end, + ?assertEqual([], logger_ct_backend:recv(FilterFun)), + logger_ct_backend:stop_capture(), + + %% stop the connection + escalus_connection:stop(NewAlice). + resume_expired_session_returns_correct_h(Config) -> %% connect bob and alice Bob = connect_fresh(Config, bob, sr_presence), diff --git a/src/mod_ping.erl b/src/mod_ping.erl index 2811d97c57..c6348c0977 100644 --- a/src/mod_ping.erl +++ b/src/mod_ping.erl @@ -29,6 +29,7 @@ -export([user_send_packet/3, user_send_iq/3, user_ping_response/3, + filter_local_packet/3, iq_ping/5]). %% Record that will be stored in the c2s state when the server pings the client, @@ -40,7 +41,8 @@ %%==================================================================== hooks(HostType) -> - [{user_ping_response, HostType, fun ?MODULE:user_ping_response/3, #{}, 100} + [{user_ping_response, HostType, fun ?MODULE:user_ping_response/3, #{}, 100}, + {filter_local_packet, HostType, fun ?MODULE:filter_local_packet/3, #{}, 100} | c2s_hooks(HostType)]. -spec c2s_hooks(mongooseim:host_type()) -> gen_hook:hook_list(mongoose_c2s_hooks:fn()). @@ -120,6 +122,19 @@ iq_ping(Acc, _From, _To, #iq{sub_el = SubEl} = IQ, _) -> %% Hook callbacks %%==================================================================== +-spec filter_local_packet(Acc, Params, Extra) -> {ok, Acc} | {stop, drop} when + Acc :: mongoose_hooks:filter_packet_acc(), + Params :: map(), + Extra :: gen_hook:extra(). +filter_local_packet({_, _, _, Stanza} = Acc, _Params, _Extra) -> + case is_ping_error(Stanza) of + true -> + ?LOG_DEBUG(#{what => ping_error_received, acc => Acc}), + {stop, drop}; + false -> + {ok, Acc} + end. + -spec user_send_iq(mongoose_acc:t(), mongoose_c2s_hooks:params(), gen_hook:extra()) -> mongoose_c2s_hooks:result(). user_send_iq(Acc, #{c2s_data := StateData}, #{host_type := HostType}) -> @@ -200,3 +215,13 @@ ping_get(Id) -> #xmlel{name = <<"iq">>, attrs = [{<<"type">>, <<"get">>}, {<<"id">>, Id}], children = [#xmlel{name = <<"ping">>, attrs = [{<<"xmlns">>, ?NS_PING}]}]}. + +-spec is_ping_error(exml:element()) -> boolean(). +is_ping_error(#xmlel{name = <<"iq">>, + attrs = [{<<"type">>, <<"error">>}, _, _, _], + children = [#xmlel{name = <<"ping">>, + attrs = [{<<"xmlns">>, ?NS_PING}]}, + #xmlel{name = <<"error">>}]}) -> + true; +is_ping_error(_) -> + false.