From 2c099c20d88413bb82dcc0dd7f12e2793cf14814 Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Wed, 9 Nov 2022 20:57:35 +0100 Subject: [PATCH 1/2] Merge c2s_preprocessing_hook into user_send_packet --- big_tests/tests/acc_e2e_SUITE.erl | 8 ++-- doc/developers-guide/hooks_description.md | 1 - src/c2s/mongoose_c2s.erl | 12 +----- src/c2s/mongoose_c2s_hooks.erl | 17 --------- src/jingle_sip/mod_jingle_sip.erl | 38 ++++++------------- src/mod_amp.erl | 8 ++-- .../mod_stream_management.erl | 2 +- 7 files changed, 22 insertions(+), 64 deletions(-) diff --git a/big_tests/tests/acc_e2e_SUITE.erl b/big_tests/tests/acc_e2e_SUITE.erl index 142b26b72c6..208627ac534 100644 --- a/big_tests/tests/acc_e2e_SUITE.erl +++ b/big_tests/tests/acc_e2e_SUITE.erl @@ -74,26 +74,26 @@ end_per_suite(Config) -> init_per_group(message, Config) -> % saves ref, timestamp and some attrs of acc in ets - add_handler(c2s_preprocessing_hook, test_save_acc, 50), + add_handler(user_send_packet, test_save_acc, 5), % checks it is the same acc add_handler(filter_local_packet, test_check_acc, 50), % checks it is the same acc and it has been stripped but keeps persistent props add_handler(user_receive_packet, test_check_final_acc, 50), Config; init_per_group(cache_and_strip, Config) -> - add_handler(c2s_preprocessing_hook, save_my_jid, 50), + add_handler(user_send_packet, save_my_jid, 5), add_handler(filter_local_packet, drop_if_jid_not_mine, 1), Config; init_per_group(_GroupName, Config) -> Config. end_per_group(message, _Config) -> - remove_handler(c2s_preprocessing_hook, test_save_acc, 50), + remove_handler(user_send_packet, test_save_acc, 5), remove_handler(filter_local_packet, test_check_acc, 50), remove_handler(user_receive_packet, test_check_final_acc, 50), ok; end_per_group(cache_and_strip, _Config) -> - remove_handler(c2s_preprocessing_hook, save_my_jid, 50), + remove_handler(user_send_packet, save_my_jid, 5), remove_handler(filter_local_packet, drop_if_jid_not_mine, 1), ok; end_per_group(_GroupName, _Config) -> diff --git a/doc/developers-guide/hooks_description.md b/doc/developers-guide/hooks_description.md index e77b4b43b6a..7eb8be747b4 100644 --- a/doc/developers-guide/hooks_description.md +++ b/doc/developers-guide/hooks_description.md @@ -164,7 +164,6 @@ This is the perfect place to plug in custom security control. * auth_failed * c2s_broadcast_recipients * c2s_filter_packet -* c2s_preprocessing_hook * c2s_presence_in * c2s_remote_hook * c2s_stream_features diff --git a/src/c2s/mongoose_c2s.erl b/src/c2s/mongoose_c2s.erl index 55b23d7966e..d88ddae71ab 100644 --- a/src/c2s/mongoose_c2s.erl +++ b/src/c2s/mongoose_c2s.erl @@ -661,17 +661,7 @@ handle_foreign_packet(StateData = #c2s_data{host_type = HostType, lserver = LSer -spec handle_c2s_packet(c2s_data(), c2s_state(), exml:element()) -> fsm_res(). handle_c2s_packet(StateData = #c2s_data{host_type = HostType}, C2SState, El) -> HookParams = hook_arg(StateData, C2SState, internal, El, undefined), - Acc0 = element_to_origin_accum(StateData, El), - case mongoose_c2s_hooks:c2s_preprocessing_hook(HostType, Acc0, HookParams) of - {ok, Acc1} -> - do_handle_c2s_packet(StateData, C2SState, Acc1, HookParams); - {stop, _Acc1} -> - {next_state, C2SState, StateData} - end. - --spec do_handle_c2s_packet(c2s_data(), c2s_state(), mongoose_acc:t(), mongoose_c2s_hooks:hook_params()) -> - fsm_res(). -do_handle_c2s_packet(StateData = #c2s_data{host_type = HostType}, C2SState, Acc, HookParams) -> + Acc = element_to_origin_accum(StateData, El), case mongoose_c2s_hooks:user_send_packet(HostType, Acc, HookParams) of {ok, Acc1} -> Acc2 = handle_stanza_from_client(StateData, HookParams, Acc1, mongoose_acc:stanza_name(Acc1)), diff --git a/src/c2s/mongoose_c2s_hooks.erl b/src/c2s/mongoose_c2s_hooks.erl index a4c99f16d5b..a73ba2c37f8 100644 --- a/src/c2s/mongoose_c2s_hooks.erl +++ b/src/c2s/mongoose_c2s_hooks.erl @@ -12,7 +12,6 @@ -export_type([hook_fn/0, hook_params/0, hook_result/0]). %% XML handlers --export([c2s_preprocessing_hook/3]). -export([user_send_packet/3, user_receive_packet/3, user_send_message/3, @@ -34,22 +33,6 @@ user_socket_closed/3, user_socket_error/3]). -%%% @doc Event triggered after a user sends _any_ packet to the server. -%%% The purpose is to modify, or reject, packets, before they are further processed. -%%% TODO: this can really be merged into `user_send_packet' with early priority. --spec c2s_preprocessing_hook(HostType, Acc, Params) -> Result when - HostType :: mongooseim:host_type(), - Acc :: mongoose_acc:t(), - Params :: hook_params(), - Result :: hook_result(). -c2s_preprocessing_hook(HostType, Acc, #{c2s_data := StateData} = Params) -> - ParamsWithLegacyArgs = ejabberd_hooks:add_args(Params, [StateData]), - {Tag, Acc1} = gen_hook:run_fold(c2s_preprocessing_hook, HostType, Acc, ParamsWithLegacyArgs), - case mongoose_acc:get(hook, result, undefined, Acc1) of - drop -> {stop, Acc1}; - _ -> {Tag, Acc1} - end. - %%% @doc Event triggered after a user sends _any_ packet to the server. %%% Examples of handlers can be metrics, archives, and any other subsystem %%% that wants to see all stanzas the user delivers. diff --git a/src/jingle_sip/mod_jingle_sip.erl b/src/jingle_sip/mod_jingle_sip.erl index ba8a49630ca..b9f71fe29e0 100644 --- a/src/jingle_sip/mod_jingle_sip.erl +++ b/src/jingle_sip/mod_jingle_sip.erl @@ -24,16 +24,14 @@ -include("jlib.hrl"). -include("mongoose.hrl"). -include("mongoose_config_spec.hrl"). --include_lib("nklib/include/nklib.hrl"). -include_lib("nksip/include/nksip.hrl"). --include_lib("nksip/include/nksip_call.hrl"). -define(SERVICE, "mim_sip"). %% gen_mod callbacks -export([start/2, stop/1, config_spec/0]). --export([intercept_jingle_stanza/3]). +-export([user_send_packet/3]). -export([content_to_nksip_media/1]). @@ -121,27 +119,16 @@ process_u2p(#{username := U, phone := P}) -> {U, P}. hooks(Host) -> - [{c2s_preprocessing_hook, Host, fun ?MODULE:intercept_jingle_stanza/3, #{}, 75}]. - --spec intercept_jingle_stanza(Acc, Params, Extra) -> {ok, Acc} when - Acc :: mongoose_acc:t(), - Params :: map(), - Extra :: map(). -intercept_jingle_stanza(Acc, _, _) -> - NewAcc = case mongoose_acc:get(hook, result, undefined, Acc) of - drop -> - Acc; - _ -> - maybe_iq_stanza(Acc) - end, - {ok, NewAcc}. + [{user_send_packet, Host, fun ?MODULE:user_send_packet/3, #{}, 10}]. -maybe_iq_stanza(Acc) -> +-spec user_send_packet(mongoose_acc:t(), mongoose_c2s_hooks:hook_params(), gen_hook:extra()) -> + gen_hook:hook_fn_ret(mongoose_acc:t()). +user_send_packet(Acc, _, _) -> case mongoose_acc:stanza_name(Acc) of <<"iq">> -> maybe_iq_to_other_user(Acc); _ -> - Acc + {ok, Acc} end. maybe_iq_to_other_user(Acc) -> @@ -162,19 +149,19 @@ maybe_jingle_stanza(#iq{xmlns = ?JINGLE_NS, sub_el = Jingle, type = set} = IQ, A To = mongoose_acc:to_jid(Acc), maybe_translate_to_sip(JingleAction, From, To, IQ, Acc); maybe_jingle_stanza(_, Acc) -> - Acc. + {ok, Acc}. maybe_jingle_get_stanza_to_self(#iq{xmlns = ?JINGLE_NS, sub_el = Jingle, type = get} = IQ, Acc) -> JingleAction = exml_query:attr(Jingle, <<"action">>), case JingleAction of <<"existing-session-initiate">> -> resend_session_initiate(IQ, Acc), - mongoose_acc:set(hook, result, drop, Acc); + {stop, Acc}; _ -> - Acc + {ok, Acc} end; maybe_jingle_get_stanza_to_self(_, Acc) -> - Acc. + {ok, Acc}. maybe_translate_to_sip(JingleAction, From, To, IQ, Acc) when JingleAction =:= <<"session-initiate">>; @@ -193,12 +180,12 @@ maybe_translate_to_sip(JingleAction, From, To, IQ, Acc) ?LOG_ERROR(#{what => sip_translate_failed, acc => Acc, class => Class, reason => Error, stacktrace => StackTrace}) end, - mongoose_acc:set(hook, result, drop, Acc); + {stop, Acc}; maybe_translate_to_sip(JingleAction, _, _, _, Acc) -> ?LOG_WARNING(#{what => sip_unknown_action, text => <<"Forwarding unknown action to SIP">>, jingle_action => JingleAction, acc => Acc}), - Acc. + {ok, Acc}. route_result(ok, From, To, IQ) -> route_ok_result(From, To, IQ); @@ -610,4 +597,3 @@ nksip_uac_bye(Node, DialogHandle, Args) -> _ -> rpc:call(Node, nksip_uac, bye, [DialogHandle, Args], timer:seconds(5)) end. - diff --git a/src/mod_amp.erl b/src/mod_amp.erl index 42add58219e..ed4cd969b8b 100644 --- a/src/mod_amp.erl +++ b/src/mod_amp.erl @@ -9,7 +9,7 @@ -behaviour(mongoose_module_metrics). -xep([{xep, 79}, {version, "1.2"}, {comment, "partially implemented."}]). -export([start/2, stop/1, supported_features/0]). --export([run_initial_check/3, +-export([user_send_packet/3, check_packet/2, disco_local_features/3, c2s_stream_features/3, @@ -35,7 +35,7 @@ supported_features() -> [dynamic_domains]. -spec c2s_hooks(mongooseim:host_type()) -> gen_hook:hook_list(mongoose_c2s_hooks:hook_fn()). c2s_hooks(HostType) -> - [{c2s_preprocessing_hook, HostType, fun ?MODULE:run_initial_check/3, #{}, 10}]. + [{user_send_packet, HostType, fun ?MODULE:user_send_packet/3, #{}, 5}]. hooks(HostType) -> [ @@ -49,9 +49,9 @@ hooks(HostType) -> %% API --spec run_initial_check(mongoose_acc:t(), mongoose_c2s_hooks:hook_params(), gen_hook:extra()) -> +-spec user_send_packet(mongoose_acc:t(), mongoose_c2s_hooks:hook_params(), gen_hook:extra()) -> gen_hook:hook_fn_ret(mongoose_acc:t()). -run_initial_check(Acc, _, _) -> +user_send_packet(Acc, _, _) -> case mongoose_acc:stanza_name(Acc) of <<"message">> -> run_initial_check(Acc); _ -> {ok, Acc} diff --git a/src/stream_management/mod_stream_management.erl b/src/stream_management/mod_stream_management.erl index 7925e800575..251b01ff45e 100644 --- a/src/stream_management/mod_stream_management.erl +++ b/src/stream_management/mod_stream_management.erl @@ -103,7 +103,7 @@ hooks(HostType) -> -spec c2s_hooks(mongooseim:host_type()) -> gen_hook:hook_list(mongoose_c2s_hooks:hook_fn()). c2s_hooks(HostType) -> [ - {user_send_packet, HostType, fun ?MODULE:user_send_packet/3, #{}, 10}, + {user_send_packet, HostType, fun ?MODULE:user_send_packet/3, #{}, 20}, {user_receive_packet, HostType, fun ?MODULE:user_receive_packet/3, #{}, 100}, {user_send_xmlel, HostType, fun ?MODULE:user_send_xmlel/3, #{}, 50}, {foreign_event, HostType, fun ?MODULE:foreign_event/3, #{}, 50}, From 4e2cb5c928b82f29365b03c4b757689de5be12be Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Mon, 14 Nov 2022 14:52:47 +0100 Subject: [PATCH 2/2] Refactor handlers that are specific to a single type of stanza --- src/jingle_sip/mod_jingle_sip.erl | 33 +++++++++++-------------------- src/mod_amp.erl | 26 +++++++++++------------- 2 files changed, 23 insertions(+), 36 deletions(-) diff --git a/src/jingle_sip/mod_jingle_sip.erl b/src/jingle_sip/mod_jingle_sip.erl index b9f71fe29e0..935a40a0225 100644 --- a/src/jingle_sip/mod_jingle_sip.erl +++ b/src/jingle_sip/mod_jingle_sip.erl @@ -31,7 +31,7 @@ %% gen_mod callbacks -export([start/2, stop/1, config_spec/0]). --export([user_send_packet/3]). +-export([user_send_iq/3]). -export([content_to_nksip_media/1]). @@ -119,36 +119,27 @@ process_u2p(#{username := U, phone := P}) -> {U, P}. hooks(Host) -> - [{user_send_packet, Host, fun ?MODULE:user_send_packet/3, #{}, 10}]. - --spec user_send_packet(mongoose_acc:t(), mongoose_c2s_hooks:hook_params(), gen_hook:extra()) -> - gen_hook:hook_fn_ret(mongoose_acc:t()). -user_send_packet(Acc, _, _) -> - case mongoose_acc:stanza_name(Acc) of - <<"iq">> -> - maybe_iq_to_other_user(Acc); - _ -> - {ok, Acc} - end. + [{user_send_iq, Host, fun ?MODULE:user_send_iq/3, #{}, 10}]. -maybe_iq_to_other_user(Acc) -> - #jid{luser = StanzaTo} = mongoose_acc:to_jid(Acc), +-spec user_send_iq(mongoose_acc:t(), mongoose_c2s_hooks:hook_params(), gen_hook:extra()) -> + mongoose_c2s_hooks:hook_result(). +user_send_iq(Acc, _, _) -> + {From, To, Packet} = mongoose_acc:packet(Acc), + #jid{luser = StanzaTo} = To, #jid{luser = LUser} = mongoose_acc:get(c2s, origin_jid, Acc), case LUser of StanzaTo -> - QueryInfo = jlib:iq_query_info(mongoose_acc:element(Acc)), + QueryInfo = jlib:iq_query_info(Packet), maybe_jingle_get_stanza_to_self(QueryInfo, Acc); _ -> - QueryInfo = jlib:iq_query_info(mongoose_acc:element(Acc)), - maybe_jingle_stanza(QueryInfo, Acc) + QueryInfo = jlib:iq_query_info(Packet), + maybe_jingle_stanza(QueryInfo, From, To, Acc) end. -maybe_jingle_stanza(#iq{xmlns = ?JINGLE_NS, sub_el = Jingle, type = set} = IQ, Acc) -> +maybe_jingle_stanza(#iq{xmlns = ?JINGLE_NS, sub_el = Jingle, type = set} = IQ, From, To, Acc) -> JingleAction = exml_query:attr(Jingle, <<"action">>), - From = mongoose_acc:from_jid(Acc), - To = mongoose_acc:to_jid(Acc), maybe_translate_to_sip(JingleAction, From, To, IQ, Acc); -maybe_jingle_stanza(_, Acc) -> +maybe_jingle_stanza(_, _, _, Acc) -> {ok, Acc}. maybe_jingle_get_stanza_to_self(#iq{xmlns = ?JINGLE_NS, sub_el = Jingle, type = get} = IQ, Acc) -> diff --git a/src/mod_amp.erl b/src/mod_amp.erl index ed4cd969b8b..1188487bcc6 100644 --- a/src/mod_amp.erl +++ b/src/mod_amp.erl @@ -9,7 +9,7 @@ -behaviour(mongoose_module_metrics). -xep([{xep, 79}, {version, "1.2"}, {comment, "partially implemented."}]). -export([start/2, stop/1, supported_features/0]). --export([user_send_packet/3, +-export([user_send_message/3, check_packet/2, disco_local_features/3, c2s_stream_features/3, @@ -35,7 +35,7 @@ supported_features() -> [dynamic_domains]. -spec c2s_hooks(mongooseim:host_type()) -> gen_hook:hook_list(mongoose_c2s_hooks:hook_fn()). c2s_hooks(HostType) -> - [{user_send_packet, HostType, fun ?MODULE:user_send_packet/3, #{}, 5}]. + [{user_send_message, HostType, fun ?MODULE:user_send_message/3, #{}, 5}]. hooks(HostType) -> [ @@ -49,13 +49,11 @@ hooks(HostType) -> %% API --spec user_send_packet(mongoose_acc:t(), mongoose_c2s_hooks:hook_params(), gen_hook:extra()) -> - gen_hook:hook_fn_ret(mongoose_acc:t()). -user_send_packet(Acc, _, _) -> - case mongoose_acc:stanza_name(Acc) of - <<"message">> -> run_initial_check(Acc); - _ -> {ok, Acc} - end. +-spec user_send_message(mongoose_acc:t(), mongoose_c2s_hooks:hook_params(), gen_hook:extra()) -> + mongoose_c2s_hooks:hook_result(). +user_send_message(Acc, _, _) -> + {From, To, Element} = mongoose_acc:packet(Acc), + run_initial_check(Acc, From, To, Element). -spec check_packet(mongoose_acc:t(), amp_event()) -> mongoose_acc:t(). check_packet(Acc, Event) -> @@ -89,13 +87,11 @@ xmpp_send_element(Acc, _Params, _Extra) -> _ -> delivery_failed end, {ok, check_packet(Acc, Event)}. -%% Internal --spec run_initial_check(mongoose_acc:t()) -> gen_hook:hook_fn_ret(mongoose_acc:t()). -run_initial_check(Acc) -> - Packet = mongoose_acc:element(Acc), - From = mongoose_acc:from_jid(Acc), - To = mongoose_acc:to_jid(Acc), +%% Internal +-spec run_initial_check(mongoose_acc:t(), jid:jid(), jid:jid(), exml:element()) -> + mongoose_c2s_hooks:hook_result(). +run_initial_check(Acc, From, To, Packet) -> Result = case amp:extract_requested_rules(Packet) of none -> nothing_to_do; {rules, Rules} -> validate_and_process_rules(Packet, From, Rules, Acc);