From 15095791d8d85b9f07074c2e1b57d254a9f264cd Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Wed, 12 May 2021 13:12:25 +0200 Subject: [PATCH 01/13] MAM should happen early This way, the mam_id can be stored in the accumulator and reused by carbons, which is a requirement by MAM and Carbons XEPs. It could also be used by Inbox. --- src/mam/mod_mam.erl | 4 ++-- src/mam/mod_mam_muc.erl | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/mam/mod_mam.erl b/src/mam/mod_mam.erl index 9037b48076..7d9728feab 100644 --- a/src/mam/mod_mam.erl +++ b/src/mam/mod_mam.erl @@ -675,8 +675,8 @@ config_metrics(Host) -> -spec hooks(jid:lserver()) -> [ejabberd_hooks:hook()]. hooks(Host) -> - [{user_send_packet, Host, ?MODULE, user_send_packet, 90}, - {rest_user_send_packet, Host, ?MODULE, user_send_packet, 90}, + [{user_send_packet, Host, ?MODULE, user_send_packet, 60}, + {rest_user_send_packet, Host, ?MODULE, user_send_packet, 60}, {filter_local_packet, Host, ?MODULE, filter_packet, 90}, {remove_user, Host, ?MODULE, remove_user, 50}, {anonymous_purge_hook, Host, ?MODULE, remove_user, 50}, diff --git a/src/mam/mod_mam_muc.erl b/src/mam/mod_mam_muc.erl index ad91345a29..edf4e09ed5 100644 --- a/src/mam/mod_mam_muc.erl +++ b/src/mam/mod_mam_muc.erl @@ -604,7 +604,7 @@ is_archivable_message(MUCHost, Dir, Packet) -> -spec hooks(jid:lserver(), jid:lserver()) -> [ejabberd_hooks:hook()]. hooks(Host, MUCHost) -> - [{filter_room_packet, MUCHost, ?MODULE, filter_room_packet, 90}, + [{filter_room_packet, MUCHost, ?MODULE, filter_room_packet, 60}, {forget_room, MUCHost, ?MODULE, forget_room, 90}, {get_personal_data, Host, ?MODULE, get_personal_data, 50} | mongoose_metrics_mam_hooks:get_mam_muc_hooks(Host)]. From 83389c42cb10c52c8c38f1ac0df3834e10cf4855 Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Fri, 14 May 2021 18:23:08 +0200 Subject: [PATCH 02/13] Make ejabberd_sm:store_info like maps:put --- src/ejabberd_c2s.erl | 14 +++++++------- src/ejabberd_sm.erl | 15 +++++++-------- src/event_pusher/mod_event_pusher_push.erl | 2 +- src/mod_carboncopy.erl | 18 +++++++----------- 4 files changed, 22 insertions(+), 27 deletions(-) diff --git a/src/ejabberd_c2s.erl b/src/ejabberd_c2s.erl index 1711ec8ef6..56445636f2 100644 --- a/src/ejabberd_c2s.erl +++ b/src/ejabberd_c2s.erl @@ -40,7 +40,7 @@ get_subscription/2, get_subscribed/1, send_filtered/5, - store_session_info/3, + store_session_info/4, remove_session_info/3, get_info/1, run_remote_hook/3, @@ -152,8 +152,8 @@ terminate_session(#jid{} = Jid, Reason) -> terminate_session(Pid, Reason) when is_pid(Pid) -> Pid ! {exit, Reason}. -store_session_info(FsmRef, JID, KV) -> - FsmRef ! {store_session_info, JID, KV, self()}. +store_session_info(FsmRef, JID, Key, Value) -> + FsmRef ! {store_session_info, JID, Key, Value, self()}. remove_session_info(FsmRef, JID, Key) -> FsmRef ! {remove_session_info, JID, Key, self()}. @@ -1112,8 +1112,8 @@ handle_info(check_buffer_full, StateName, StateData) -> fsm_next_state(StateName, StateData#state{stream_mgmt_constraint_check_tref = undefined}) end; -handle_info({store_session_info, JID, KV, _FromPid}, StateName, StateData) -> - ejabberd_sm:store_info(JID, KV), +handle_info({store_session_info, JID, Key, Value, _FromPid}, StateName, StateData) -> + ejabberd_sm:store_info(JID, Key, Value), fsm_next_state(StateName, StateData); handle_info({remove_session_info, JID, Key, _FromPid}, StateName, StateData) -> ejabberd_sm:remove_info(JID, Key), @@ -2510,8 +2510,8 @@ bounce_messages(UnreadMessages) -> {ok, {route, From, To, Acc}, RemainedUnreadMessages} -> ejabberd_router:route(From, To, Acc), bounce_messages(RemainedUnreadMessages); - {ok, {store_session_info, JID, KV, _FromPid}, _} -> - ejabberd_sm:store_info(JID, KV); + {ok, {store_session_info, JID, Key, Value, _FromPid}, _} -> + ejabberd_sm:store_info(JID, Key, Value); {ok, _, RemainedUnreadMessages} -> % ignore this one, get the next message bounce_messages(RemainedUnreadMessages); diff --git a/src/ejabberd_sm.erl b/src/ejabberd_sm.erl index 4a682ba0cd..13d7f304a4 100644 --- a/src/ejabberd_sm.erl +++ b/src/ejabberd_sm.erl @@ -35,7 +35,7 @@ make_new_sid/0, open_session/4, open_session/5, close_session/4, - store_info/2, + store_info/3, get_info/2, remove_info/2, check_in_subscription/5, @@ -100,7 +100,6 @@ -type backend() :: ejabberd_sm_mnesia | ejabberd_sm_redis. -type close_reason() :: resumed | normal | replaced. -type info_key() :: atom(). --type info_item() :: {info_key(), any()}. -export_type([session/0, sid/0, @@ -224,9 +223,9 @@ close_session(Acc, SID, JID, Reason) -> ejabberd_gen_sm:delete_session(sm_backend(), SID, LUser, LServer, LResource), mongoose_hooks:sm_remove_connection_hook(Acc, SID, JID, Info, Reason). --spec store_info(jid:jid(), info_item()) -> - {ok, {any(), any()}} | {error, offline}. -store_info(JID, {Key, Value} = KV) -> +-spec store_info(jid:jid(), info_key(), any()) -> + {ok, any()} | {error, offline}. +store_info(JID, Key, Value) -> case get_session(JID) of offline -> {error, offline}; #session{sid = SID, priority = SPriority, info = SInfo} -> @@ -235,12 +234,12 @@ store_info(JID, {Key, Value} = KV) -> %% It's safe to allow process update its own record update_session(SID, JID, SPriority, maps:put(Key, Value, SInfo)), - {ok, KV}; + {ok, Key}; {_, Pid} -> %% Ask the process to update its record itself %% Async operation - ejabberd_c2s:store_session_info(Pid, JID, KV), - {ok, KV} + ejabberd_c2s:store_session_info(Pid, JID, Key, Value), + {ok, Key} end end. diff --git a/src/event_pusher/mod_event_pusher_push.erl b/src/event_pusher/mod_event_pusher_push.erl index c4f407cea0..97dec47302 100644 --- a/src/event_pusher/mod_event_pusher_push.erl +++ b/src/event_pusher/mod_event_pusher_push.erl @@ -264,7 +264,7 @@ enable_node(From, BarePubSubJID, Node, FormFields, IQ) -> -spec store_session_info(jid:jid(), publish_service()) -> any(). store_session_info(Jid, Service) -> - ejabberd_sm:store_info(Jid, {?SESSION_KEY, Service}). + ejabberd_sm:store_info(Jid, ?SESSION_KEY, Service). -spec maybe_remove_push_node_from_sessions_info(jid:jid(), jid:jid(), pubsub_node() | undefined) -> ok. diff --git a/src/mod_carboncopy.erl b/src/mod_carboncopy.erl index 2f1c1ea9c9..1a54740a6d 100644 --- a/src/mod_carboncopy.erl +++ b/src/mod_carboncopy.erl @@ -47,7 +47,6 @@ -define(NS_CC_2, <<"urn:xmpp:carbons:2">>). -define(NS_CC_1, <<"urn:xmpp:carbons:1">>). -define(CC_KEY, 'cc'). --define(CC_DISABLED, undefined). -include("mongoose.hrl"). -include("jlib.hrl"). @@ -133,7 +132,7 @@ user_receive_packet(Acc, JID, _From, To, Packet) -> check_and_forward(JID, To, #xmlel{name = <<"message">>} = Packet, Direction) -> case classify_packet(Packet) of ignore -> stop; - forward -> send_copies(JID, To, Packet, Direction) + forward -> send_copies(Acc, JID, To, Packet, Direction) end; check_and_forward(_JID, _To, _Packet, _) -> ok. @@ -269,20 +268,17 @@ carbon_copy_children(?NS_CC_2, JID, Packet, Direction) -> enable(JID, CC) -> ?LOG_INFO(#{what => cc_enable, user => JID#jid.luser, server => JID#jid.lserver}), - KV = {?CC_KEY, cc_ver_to_int(CC)}, - case ejabberd_sm:store_info(JID, KV) of - {ok, KV} -> ok; + case ejabberd_sm:store_info(JID, ?CC_KEY, cc_ver_to_int(CC)) of + {ok, ?CC_KEY} -> ok; {error, _} = Err -> Err end. disable(JID) -> ?LOG_INFO(#{what => cc_disable, user => JID#jid.luser, server => JID#jid.lserver}), - KV = {?CC_KEY, ?CC_DISABLED}, - case ejabberd_sm:store_info(JID, KV) of - {error, offline} -> ok; - {ok, KV} -> ok; - Err -> {error, Err} + case ejabberd_sm:remove_info(JID, ?CC_KEY) of + ok -> ok; + {error, offline} -> ok end. complete_packet(From, #xmlel{name = <<"message">>, attrs = OrigAttrs} = Packet, sent) -> @@ -311,7 +307,7 @@ filter_cc_enabled_resources(AllSessions) -> fun_filter_cc_enabled_resource(Session = #session{usr = {_, _, R}}) -> case mongoose_session:get_info(Session, ?CC_KEY, undefined) of - {?CC_KEY, V} when is_integer(V) andalso V =/= ?CC_DISABLED -> + {?CC_KEY, V} when is_integer(V) -> {true, {cc_ver_from_int(V), R}}; _ -> false From 82b6487b740d861bf51a63dfe24eb14f4114ce74 Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Fri, 14 May 2021 18:23:46 +0200 Subject: [PATCH 03/13] Place Carbon namespaces in mongoose_ns.hrl --- include/mongoose_ns.hrl | 5 +++++ src/mod_carboncopy.erl | 2 -- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/include/mongoose_ns.hrl b/include/mongoose_ns.hrl index 7dbca515b5..bd833b7866 100644 --- a/include/mongoose_ns.hrl +++ b/include/mongoose_ns.hrl @@ -61,6 +61,11 @@ -define(NS_PUSH, <<"urn:xmpp:push:0">>). % Push Notifications v0.2.1 -define(NS_STANZAID, <<"urn:xmpp:sid:0">>). +-define(NS_HINTS, <<"urn:xmpp:hints">>). +-define(NS_CC_RULES, <<"urn:xmpp:carbons:rules:0">>). +-define(NS_CC_2, <<"urn:xmpp:carbons:2">>). +-define(NS_CC_1, <<"urn:xmpp:carbons:1">>). + -define(NS_RSM, <<"http://jabber.org/protocol/rsm">>). -define(NS_EJABBERD_CONFIG, <<"ejabberd:config">>). diff --git a/src/mod_carboncopy.erl b/src/mod_carboncopy.erl index 1a54740a6d..f656cea6b5 100644 --- a/src/mod_carboncopy.erl +++ b/src/mod_carboncopy.erl @@ -44,8 +44,6 @@ remove_connection/5 ]). --define(NS_CC_2, <<"urn:xmpp:carbons:2">>). --define(NS_CC_1, <<"urn:xmpp:carbons:1">>). -define(CC_KEY, 'cc'). -include("mongoose.hrl"). From 754476e3f453a706074a8c71bc078fd558eacaf8 Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Fri, 14 May 2021 18:25:19 +0200 Subject: [PATCH 04/13] Rearrange carbons test suite for readability --- big_tests/tests/carboncopy_SUITE.erl | 134 ++++++++++++++------------- 1 file changed, 70 insertions(+), 64 deletions(-) diff --git a/big_tests/tests/carboncopy_SUITE.erl b/big_tests/tests/carboncopy_SUITE.erl index 82ace7e348..906faa15f7 100644 --- a/big_tests/tests/carboncopy_SUITE.erl +++ b/big_tests/tests/carboncopy_SUITE.erl @@ -1,18 +1,19 @@ -module(carboncopy_SUITE). - -compile([export_all]). -include_lib("common_test/include/ct.hrl"). -include_lib("proper/include/proper.hrl"). -include_lib("eunit/include/eunit.hrl"). -define(AE(Expected, Actual), ?assertEqual(Expected, Actual)). +-define(BODY, <<"And pious action">>). -all() -> [{group, all}]. +all() -> + [{group, all}]. groups() -> - G = [{all, [parallel, shuffle], + G = [{all, [parallel], [discovering_support, enabling_carbons, disabling_carbons, @@ -28,20 +29,33 @@ groups() -> ]}], ct_helper:repeat_all_until_all_ok(G). -init_per_suite(C) -> escalus:init_per_suite(C). -end_per_suite(C) -> escalus_fresh:clean(), escalus:end_per_suite(C). -init_per_testcase(Name, C) -> escalus:init_per_testcase(Name, C). -end_per_testcase(Name, C) -> escalus:end_per_testcase(Name, C). -run_prop(PropName, Property) -> - ?AE(true, proper:quickcheck(proper:conjunction([{PropName, Property}]), - [verbose, long_result, {numtests, 3}])). +%%%=================================================================== +%%% Overall setup/teardown +%%%=================================================================== +init_per_suite(C) -> + escalus:init_per_suite(C). + +end_per_suite(C) -> + escalus_fresh:clean(), + escalus:end_per_suite(C). + +%%%=================================================================== +%%% Testcase specific setup/teardown +%%%=================================================================== +init_per_testcase(Name, C) -> + escalus:init_per_testcase(Name, C). + +end_per_testcase(Name, C) -> + escalus:end_per_testcase(Name, C). +%%%=================================================================== +%%% Individual Test Cases (from groups() definition) +%%%=================================================================== discovering_support(Config) -> escalus:fresh_story( Config, [{alice, 1}], fun(Alice) -> - IqGet = escalus_stanza:disco_info(ct:get_config({hosts, - mim, domain})), + IqGet = escalus_stanza:disco_info(ct:get_config({hosts, mim, domain})), escalus_client:send(Alice, IqGet), Result = escalus_client:wait_for_stanza(Alice), escalus:assert(is_iq_result, [IqGet], Result), @@ -49,55 +63,50 @@ discovering_support(Config) -> end). enabling_carbons(Config) -> - escalus:fresh_story(Config, [{alice, 1}], fun carbons_get_enabled/1). + escalus:fresh_story(Config, [{alice, 1}], fun enable_carbons/1). disabling_carbons(Config) -> escalus:fresh_story(Config, [{alice, 1}], - fun(Alice) -> carbons_get_enabled(Alice), - carbons_get_disabled(Alice) end). + fun(Alice) -> enable_carbons(Alice), + disable_carbons(Alice) end). avoiding_carbons(Config) -> escalus:fresh_story( Config, [{alice, 2}, {bob, 1}], fun(Alice1, Alice2, Bob) -> - carbons_get_enabled([Alice1, Alice2]), - Msg = escalus_stanza:chat_without_carbon_to(Bob, - <<"And pious action">>), + enable_carbons([Alice1, Alice2]), + Msg = escalus_stanza:chat_without_carbon_to(Bob, ?BODY), escalus_client:send(Alice1, Msg), - escalus:assert( - is_chat_message, [<<"And pious action">>], - escalus_client:wait_for_stanza(Bob)), - escalus_client:wait_for_stanzas(Alice2, 1), - [] = escalus_client:peek_stanzas(Alice2) + BobReceived = escalus_client:wait_for_stanza(Bob), + escalus:assert(is_chat_message, [?BODY], BobReceived), + ?assertEqual([], escalus_client:wait_for_stanzas(Alice2, 1, 500)), + ?assertEqual([], escalus_client:peek_stanzas(Alice2)) end). non_enabled_clients_dont_get_sent_carbons(Config) -> escalus:fresh_story( Config, [{alice, 2}, {bob, 1}], fun(Alice1, Alice2, Bob) -> - Msg = escalus_stanza:chat_to(Bob, <<"And pious action">>), + Msg = escalus_stanza:chat_to(Bob, ?BODY), escalus_client:send(Alice1, Msg), - escalus:assert( - is_chat_message, [<<"And pious action">>], - escalus_client:wait_for_stanza(Bob)), - escalus_client:wait_for_stanzas(Alice2, 1), - [] = escalus_client:peek_stanzas(Alice2) + BobReceived = escalus_client:wait_for_stanza(Bob), + escalus:assert(is_chat_message, [?BODY], BobReceived), + ?assertEqual([], escalus_client:wait_for_stanzas(Alice2, 1, 500)), + ?assertEqual([], escalus_client:peek_stanzas(Alice2)) end). non_enabled_clients_dont_get_received_carbons(Config) -> escalus:fresh_story( Config, [{alice, 2}, {bob, 1}], fun(Alice1, Alice2, Bob) -> - Msg = escalus_stanza:chat_to(Alice1, <<"And pious action">>), + Msg = escalus_stanza:chat_to(Alice1, ?BODY), escalus_client:send(Bob, Msg), - escalus:assert( - is_chat_message, [<<"And pious action">>], - escalus_client:wait_for_stanza(Alice1)), - escalus_client:wait_for_stanzas(Alice2, 1), - [] = escalus_client:peek_stanzas(Alice2) + AliceReceived = escalus_client:wait_for_stanza(Alice1), + escalus:assert(is_chat_message, [?BODY], AliceReceived), + ?assertEqual([], escalus_client:wait_for_stanzas(Alice2, 1, 500)), + ?assertEqual([], escalus_client:peek_stanzas(Alice2)) end). - enabled_single_resource_doesnt_get_carbons(Config) -> BobsMessages = [ <<"There's such a thing as dwelling">>, @@ -108,7 +117,7 @@ enabled_single_resource_doesnt_get_carbons(Config) -> escalus:fresh_story( Config, [{alice, 1}, {bob, 1}], fun(Alice, Bob) -> - carbons_get_enabled(Alice), + enable_carbons(Alice), [ escalus_client:send(Bob, escalus_stanza:chat_to(Alice, M)) || M <- BobsMessages ], [ escalus:assert(is_chat_message, [M], escalus_client:wait_for_stanza(Alice)) @@ -119,38 +128,33 @@ unavailable_resources_dont_get_carbons(Config) -> escalus:fresh_story( Config, [{alice, 2}, {bob, 1}], fun(Alice1, Alice2, Bob) -> - carbons_get_enabled(Alice1), - carbons_get_enabled(Alice2), + enable_carbons([Alice1, Alice2]), client_unsets_presence(Alice1), escalus:assert(is_presence_with_type, [<<"unavailable">>], escalus_client:wait_for_stanza(Alice2)), - escalus_client:send(Bob, escalus_stanza:chat_to(Alice2, <<"one">>)), + escalus_client:send(Bob, escalus_stanza:chat_to(Alice2, ?BODY)), client_sets_presence(Alice1), %% no carbons for Alice1, only presences escalus_new_assert:mix_match([is_presence, is_presence], - escalus:wait_for_stanzas(Alice1, 2)) + escalus:wait_for_stanzas(Alice1, 2)), + AliceReceived = escalus_client:wait_for_stanza(Alice2), + escalus:assert(is_chat_message, [?BODY], AliceReceived) end). -client_unsets_presence(Client) -> - escalus_client:send(Client, escalus_stanza:presence(<<"unavailable">>)). - -client_sets_presence(Client) -> - escalus_client:send(Client, escalus_stanza:presence(<<"available">>)). - dropped_client_doesnt_create_duplicate_carbons(Config) -> escalus:fresh_story( Config, [{alice, 2}, {bob, 1}], fun(Alice1, Alice2, Bob) -> - Msg = escalus_stanza:chat_to(Bob, <<"And pious action">>), + enable_carbons([Alice1, Alice2]), + Msg = escalus_stanza:chat_to(Bob, ?BODY), escalus_client:stop(Config, Alice2), escalus:assert(is_presence_with_type, [<<"unavailable">>], - escalus_client:wait_for_stanza(Alice1)), + escalus_client:wait_for_stanza(Alice1)), escalus_client:send(Alice1, Msg), - escalus:assert( - is_chat_message, [<<"And pious action">>], - escalus_client:wait_for_stanza(Bob)), + escalus:assert(is_chat_message, [?BODY], + escalus_client:wait_for_stanza(Bob)), [] = escalus_client:peek_stanzas(Alice1) end). @@ -188,7 +192,6 @@ prop_normal_routing_to_bare_jid(Config) -> end))). - %% %% Test scenarios w/assertions %% @@ -196,7 +199,7 @@ prop_normal_routing_to_bare_jid(Config) -> all_bobs_resources_get_message_to_bare_jid([Alice, Bob1 | Bobs], Msg) -> %% All connected resources receive messages sent %% to the user's bare JID without carbon wrappers. - carbons_get_enabled([Bob1|Bobs]), + enable_carbons([Bob1|Bobs]), escalus_client:send( Alice, escalus_stanza:chat_to(escalus_client:short_jid(Bob1), Msg)), GotMsg = fun(BobsResource) -> @@ -209,7 +212,7 @@ all_bobs_resources_get_message_to_bare_jid([Alice, Bob1 | Bobs], Msg) -> lists:foreach(GotMsg, [Bob1|Bobs]). all_bobs_other_resources_get_received_carbons([Alice, Bob1 | Bobs], Msg) -> - carbons_get_enabled([Bob1|Bobs]), + enable_carbons([Bob1|Bobs]), escalus_client:send(Alice, escalus_stanza:chat_to(Bob1, Msg)), GotForward = fun(BobsResource) -> escalus:assert( @@ -222,7 +225,7 @@ all_bobs_other_resources_get_received_carbons([Alice, Bob1 | Bobs], Msg) -> lists:foreach(GotForward, Bobs). all_bobs_other_resources_get_sent_carbons([Alice, Bob1 | Bobs], Msg) -> - carbons_get_enabled([Bob1|Bobs]), + enable_carbons([Bob1|Bobs]), escalus_client:send(Bob1, escalus_stanza:chat_to(Alice, Msg)), escalus:assert(is_chat_message, [Msg], escalus_client:wait_for_stanza(Alice)), GotCarbon = fun(BobsResource) -> @@ -235,20 +238,13 @@ all_bobs_other_resources_get_sent_carbons([Alice, Bob1 | Bobs], Msg) -> escalus_assert:has_no_stanzas(BobsResource) end, lists:foreach(GotCarbon, Bobs). -carbons_get_disabled(ClientOrClients) -> - disable_carbons(ClientOrClients). - -carbons_get_enabled(ClientOrClients) -> - enable_carbons(ClientOrClients). - - %% %% Internal helpers %% %% Wrapper around escalus:story. Returns PropEr result. true_story(Config, UserSpecs, TestFun) -> - try escalus_fresh:story_with_client_list(Config, UserSpecs, TestFun), true + try escalus_fresh:story_with_client_list(Config, UserSpecs, TestFun), true catch E -> {error, E} end. @@ -283,3 +279,13 @@ disable_carbons(Client) -> escalus_client:send(Client, IqSet), Result = escalus_client:wait_for_stanza(Client), escalus:assert(is_iq, [<<"result">>], Result). + +client_unsets_presence(Client) -> + escalus_client:send(Client, escalus_stanza:presence(<<"unavailable">>)). + +client_sets_presence(Client) -> + escalus_client:send(Client, escalus_stanza:presence(<<"available">>)). + +run_prop(PropName, Property) -> + ?AE(true, proper:quickcheck(proper:conjunction([{PropName, Property}]), + [verbose, long_result, {numtests, 3}])). From b0982ca3de53c2f4b62e948940a2847e72b679f8 Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Tue, 18 May 2021 22:53:45 +0200 Subject: [PATCH 05/13] Fix carbon tests to always have more than one resource --- big_tests/tests/carboncopy_SUITE.erl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/big_tests/tests/carboncopy_SUITE.erl b/big_tests/tests/carboncopy_SUITE.erl index 906faa15f7..28f7b3a7d7 100644 --- a/big_tests/tests/carboncopy_SUITE.erl +++ b/big_tests/tests/carboncopy_SUITE.erl @@ -214,6 +214,7 @@ all_bobs_resources_get_message_to_bare_jid([Alice, Bob1 | Bobs], Msg) -> all_bobs_other_resources_get_received_carbons([Alice, Bob1 | Bobs], Msg) -> enable_carbons([Bob1|Bobs]), escalus_client:send(Alice, escalus_stanza:chat_to(Bob1, Msg)), + escalus_client:wait_for_stanza(Bob1), GotForward = fun(BobsResource) -> escalus:assert( is_forwarded_received_message, @@ -250,7 +251,7 @@ true_story(Config, UserSpecs, TestFun) -> end. %% Number of resources per users -no_of_resources() -> rand:uniform(4). +no_of_resources() -> 1 + rand:uniform(4). %% A sample chat message utterance() -> From 8f54444e308a488236d1df2e4a6aa36c7dc05fb7 Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Fri, 14 May 2021 18:45:05 +0200 Subject: [PATCH 06/13] Pass Acc along the carbon callstack --- src/mod_carboncopy.erl | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/src/mod_carboncopy.erl b/src/mod_carboncopy.erl index f656cea6b5..14eb687f7d 100644 --- a/src/mod_carboncopy.erl +++ b/src/mod_carboncopy.erl @@ -88,13 +88,14 @@ stop(Host) -> config_spec() -> #section{items = #{<<"iqdisc">> => mongoose_config_spec:iqdisc()}}. -iq_handler2(From, To, Acc, IQ) -> - iq_handler(From, To, Acc, IQ, ?NS_CC_2). -iq_handler1(From, To, Acc, IQ) -> - iq_handler(From, To, Acc, IQ, ?NS_CC_1). - -iq_handler(From, _To, Acc, #iq{type = set, sub_el = #xmlel{name = Operation, - children = []}} = IQ, CC) -> +iq_handler2(From, _To, Acc, IQ) -> + iq_handler(Acc, From, IQ, ?NS_CC_2). +iq_handler1(From, _To, Acc, IQ) -> + iq_handler(Acc, From, IQ, ?NS_CC_1). + +iq_handler(Acc, From, #iq{type = set, + sub_el = #xmlel{name = Operation, + children = []}} = IQ, CC) -> ?LOG_DEBUG(#{what => cc_iq_received, acc => Acc}), Result = case Operation of <<"enable">> -> @@ -105,21 +106,21 @@ iq_handler(From, _To, Acc, #iq{type = set, sub_el = #xmlel{name = Operation, case Result of ok -> ?LOG_DEBUG(#{what => cc_iq_result, acc => Acc}), - {Acc, IQ#iq{type=result, sub_el=[]}}; + {Acc, IQ#iq{type = result, sub_el = []}}; {error, Reason} -> ?LOG_WARNING(#{what => cc_iq_failed, acc => Acc, reason => Reason}), - {Acc, IQ#iq{type=error, sub_el = [mongoose_xmpp_errors:bad_request()]}} + {Acc, IQ#iq{type = error, sub_el = [mongoose_xmpp_errors:not_allowed()]}} end; -iq_handler(_From, _To, Acc, IQ, _CC) -> - {Acc, IQ#iq{type=error, sub_el = [mongoose_xmpp_errors:not_allowed()]}}. +iq_handler(Acc, _From, IQ, _CC) -> + {Acc, IQ#iq{type = error, sub_el = [mongoose_xmpp_errors:bad_request()]}}. user_send_packet(Acc, From, To, Packet) -> - check_and_forward(From, To, Packet, sent), + check_and_forward(Acc, From, To, Packet, sent), Acc. user_receive_packet(Acc, JID, _From, To, Packet) -> - check_and_forward(JID, To, Packet, received), + check_and_forward(Acc, JID, To, Packet, received), Acc. % Check if the traffic is local. @@ -127,13 +128,13 @@ user_receive_packet(Acc, JID, _From, To, Packet) -> % - registered to the user_send_packet hook, to be called only once even for multicast % - do not support "private" message mode, and do not modify the original packet in any way % - we also replicate "read" notifications -check_and_forward(JID, To, #xmlel{name = <<"message">>} = Packet, Direction) -> +-spec check_and_forward(mongoose_acc:t(), jid:jid(), jid:jid(), exml:element(), sent | received) -> ok | stop. +check_and_forward(Acc, JID, To, #xmlel{name = <<"message">>} = Packet, Direction) -> case classify_packet(Packet) of ignore -> stop; forward -> send_copies(Acc, JID, To, Packet, Direction) end; - -check_and_forward(_JID, _To, _Packet, _) -> ok. +check_and_forward(_Acc, _JID, _To, _Packet, _) -> ok. -spec classify_packet(_) -> classification(). classify_packet(Packet) -> From bd4b695097f3d151bddd80d543c6234542dc5b37 Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Tue, 18 May 2021 22:52:44 +0200 Subject: [PATCH 07/13] Upgrade Carbons to 0.13.3 Add, extend, and fix, carbon unit-tests --- include/mongoose_ns.hrl | 1 + src/mod_carboncopy.erl | 146 +++++++++++++++---------- test/carboncopy_proper_tests_SUITE.erl | 93 +++++++++++----- 3 files changed, 155 insertions(+), 85 deletions(-) diff --git a/include/mongoose_ns.hrl b/include/mongoose_ns.hrl index bd833b7866..1cf4197093 100644 --- a/include/mongoose_ns.hrl +++ b/include/mongoose_ns.hrl @@ -60,6 +60,7 @@ -define(NS_HTTP_UPLOAD_030, <<"urn:xmpp:http:upload:0">>). -define(NS_PUSH, <<"urn:xmpp:push:0">>). % Push Notifications v0.2.1 -define(NS_STANZAID, <<"urn:xmpp:sid:0">>). +-define(NS_RECEIPTS, <<"urn:xmpp:receipts">>). -define(NS_HINTS, <<"urn:xmpp:hints">>). -define(NS_CC_RULES, <<"urn:xmpp:carbons:rules:0">>). diff --git a/src/mod_carboncopy.erl b/src/mod_carboncopy.erl index 14eb687f7d..49f3709879 100644 --- a/src/mod_carboncopy.erl +++ b/src/mod_carboncopy.erl @@ -25,7 +25,8 @@ %%%---------------------------------------------------------------------- -module (mod_carboncopy). -author ('ecestari@process-one.net'). --xep([{xep, 280}, {version, "0.10"}]). +-xep([{xep, 280}, {version, "0.6"}]). +-xep([{xep, 280}, {version, "0.13.3"}]). -behaviour(gen_mod). -behaviour(mongoose_module_metrics). @@ -33,8 +34,7 @@ -export([start/2, stop/1, config_spec/0, - is_carbon_copy/1, - classify_packet/1]). + is_carbon_copy/1]). %% Hooks -export([user_send_packet/4, @@ -44,6 +44,9 @@ remove_connection/5 ]). +%% Tests +-export([should_forward/3]). + -define(CC_KEY, 'cc'). -include("mongoose.hrl"). @@ -51,7 +54,7 @@ -include("session.hrl"). -include("mongoose_config_spec.hrl"). --type classification() :: 'ignore' | 'forward'. +-type direction() :: sent | received. is_carbon_copy(Packet) -> case xml:get_subtag(Packet, <<"sent">>) of @@ -123,69 +126,102 @@ user_receive_packet(Acc, JID, _From, To, Packet) -> check_and_forward(Acc, JID, To, Packet, received), Acc. +remove_connection(Acc, LUser, LServer, LResource, _Status) -> + JID = jid:make_noprep(LUser, LServer, LResource), + disable(JID), + Acc. + % Check if the traffic is local. % Modified from original version: % - registered to the user_send_packet hook, to be called only once even for multicast % - do not support "private" message mode, and do not modify the original packet in any way % - we also replicate "read" notifications --spec check_and_forward(mongoose_acc:t(), jid:jid(), jid:jid(), exml:element(), sent | received) -> ok | stop. +-spec check_and_forward(mongoose_acc:t(), jid:jid(), jid:jid(), exml:element(), direction()) -> ok | stop. check_and_forward(Acc, JID, To, #xmlel{name = <<"message">>} = Packet, Direction) -> - case classify_packet(Packet) of - ignore -> stop; - forward -> send_copies(Acc, JID, To, Packet, Direction) + case should_forward(Packet, To, Direction) of + false -> stop; + true -> send_copies(Acc, JID, To, Packet, Direction) end; check_and_forward(_Acc, _JID, _To, _Packet, _) -> ok. --spec classify_packet(_) -> classification(). -classify_packet(Packet) -> - is_chat(Packet). - --spec is_chat(_) -> classification(). -is_chat(#xmlel{name = <<"message">>, attrs = Attrs} = Packet) -> - case xml:get_attr_s(<<"type">>, Attrs) of - <<"chat">> -> is_private(Packet); - _ -> ignore - end. - --spec is_private(_) -> classification(). -is_private(Packet) -> - case xml:get_subtag(Packet, <<"private">>) of - false -> is_no_copy(Packet); - _ -> ignore - end. - --spec is_no_copy(_) -> classification(). -is_no_copy(Packet) -> - case xml:get_subtag(Packet, <<"no-copy">>) of - false -> is_received(Packet); - _ -> ignore +%%%=================================================================== +%%% Classification +%%%=================================================================== + +-spec should_forward(exml:element(), jid:jid(), direction()) -> boolean(). +should_forward(Packet, To, Direction) -> + (not is_carbon_private(Packet)) andalso + (not has_nocopy_hint(Packet)) andalso + (not is_received(Packet)) andalso + (not is_sent(Packet)) andalso + (is_chat(Packet) orelse is_valid_muc(Packet, To, Direction)). + +-spec is_chat(exml:element()) -> boolean(). +is_chat(Packet) -> + case exml_query:attr(Packet, <<"type">>, <<"normal">>) of + <<"normal">> -> contains_body(Packet) orelse + contains_receipts(Packet) orelse + contains_csn(Packet); + <<"chat">> -> true; + _ -> false end. --spec is_received(_) -> classification(). +-spec is_valid_muc(exml:element(), jid:jid(), direction()) -> boolean(). +is_valid_muc(_, _, sent) -> + false; +is_valid_muc(Packet, To, _) -> + is_mediated_invitation(Packet) orelse + is_direct_muc_invitation(Packet) orelse + is_received_private_muc(Packet, To). + +-spec is_mediated_invitation(exml:element()) -> boolean(). +is_mediated_invitation(Packet) -> + undefined =/= exml_query:path(Packet, + [{element_with_ns, <<"x">>, ?NS_MUC_USER}, + {element, <<"invite">>}, + {attr, <<"from">>}]). + +-spec is_direct_muc_invitation(exml:element()) -> boolean(). +is_direct_muc_invitation(Packet) -> + undefined =/= exml_query:subelement_with_name_and_ns(Packet, <<"x">>, ?NS_CONFERENCE). + +-spec is_received_private_muc(exml:element(), jid:jid()) -> boolean(). +is_received_private_muc(_, #jid{lresource = <<>>}) -> + false; +is_received_private_muc(Packet, _) -> + undefined =/= exml_query:subelement_with_name_and_ns(Packet, <<"x">>, ?NS_MUC_USER). + +-spec is_carbon_private(exml:element()) -> boolean(). +is_carbon_private(Packet) -> + undefined =/= exml_query:subelement_with_name_and_ns(Packet, <<"private">>, ?NS_CC_2). + +-spec has_nocopy_hint(exml:element()) -> boolean(). +has_nocopy_hint(Packet) -> + undefined =/= exml_query:subelement_with_name_and_ns(Packet, <<"no-copy">>, ?NS_HINTS). + +-spec contains_body(exml:element()) -> boolean(). +contains_body(Packet) -> + undefined =/= exml_query:subelement(Packet, <<"body">>). + +-spec contains_receipts(exml:element()) -> boolean(). +contains_receipts(Packet) -> + undefined =/= exml_query:subelement_with_name_and_ns(Packet, <<"received">>, ?NS_RECEIPTS). + +-spec contains_csn(exml:element()) -> boolean(). +contains_csn(Packet) -> + undefined =/= exml_query:subelement_with_ns(Packet, ?NS_CHATSTATES). + +-spec is_received(exml:element()) -> boolean(). is_received(Packet) -> - case xml:get_subtag(Packet, <<"received">>) of - false -> is_sent(Packet); - _ -> ignore - end. + undefined =/= exml_query:subelement_with_name_and_ns(Packet, <<"received">>, ?NS_CC_2). --spec is_sent(_) -> classification(). +-spec is_sent(exml:element()) -> boolean(). is_sent(Packet) -> - case xml:get_subtag(Packet, <<"sent">>) of - false -> forward; - SubTag -> is_forwarded(SubTag) - end. + undefined =/= exml_query:subelement_with_name_and_ns(Packet, <<"sent">>, ?NS_CC_2). --spec is_forwarded(_) -> classification(). -is_forwarded(SubTag) -> - case xml:get_subtag(SubTag, <<"forwarded">>) of - false -> forward; - _ -> ignore - end. - -remove_connection(Acc, LUser, LServer, LResource, _Status) -> - JID = jid:make_noprep(LUser, LServer, LResource), - disable(JID), - Acc. +%%%=================================================================== +%%% Internal +%%%=================================================================== %% @@ -214,12 +250,6 @@ jids_minus_specific_resource(JID, R, CCResList, _PrioRes) -> [ {jid:replace_resource(JID, CCRes), CCVersion} || {CCVersion, CCRes} <- CCResList, CCRes =/= R ]. -%% If the original user is the only resource in the list of targets -%% that means that he/she must have already received the message via -%% normal routing: -drop_singleton_jid(JID, [{JID, _CCVER}]) -> []; -drop_singleton_jid(_JID, Targets) -> Targets. - %% Direction = received | sent send_copies(JID, To, Packet, Direction) -> #jid{lresource = R} = JID, diff --git a/test/carboncopy_proper_tests_SUITE.erl b/test/carboncopy_proper_tests_SUITE.erl index 3680f45b9e..1c9c1bbb06 100644 --- a/test/carboncopy_proper_tests_SUITE.erl +++ b/test/carboncopy_proper_tests_SUITE.erl @@ -19,50 +19,73 @@ all() -> [{group, mod_message_carbons_proper_tests}]. all_tests() -> - [chat_type_test, private_message_test, no_copy_type_test, - received_type_test, sent_forwarded_type_test, sent_message_test, - simple_chat_message_test, simple_badarg_test]. + [ + private_message_test, + no_copy_type_test, + empty_message_test, + received_type_test, + sent_message_test, + simple_badarg_test, + simple_chat_message_test, + has_chat_state_notifications, + has_delivery_receipts, + is_muc_invitation, + is_direct_invitation + ]. groups() -> - [{mod_message_carbons_proper_tests, [sequence], all_tests()}]. + [{mod_message_carbons_proper_tests, [parallel], all_tests()}]. private_message_test(_) -> property(private_message_test, ?FORALL(Msg, private_carbon_message(), - ignore == mod_carboncopy:classify_packet(Msg))). - -chat_type_test(_) -> - property(chat_type_test, ?FORALL(Msg, non_chat_message(), - ignore == mod_carboncopy:classify_packet(Msg))). + false == mod_carboncopy:should_forward(Msg, alice(), received))). no_copy_type_test(_) -> property(no_copy_type_test, ?FORALL(Msg, no_copy_message(), - ignore == mod_carboncopy:classify_packet(Msg))). + false == mod_carboncopy:should_forward(Msg, alice(), received))). + +empty_message_test(_) -> + property(empty_message_test, ?FORALL(Msg, non_chat_message(), + false == mod_carboncopy:should_forward(Msg, alice(), received))). received_type_test(_) -> property(received_type_test, ?FORALL(Msg, received_message(), - ignore == mod_carboncopy:classify_packet(Msg))). - -sent_forwarded_type_test(_) -> - property(sent_forwarded_type_test, ?FORALL(Msg, sent_forwarded_message(), - ignore == mod_carboncopy:classify_packet(Msg))). + false == mod_carboncopy:should_forward(Msg, alice(), received))). sent_message_test(_) -> property(sent_message_test, ?FORALL(Msg, sent_message(), - forward == mod_carboncopy:classify_packet(Msg))). + false == mod_carboncopy:should_forward(Msg, alice(), received))). + +simple_badarg_test(_) -> + property(simple_badarg_test, ?FORALL(Msg, badarg_message(), + false == mod_carboncopy:should_forward(Msg, alice(), received))). simple_chat_message_test(_) -> property(simple_chat_message_test, ?FORALL(Msg, simple_chat_message(), - forward == mod_carboncopy:classify_packet(Msg))). + true == mod_carboncopy:should_forward(Msg, alice(), received))). -simple_badarg_test(_) -> - property(simple_badarg_test, ?FORALL(Msg, badarg_message(), - ignore == mod_carboncopy:classify_packet(Msg))). +has_chat_state_notifications(_) -> + property(has_chat_state_notifications, ?FORALL(Msg, chat_state_notification(), + true == mod_carboncopy:should_forward(Msg, alice(), received))). + +has_delivery_receipts(_) -> + property(has_delivery_receipts, ?FORALL(Msg, delivery_receipt(), + true == mod_carboncopy:should_forward(Msg, alice(), received))). + +is_muc_invitation(_) -> + property(is_muc_invitation, ?FORALL(Msg, muc_invitation(), + true == mod_carboncopy:should_forward(Msg, alice(), received))). + +is_direct_invitation(_) -> + property(is_direct_invitation, ?FORALL(Msg, direct_invitation(), + true == mod_carboncopy:should_forward(Msg, alice(), received))). property(Name, Prop) -> Props = proper:conjunction([{Name, Prop}]), true = proper:quickcheck(Props, [verbose, long_result, {numtests, 50}]). - +alice() -> + jid:make_noprep(<<"alice">>, <<"localhost">>, <<>>). %% %% Generators @@ -79,18 +102,13 @@ private_carbon_message() -> no_copy_message() -> xmlel("message", [{<<"type">>,<<"chat">>}], - [xmlel("no-copy",[{<<"xmlns">>, <<"urn:xmpp:carbons:2">>}],[])]). + [xmlel("no-copy",[{<<"xmlns">>, <<"urn:xmpp:hints">>}],[])]). received_message() -> xmlel("message", [{<<"type">>,<<"chat">>}], [xmlel("received",[{<<"xmlns">>, <<"urn:xmpp:carbons:2">>}],[])]). -sent_forwarded_message() -> - xmlel("message", - [{<<"type">>,<<"chat">>}], - [xmlel("sent",[{<<"xmlns">>, <<"urn:xmpp:carbons:2">>}],[xmlel("forwarded",[],[])])]). - sent_message() -> xmlel("message", [{<<"type">>,<<"chat">>}], @@ -99,5 +117,26 @@ sent_message() -> simple_chat_message() -> xmlel("message", [{<<"type">>,<<"chat">>}],[]). +chat_state_notification() -> + xmlel("message", + [{<<"type">>,<<"chat">>}], + [xmlel("someelement",[{<<"xmlns">>, <<"http://jabber.org/protocol/chatstates">>}],[])]). + +delivery_receipt() -> + xmlel("message", + [{<<"type">>,<<"chat">>}], + [xmlel("received",[{<<"xmlns">>, <<"urn:xmpp:receipts">>}],[])]). + +muc_invitation() -> + xmlel("message", + [{<<"type">>,<<"chat">>}], + [xmlel("x",[{<<"xmlns">>, <<"jabber:x:conference">>}], + [xmlel("invite", [<<"from">>], [<<"alice@localhost">>])])]). + +direct_invitation() -> + xmlel("message", + [{<<"type">>,<<"chat">>}], + [xmlel("x",[{<<"xmlns">>, <<"jabber:x:conference">>}],[])]). + badarg_message() -> xmlel("message", [{<<"type">>,<<"123">>}],[]). From f7eae778624a006ad7285de5f83104352ca2def0 Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Wed, 19 May 2021 00:36:41 +0200 Subject: [PATCH 08/13] Add MAM-ID to the accumulators --- big_tests/tests/mam_SUITE.erl | 40 ++++++++++++++++++++-- src/mam/mod_mam.erl | 52 +++++++++++++++-------------- src/mod_carboncopy.erl | 63 +++++++++++++++++++++-------------- 3 files changed, 103 insertions(+), 52 deletions(-) diff --git a/big_tests/tests/mam_SUITE.erl b/big_tests/tests/mam_SUITE.erl index 9abffab9d8..ba5b0c9648 100644 --- a/big_tests/tests/mam_SUITE.erl +++ b/big_tests/tests/mam_SUITE.erl @@ -115,7 +115,8 @@ archive_chat_markers/1, dont_archive_chat_markers/1, save_unicode_messages/1, - unicode_messages_can_be_extracted/1]). + unicode_messages_can_be_extracted/1, + stanza_id_is_appended_to_carbons/1]). -import(distributed_helper, [mim/0, require_rpc_nodes/1, @@ -394,7 +395,8 @@ archived_cases() -> filter_forwarded]. stanzaid_cases() -> - [message_with_stanzaid]. + [message_with_stanzaid, + stanza_id_is_appended_to_carbons]. retract_cases() -> [retract_message, @@ -1499,6 +1501,32 @@ save_unicode_messages(Config) -> end, escalus_fresh:story(Config, [{alice, 1}, {bob, 1}], F). +stanza_id_is_appended_to_carbons(Config) -> + F = fun(Alice1, Alice2, Bob1, Bob2) -> + Msg = <<"OH, HAI!">>, + enable_carbons([Alice1, Alice2, Bob1, Bob2]), + escalus:send(Alice1, escalus_stanza:chat_to(Bob1, Msg)), + mam_helper:wait_for_archive_size(Alice1, 1), + escalus_client:wait_for_stanza(Bob1), + Alice2CC = escalus_client:wait_for_stanza(Alice2), + Bob2CC = escalus_client:wait_for_stanza(Bob2), + + SID = fun(Packet, Direction) -> + exml_query:path(Packet, [{element_with_ns, Direction, <<"urn:xmpp:carbons:2">>}, + {element_with_ns, <<"forwarded">>, <<"urn:xmpp:forward:0">>}, + {element_with_ns, <<"message">>, <<"jabber:client">>}, + {element_with_ns, <<"stanza-id">>, <<"urn:xmpp:sid:0">>}, + {attr, <<"id">>}]) + end, + ?assert_equal(true, undefined =/= SID(Bob2CC, <<"received">>)), + ?assert_equal(true, undefined =/= SID(Alice2CC, <<"sent">>)), + escalus:assert(is_forwarded_sent_message, + [escalus_client:full_jid(Alice1), escalus_client:full_jid(Bob1), Msg], Alice2CC), + escalus:assert(is_forwarded_received_message, + [escalus_client:full_jid(Alice1), escalus_client:full_jid(Bob1), Msg], Bob2CC) + end, + escalus_fresh:story(Config, [{alice, 2}, {bob, 2}], F). + muc_text_search_request(Config) -> P = ?config(props, Config), F = fun(Alice, Bob) -> @@ -3132,3 +3160,11 @@ origin_id_to_retract(Config) -> origin_id() -> <<"orig-id-1">>. + +enable_carbons(Clients) when is_list(Clients) -> + lists:foreach(fun enable_carbons/1, Clients); +enable_carbons(Client) -> + IqSet = escalus_stanza:carbons_enable(), + escalus_client:send(Client, IqSet), + Result = escalus_client:wait_for_stanza(Client), + escalus:assert(is_iq, [<<"result">>], Result). diff --git a/src/mam/mod_mam.erl b/src/mam/mod_mam.erl index 7d9728feab..654154fda2 100644 --- a/src/mam/mod_mam.erl +++ b/src/mam/mod_mam.erl @@ -75,12 +75,9 @@ [maybe_add_arcid_elems/4, wrap_message/6, result_set/4, - result_query/2, result_prefs/4, - make_fin_message/5, make_fin_element/4, parse_prefs/1, - borders_decode/1, is_mam_result_message/1, features/2]). @@ -264,8 +261,8 @@ process_mam_iq(From=#jid{lserver=Host}, To, Acc, IQ) -> Packet :: exml:element()) -> mongoose_acc:t(). user_send_packet(Acc, From, To, Packet) -> ?LOG_DEBUG(#{what => mam_user_send_packet, acc => Acc}), - handle_package(outgoing, false, From, To, From, Packet), - Acc. + {_, Acc2} = handle_package(outgoing, true, From, To, From, Packet, Acc), + Acc2. %% @doc Handle an incoming message. @@ -282,28 +279,31 @@ user_send_packet(Acc, From, To, Packet) -> -spec filter_packet(Value :: fpacket() | drop) -> fpacket() | drop. filter_packet(drop) -> drop; -filter_packet({From, To = #jid{lserver = LServer}, Acc, Packet}) -> - ?LOG_DEBUG(#{what => mam_user_receive_packet, acc => Acc}), - {AmpEvent, PacketAfterArchive} = +filter_packet({From, To = #jid{lserver = LServer}, Acc1, Packet}) -> + ?LOG_DEBUG(#{what => mam_user_receive_packet, acc => Acc1}), + {AmpEvent, PacketAfterArchive, Acc3} = case ejabberd_users:does_user_exist(To) of false -> - {mam_failed, Packet}; + {mam_failed, Packet, Acc1}; true -> - case process_incoming_packet(From, To, Packet) of - undefined -> {mam_failed, Packet}; - MessID -> {archived, maybe_add_arcid_elems( - To, MessID, Packet, - mod_mam_params:add_stanzaid_element(?MODULE, LServer))} + case process_incoming_packet(From, To, Packet, Acc1) of + {undefined, Acc2} -> + {mam_failed, Packet, Acc2}; + {MessID, Acc2} -> + Packet2 = maybe_add_arcid_elems( + To, MessID, Packet, + mod_mam_params:add_stanzaid_element(?MODULE, LServer)), + {archived, Packet2, Acc2} end end, - Acc1 = mongoose_acc:update_stanza(#{ element => PacketAfterArchive, + Acc4 = mongoose_acc:update_stanza(#{ element => PacketAfterArchive, from_jid => From, - to_jid => To }, Acc), - Acc2 = mod_amp:check_packet(Acc1, AmpEvent), - {From, To, Acc2, mongoose_acc:element(Acc2)}. + to_jid => To }, Acc3), + Acc5 = mod_amp:check_packet(Acc4, AmpEvent), + {From, To, Acc5, mongoose_acc:element(Acc5)}. -process_incoming_packet(From, To, Packet) -> - handle_package(incoming, true, To, From, From, Packet). +process_incoming_packet(From, To, Packet, Acc) -> + handle_package(incoming, true, To, From, From, Packet, Acc). %% hook handler -spec remove_user(mongoose_acc:t(), jid:user(), jid:server()) -> mongoose_acc:t(). @@ -467,11 +467,12 @@ amp_deliver_strategy([direct, none]) -> [direct, stored, none]. -spec handle_package(Dir :: incoming | outgoing, ReturnMessID :: boolean(), LocJID :: jid:jid(), RemJID :: jid:jid(), SrcJID :: jid:jid(), - Packet :: exml:element()) -> MaybeMessID :: binary() | undefined. + Packet :: exml:element(), Acc :: mongoose_acc:t()) -> + {MaybeMessID :: binary() | undefined, Acc :: mongoose_acc:t()}. handle_package(Dir, ReturnMessID, LocJID = #jid{}, RemJID = #jid{}, - SrcJID = #jid{}, Packet) -> + SrcJID = #jid{}, Packet, Acc) -> Host = server_host(LocJID), case is_archivable_message(Host, Dir, Packet) andalso should_archive_if_groupchat(Host, exml_query:attr(Packet, <<"type">>)) of @@ -490,12 +491,13 @@ handle_package(Dir, ReturnMessID, direction => Dir, packet => Packet}, Result = archive_message(Host, Params), - return_external_message_id_if_ok(ReturnMessID, Result, MessID); + ExtMessId = return_external_message_id_if_ok(ReturnMessID, Result, MessID), + {ExtMessId, mongoose_acc:set(mam, mam_id, ExtMessId, Acc)}; false -> - undefined + {undefined, Acc} end; false -> - undefined + {undefined, Acc} end. should_archive_if_groupchat(Host, <<"groupchat">>) -> diff --git a/src/mod_carboncopy.erl b/src/mod_carboncopy.erl index 49f3709879..fabb418882 100644 --- a/src/mod_carboncopy.erl +++ b/src/mod_carboncopy.erl @@ -251,7 +251,7 @@ jids_minus_specific_resource(JID, R, CCResList, _PrioRes) -> || {CCVersion, CCRes} <- CCResList, CCRes =/= R ]. %% Direction = received | sent -send_copies(JID, To, Packet, Direction) -> +send_copies(Acc, JID, To, Packet, Direction) -> #jid{lresource = R} = JID, {PrioRes, CCResList} = get_cc_enabled_resources(JID), Targets = case is_bare_to(Direction, To, PrioRes) of @@ -262,37 +262,36 @@ send_copies(JID, To, Packet, Direction) -> ?LOG_DEBUG(#{what => cc_send_copies, targets => Targets, resources => PrioRes, ccenabled => CCResList}), lists:foreach(fun({Dest, Version}) -> - #jid{lresource = Resource} = JID, - ?LOG_DEBUG(#{what => cc_forwarding, - user => JID#jid.luser, server => JID#jid.lserver, - resource => Resource, exml_packet => Packet}), - Sender = jid:replace_resource(JID, <<>>), - New = build_forward_packet - (JID, Packet, Sender, Dest, Direction, Version), - ejabberd_router:route(Sender, Dest, New) - end, drop_singleton_jid(JID, Targets)), - ok. - -build_forward_packet(JID, Packet, Sender, Dest, Direction, Version) -> + ?LOG_DEBUG(#{what => cc_forwarding, + user => JID#jid.luser, server => JID#jid.lserver, + resource => JID#jid.lresource, exml_packet => Packet}), + Sender = jid:to_bare(JID), + New = build_forward_packet(Acc, JID, Packet, Sender, Dest, Direction, Version), + ejabberd_router:route(Sender, Dest, New) + end, Targets). + +build_forward_packet(Acc, JID, Packet, Sender, Dest, Direction, Version) -> + % The wrapping message SHOULD maintain the same 'type' attribute value; + Type = exml_query:attr(Packet, <<"type">>, <<"normal">>), #xmlel{name = <<"message">>, attrs = [{<<"xmlns">>, <<"jabber:client">>}, - {<<"type">>, <<"chat">>}, + {<<"type">>, Type}, {<<"from">>, jid:to_binary(Sender)}, {<<"to">>, jid:to_binary(Dest)}], - children = carbon_copy_children(Version, JID, Packet, Direction)}. + children = carbon_copy_children(Acc, Version, JID, Packet, Direction)}. -carbon_copy_children(?NS_CC_1, JID, Packet, Direction) -> - [ #xmlel{name = list_to_binary(atom_to_list(Direction)), +carbon_copy_children(Acc, ?NS_CC_1, JID, Packet, Direction) -> + [ #xmlel{name = atom_to_binary(Direction, utf8), attrs = [{<<"xmlns">>, ?NS_CC_1}]}, #xmlel{name = <<"forwarded">>, attrs = [{<<"xmlns">>, ?NS_FORWARD}], - children = [complete_packet(JID, Packet, Direction)]} ]; -carbon_copy_children(?NS_CC_2, JID, Packet, Direction) -> - [ #xmlel{name = list_to_binary(atom_to_list(Direction)), + children = [complete_packet(Acc, JID, Packet, Direction)]} ]; +carbon_copy_children(Acc, ?NS_CC_2, JID, Packet, Direction) -> + [ #xmlel{name = atom_to_binary(Direction, utf8), attrs = [{<<"xmlns">>, ?NS_CC_2}], children = [ #xmlel{name = <<"forwarded">>, attrs = [{<<"xmlns">>, ?NS_FORWARD}], - children = [complete_packet(JID, Packet, Direction)]} ]} ]. + children = [complete_packet(Acc, JID, Packet, Direction)]} ]} ]. enable(JID, CC) -> ?LOG_INFO(#{what => cc_enable, @@ -310,18 +309,19 @@ disable(JID) -> {error, offline} -> ok end. -complete_packet(From, #xmlel{name = <<"message">>, attrs = OrigAttrs} = Packet, sent) -> +complete_packet(Acc, From, #xmlel{name = <<"message">>, attrs = OrigAttrs} = Packet, sent) -> %% if this is a packet sent by user on this host, then Packet doesn't %% include the 'from' attribute. We must add it. Attrs = lists:keystore(<<"xmlns">>, 1, OrigAttrs, {<<"xmlns">>, <<"jabber:client">>}), + Packet2 = set_stanza_id(Acc, From, Packet), case proplists:get_value(<<"from">>, Attrs) of undefined -> - Packet#xmlel{attrs = [{<<"from">>, jid:to_binary(From)}|Attrs]}; + Packet2#xmlel{attrs = [{<<"from">>, jid:to_binary(From)} | Attrs]}; _ -> - Packet#xmlel{attrs = Attrs} + Packet2#xmlel{attrs = Attrs} end; -complete_packet(_From, #xmlel{name = <<"message">>, attrs=OrigAttrs} = Packet, received) -> +complete_packet(_Acc, _From, #xmlel{name = <<"message">>, attrs = OrigAttrs} = Packet, received) -> Attrs = lists:keystore(<<"xmlns">>, 1, OrigAttrs, {<<"xmlns">>, <<"jabber:client">>}), Packet#xmlel{attrs = Attrs}. @@ -356,3 +356,16 @@ cc_ver_to_int(?NS_CC_2) -> 2. cc_ver_from_int(1) -> ?NS_CC_1; cc_ver_from_int(2) -> ?NS_CC_2. + +%% Servers SHOULD include the element as a child +%% of the forwarded message when using Message Carbons (XEP-0280) +%% https://xmpp.org/extensions/xep-0313.html#archives_id +set_stanza_id(Acc, From, Packet) -> + MamId = mongoose_acc:get(mam, mam_id, undefined, Acc), + set_stanza_id(MamId, From, Acc, Packet). + +set_stanza_id(undefined, _From, _Acc, Packet) -> + Packet; +set_stanza_id(MamId, From, _Acc, Packet) -> + By = jid:to_binary(jid:to_bare(From)), + mod_mam_utils:replace_arcid_elem(<<"stanza-id">>, By, MamId, Packet). From 92535ef36e2606d771c6b486389bb10a90cea06a Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Wed, 19 May 2021 11:24:18 +0200 Subject: [PATCH 09/13] big_tests reuse enable disable carbons --- big_tests/tests/carboncopy_SUITE.erl | 20 ++------------------ big_tests/tests/mam_SUITE.erl | 10 +--------- big_tests/tests/mongoose_helper.erl | 17 +++++++++++++++++ 3 files changed, 20 insertions(+), 27 deletions(-) diff --git a/big_tests/tests/carboncopy_SUITE.erl b/big_tests/tests/carboncopy_SUITE.erl index 28f7b3a7d7..7ba04c0d79 100644 --- a/big_tests/tests/carboncopy_SUITE.erl +++ b/big_tests/tests/carboncopy_SUITE.erl @@ -8,6 +8,7 @@ -define(AE(Expected, Actual), ?assertEqual(Expected, Actual)). -define(BODY, <<"And pious action">>). +-import(mongoose_helper, [enable_carbons/1, disable_carbons/1]). all() -> [{group, all}]. @@ -63,7 +64,7 @@ discovering_support(Config) -> end). enabling_carbons(Config) -> - escalus:fresh_story(Config, [{alice, 1}], fun enable_carbons/1). + escalus:fresh_story(Config, [{alice, 1}], fun mongoose_helper:enable_carbons/1). disabling_carbons(Config) -> escalus:fresh_story(Config, [{alice, 1}], @@ -264,23 +265,6 @@ utterance() -> <<"Long withering out a young man revenue.">>]). -enable_carbons(Clients) when is_list(Clients) -> - lists:foreach(fun enable_carbons/1, Clients); -enable_carbons(Client) -> - IqSet = escalus_stanza:carbons_enable(), - escalus_client:send(Client, IqSet), - Result = escalus_client:wait_for_stanza(Client), - escalus:assert(is_iq, [<<"result">>], Result). - - -disable_carbons(Clients) when is_list(Clients) -> - lists:foreach(fun disable_carbons/1, Clients); -disable_carbons(Client) -> - IqSet = escalus_stanza:carbons_disable(), - escalus_client:send(Client, IqSet), - Result = escalus_client:wait_for_stanza(Client), - escalus:assert(is_iq, [<<"result">>], Result). - client_unsets_presence(Client) -> escalus_client:send(Client, escalus_stanza:presence(<<"unavailable">>)). diff --git a/big_tests/tests/mam_SUITE.erl b/big_tests/tests/mam_SUITE.erl index ba5b0c9648..f668e1b1cb 100644 --- a/big_tests/tests/mam_SUITE.erl +++ b/big_tests/tests/mam_SUITE.erl @@ -1504,7 +1504,7 @@ save_unicode_messages(Config) -> stanza_id_is_appended_to_carbons(Config) -> F = fun(Alice1, Alice2, Bob1, Bob2) -> Msg = <<"OH, HAI!">>, - enable_carbons([Alice1, Alice2, Bob1, Bob2]), + mongoose_helper:enable_carbons([Alice1, Alice2, Bob1, Bob2]), escalus:send(Alice1, escalus_stanza:chat_to(Bob1, Msg)), mam_helper:wait_for_archive_size(Alice1, 1), escalus_client:wait_for_stanza(Bob1), @@ -3160,11 +3160,3 @@ origin_id_to_retract(Config) -> origin_id() -> <<"orig-id-1">>. - -enable_carbons(Clients) when is_list(Clients) -> - lists:foreach(fun enable_carbons/1, Clients); -enable_carbons(Client) -> - IqSet = escalus_stanza:carbons_enable(), - escalus_client:send(Client, IqSet), - Result = escalus_client:wait_for_stanza(Client), - escalus:assert(is_iq, [<<"result">>], Result). diff --git a/big_tests/tests/mongoose_helper.erl b/big_tests/tests/mongoose_helper.erl index 2ec8f9c8b6..9cf2ed2c99 100644 --- a/big_tests/tests/mongoose_helper.erl +++ b/big_tests/tests/mongoose_helper.erl @@ -23,6 +23,7 @@ -export([ensure_muc_clean/0]). -export([successful_rpc/3, successful_rpc/4, successful_rpc/5]). -export([logout_user/2, logout_user/3]). +-export([enable_carbons/1, disable_carbons/1]). -export([wait_until/2, wait_until/3, wait_for_user/3]). -export([inject_module/1, inject_module/2, inject_module/3]). @@ -261,6 +262,22 @@ successful_rpc(#{} = Spec, Module, Function, Args, Timeout) -> Result end. +enable_carbons(Clients) when is_list(Clients) -> + lists:foreach(fun enable_carbons/1, Clients); +enable_carbons(Client) -> + IqSet = escalus_stanza:carbons_enable(), + escalus_client:send(Client, IqSet), + Result = escalus_client:wait_for_stanza(Client), + escalus:assert(is_iq, [<<"result">>], Result). + +disable_carbons(Clients) when is_list(Clients) -> + lists:foreach(fun disable_carbons/1, Clients); +disable_carbons(Client) -> + IqSet = escalus_stanza:carbons_disable(), + escalus_client:send(Client, IqSet), + Result = escalus_client:wait_for_stanza(Client), + escalus:assert(is_iq, [<<"result">>], Result). + logout_user(Config, User) -> Node = distributed_helper:mim(), logout_user(Config, User, Node). From ce1e8c7ca1c0bdf230aa96a3df7d6aab7e067a5e Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Wed, 19 May 2021 19:10:08 +0200 Subject: [PATCH 10/13] Update exml and base16 packages --- rebar.config | 4 ++-- rebar.lock | 10 ++++------ 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/rebar.config b/rebar.config index 2da843b5c8..2b79684b71 100644 --- a/rebar.config +++ b/rebar.config @@ -30,8 +30,8 @@ %% git config --global url."git://github".insteadOf https://github {deps, [ - {base16, {git, "https://github.com/esl/base16.git", {tag, "1.1.0"}}}, - {exml, "3.0.4", {pkg, hexml}}, + {base16, "2.0.0"}, + {exml, "3.0.5", {pkg, hexml}}, {lager, "3.8.0"}, % We're keeping this to override the dependencies {cowboy, "2.7.0"}, {exometer_core, {git, "https://github.com/esl/exometer_core.git", {branch, "master"}}}, diff --git a/rebar.lock b/rebar.lock index 6b5bb81074..f3d42ed363 100644 --- a/rebar.lock +++ b/rebar.lock @@ -1,10 +1,7 @@ {"1.1.0", [{<<"amqp_client">>,{pkg,<<"amqp_client">>,<<"3.8.4">>},0}, {<<"backoff">>,{pkg,<<"backoff">>,<<"1.1.3">>},1}, - {<<"base16">>, - {git,"https://github.com/esl/base16.git", - {ref,"3e046e9667a5e8579cd9004e00c6e2537f5e044f"}}, - 0}, + {<<"base16">>,{pkg,<<"base16">>,<<"2.0.0">>},0}, {<<"bbmustache">>,{pkg,<<"bbmustache">>,<<"1.10.0">>},0}, {<<"cache_tab">>,{pkg,<<"cache_tab">>,<<"1.0.22">>},0}, {<<"certifi">>,{pkg,<<"certifi">>,<<"2.3.1">>},2}, @@ -40,7 +37,7 @@ 0}, {<<"erlang_pmp">>,{pkg,<<"erlang_pmp">>,<<"0.1.1">>},0}, {<<"erlcloud">>,{pkg,<<"erlcloud">>,<<"3.3.1">>},0}, - {<<"exml">>,{pkg,<<"hexml">>,<<"3.0.4">>},0}, + {<<"exml">>,{pkg,<<"hexml">>,<<"3.0.5">>},0}, {<<"exometer_core">>, {git,"https://github.com/esl/exometer_core.git", {ref,"979ff04bcabc276c122b47fb7e6b54fbded62576"}}, @@ -167,6 +164,7 @@ {pkg_hash,[ {<<"amqp_client">>, <<"31F1DF5495D452F321C0275C7263A18C3BA38E1DA486CFC7271BE84F4A84AF01">>}, {<<"backoff">>, <<"DE762C05ED6DFAE862D83DC9E58AE936792B01302B3595F5CFFE86F2D8E6C1DD">>}, + {<<"base16">>, <<"9DA694FA0778DF31522A1B9EB349BA4AC9063B497234DC49509C1F5C37F336A2">>}, {<<"bbmustache">>, <<"DDC927463F0E95D66CDAC889153AF08015D609124D6D79006C248AD2DE7F6ECD">>}, {<<"cache_tab">>, <<"AD16577E7F26709CACDCB86E6A4960C8D158CAB9D189CDF49CC1E2DC33106A70">>}, {<<"certifi">>, <<"D0F424232390BF47D82DA8478022301C561CF6445B5B5FB6A84D49A9E76D2639">>}, @@ -180,7 +178,7 @@ {<<"eredis">>, <<"0B8E9CFC2C00FA1374CD107EA63B49BE08D933DF2CF175E6A89B73DD9C380DE4">>}, {<<"erlang_pmp">>, <<"42FAA89EAF427B71533444F2B433D0010837636149595658499C356A5A39589B">>}, {<<"erlcloud">>, <<"4DE8DC3CBA64A59D57E4A32606DB6AA5CAC371CA0ADB8361571B2E6A74295A56">>}, - {<<"exml">>, <<"43231F3BE52CC69CC7F4A694F2294F344FB6AF6F9DF20BA966D4C4E553E647D2">>}, + {<<"exml">>, <<"2A79262A744A9B100A9A27A8E76EADCF146F17653DABA2A88A74C8CF5B06ADF3">>}, {<<"fast_scram">>, <<"1F8F467D34374D00CE4AB8D6F78430EA8ADCCD2DA6D5BA0069B9EA8A1CAC6DE1">>}, {<<"gen_fsm_compat">>, <<"5903549F67D595F58A7101154CBE0FDD46955FBFBE40813F1E53C23A970FF5F4">>}, {<<"goldrush">>, <<"F06E5D5F1277DA5C413E84D5A2924174182FB108DABB39D5EC548B27424CD106">>}, From f7031216f16fae85713134078751f8f6bea3f039 Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Wed, 19 May 2021 19:21:52 +0200 Subject: [PATCH 11/13] Fix ejabberd_sm unit tests --- test/ejabberd_sm_SUITE.erl | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/test/ejabberd_sm_SUITE.erl b/test/ejabberd_sm_SUITE.erl index 1a141195b9..ee0d30b7f3 100644 --- a/test/ejabberd_sm_SUITE.erl +++ b/test/ejabberd_sm_SUITE.erl @@ -246,7 +246,7 @@ cannot_reproduce_race_condition_in_store_info(C) -> ok = try_to_reproduce_race_condition(C). store_info_sends_message_to_the_session_owner(C) -> - SID = {erlang:timestamp(), self()}, + SID = {erlang:system_time(microsecond), self()}, U = <<"alice2">>, S = <<"localhost">>, R = <<"res1">>, @@ -255,15 +255,15 @@ store_info_sends_message_to_the_session_owner(C) -> ?B(C):create_session(U, S, R, Session), %% but call store_info from another process JID = jid:make_noprep(U, S, R), - spawn_link(fun() -> ejabberd_sm:store_info(JID, {cc, undefined}) end), + spawn_link(fun() -> ejabberd_sm:store_info(JID, cc, undefined) end), %% The original process receives a message receive {store_session_info, #jid{luser = User, lserver = Server, lresource = Resource}, - KV, _FromPid} -> + K, V, _FromPid} -> ?eq(U, User), ?eq(S, Server), ?eq(R, Resource), - ?eq({cc, undefined}, KV), + ?eq({cc, undefined}, {K, V}), ok after 5000 -> ct:fail("store_info_sends_message_to_the_session_owner=timeout") @@ -412,9 +412,9 @@ given_session_opened(Sid, {U, S, R}, Priority, Info) -> when_session_opened(Sid, {U, S, R}, Priority, Info) -> given_session_opened(Sid, {U, S, R}, Priority, Info). -when_session_info_stored(U, S, R, {_,_}=KV) -> +when_session_info_stored(U, S, R, {K,V}) -> JID = jid:make_noprep(U, S, R), - ejabberd_sm:store_info(JID, KV). + ejabberd_sm:store_info(JID, K, V). when_session_info_removed(U, S, R, Key) -> JID = jid:make_noprep(U, S, R), @@ -608,4 +608,5 @@ set_meck(SMBackend) -> meck:new(ejabberd_commands, []), meck:expect(ejabberd_commands, register_commands, fun(_) -> ok end), + meck:expect(ejabberd_commands, unregister_commands, fun(_) -> ok end), ok. From ff6d21dee34f30acc670436b291596131db18b28 Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Thu, 20 May 2021 19:29:30 +0200 Subject: [PATCH 12/13] Apply review --- src/mod_carboncopy.erl | 1 + test/carboncopy_proper_tests_SUITE.erl | 62 +++++++++++++------------- test/ejabberd_sm_SUITE.erl | 2 +- 3 files changed, 33 insertions(+), 32 deletions(-) diff --git a/src/mod_carboncopy.erl b/src/mod_carboncopy.erl index fabb418882..8959898ec7 100644 --- a/src/mod_carboncopy.erl +++ b/src/mod_carboncopy.erl @@ -72,6 +72,7 @@ start(Host, Opts) -> IQDisc = gen_mod:get_opt(iqdisc, Opts, no_queue), mod_disco:register_feature(Host, ?NS_CC_1), mod_disco:register_feature(Host, ?NS_CC_2), + mod_disco:register_feature(Host, ?NS_CC_RULES), ejabberd_hooks:add(unset_presence_hook, Host, ?MODULE, remove_connection, 10), ejabberd_hooks:add(user_send_packet, Host, ?MODULE, user_send_packet, 89), ejabberd_hooks:add(user_receive_packet, Host, ?MODULE, user_receive_packet, 89), diff --git a/test/carboncopy_proper_tests_SUITE.erl b/test/carboncopy_proper_tests_SUITE.erl index 1c9c1bbb06..7e1950e93d 100644 --- a/test/carboncopy_proper_tests_SUITE.erl +++ b/test/carboncopy_proper_tests_SUITE.erl @@ -27,10 +27,10 @@ all_tests() -> sent_message_test, simple_badarg_test, simple_chat_message_test, - has_chat_state_notifications, - has_delivery_receipts, - is_muc_invitation, - is_direct_invitation + has_chat_state_notifications_test, + has_delivery_receipts_test, + is_muc_invitation_test, + is_direct_invitation_test ]. groups() -> @@ -64,20 +64,20 @@ simple_chat_message_test(_) -> property(simple_chat_message_test, ?FORALL(Msg, simple_chat_message(), true == mod_carboncopy:should_forward(Msg, alice(), received))). -has_chat_state_notifications(_) -> - property(has_chat_state_notifications, ?FORALL(Msg, chat_state_notification(), +has_chat_state_notifications_test(_) -> + property(has_chat_state_notifications_test, ?FORALL(Msg, chat_state_notification(), true == mod_carboncopy:should_forward(Msg, alice(), received))). -has_delivery_receipts(_) -> - property(has_delivery_receipts, ?FORALL(Msg, delivery_receipt(), +has_delivery_receipts_test(_) -> + property(has_delivery_receipts_test, ?FORALL(Msg, delivery_receipt(), true == mod_carboncopy:should_forward(Msg, alice(), received))). -is_muc_invitation(_) -> - property(is_muc_invitation, ?FORALL(Msg, muc_invitation(), +is_muc_invitation_test(_) -> + property(is_muc_invitation_test, ?FORALL(Msg, muc_invitation(), true == mod_carboncopy:should_forward(Msg, alice(), received))). -is_direct_invitation(_) -> - property(is_direct_invitation, ?FORALL(Msg, direct_invitation(), +is_direct_invitation_test(_) -> + property(is_direct_invitation_test, ?FORALL(Msg, direct_invitation(), true == mod_carboncopy:should_forward(Msg, alice(), received))). property(Name, Prop) -> @@ -92,51 +92,51 @@ alice() -> %% non_chat_message() -> - xmlel("message",[],[]). + xmlel("message", [], []). private_carbon_message() -> xmlel("message", - [{<<"type">>,<<"chat">>}], - [xmlel("private",[{<<"xmlns">>, <<"urn:xmpp:carbons:2">>}],[])]). + [{<<"type">>, <<"chat">>}], + [xmlel("private", [{<<"xmlns">>, <<"urn:xmpp:carbons:2">>}], [])]). no_copy_message() -> xmlel("message", - [{<<"type">>,<<"chat">>}], - [xmlel("no-copy",[{<<"xmlns">>, <<"urn:xmpp:hints">>}],[])]). + [{<<"type">>, <<"chat">>}], + [xmlel("no-copy", [{<<"xmlns">>, <<"urn:xmpp:hints">>}], [])]). received_message() -> xmlel("message", - [{<<"type">>,<<"chat">>}], - [xmlel("received",[{<<"xmlns">>, <<"urn:xmpp:carbons:2">>}],[])]). + [{<<"type">>, <<"chat">>}], + [xmlel("received", [{<<"xmlns">>, <<"urn:xmpp:carbons:2">>}], [])]). sent_message() -> xmlel("message", - [{<<"type">>,<<"chat">>}], - [xmlel("sent",[{<<"xmlns">>, <<"urn:xmpp:carbons:2">>}],[])]). + [{<<"type">>, <<"chat">>}], + [xmlel("sent", [{<<"xmlns">>, <<"urn:xmpp:carbons:2">>}], [])]). simple_chat_message() -> - xmlel("message", [{<<"type">>,<<"chat">>}],[]). + xmlel("message", [{<<"type">>, <<"chat">>}], []). chat_state_notification() -> xmlel("message", - [{<<"type">>,<<"chat">>}], - [xmlel("someelement",[{<<"xmlns">>, <<"http://jabber.org/protocol/chatstates">>}],[])]). + [{<<"type">>, <<"chat">>}], + [xmlel("someelement", [{<<"xmlns">>, <<"http://jabber.org/protocol/chatstates">>}], [])]). delivery_receipt() -> xmlel("message", - [{<<"type">>,<<"chat">>}], - [xmlel("received",[{<<"xmlns">>, <<"urn:xmpp:receipts">>}],[])]). + [{<<"type">>, <<"chat">>}], + [xmlel("received", [{<<"xmlns">>, <<"urn:xmpp:receipts">>}], [])]). muc_invitation() -> xmlel("message", - [{<<"type">>,<<"chat">>}], - [xmlel("x",[{<<"xmlns">>, <<"jabber:x:conference">>}], + [{<<"type">>, <<"chat">>}], + [xmlel("x", [{<<"xmlns">>, <<"jabber:x:conference">>}], [xmlel("invite", [<<"from">>], [<<"alice@localhost">>])])]). direct_invitation() -> xmlel("message", - [{<<"type">>,<<"chat">>}], - [xmlel("x",[{<<"xmlns">>, <<"jabber:x:conference">>}],[])]). + [{<<"type">>, <<"chat">>}], + [xmlel("x", [{<<"xmlns">>, <<"jabber:x:conference">>}], [])]). badarg_message() -> - xmlel("message", [{<<"type">>,<<"123">>}],[]). + xmlel("message", [{<<"type">>, <<"123">>}],[]). diff --git a/test/ejabberd_sm_SUITE.erl b/test/ejabberd_sm_SUITE.erl index ee0d30b7f3..7567487ae3 100644 --- a/test/ejabberd_sm_SUITE.erl +++ b/test/ejabberd_sm_SUITE.erl @@ -412,7 +412,7 @@ given_session_opened(Sid, {U, S, R}, Priority, Info) -> when_session_opened(Sid, {U, S, R}, Priority, Info) -> given_session_opened(Sid, {U, S, R}, Priority, Info). -when_session_info_stored(U, S, R, {K,V}) -> +when_session_info_stored(U, S, R, {K, V}) -> JID = jid:make_noprep(U, S, R), ejabberd_sm:store_info(JID, K, V). From 3a54add2dfe6a3550fa291a0b64317796db4151c Mon Sep 17 00:00:00 2001 From: Nelson Vides Date: Fri, 21 May 2021 10:07:10 +0200 Subject: [PATCH 13/13] Properly support CC versions 1 and 2 --- src/mod_carboncopy.erl | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/src/mod_carboncopy.erl b/src/mod_carboncopy.erl index 8959898ec7..20424fb2bf 100644 --- a/src/mod_carboncopy.erl +++ b/src/mod_carboncopy.erl @@ -192,10 +192,6 @@ is_received_private_muc(_, #jid{lresource = <<>>}) -> is_received_private_muc(Packet, _) -> undefined =/= exml_query:subelement_with_name_and_ns(Packet, <<"x">>, ?NS_MUC_USER). --spec is_carbon_private(exml:element()) -> boolean(). -is_carbon_private(Packet) -> - undefined =/= exml_query:subelement_with_name_and_ns(Packet, <<"private">>, ?NS_CC_2). - -spec has_nocopy_hint(exml:element()) -> boolean(). has_nocopy_hint(Packet) -> undefined =/= exml_query:subelement_with_name_and_ns(Packet, <<"no-copy">>, ?NS_HINTS). @@ -212,13 +208,28 @@ contains_receipts(Packet) -> contains_csn(Packet) -> undefined =/= exml_query:subelement_with_ns(Packet, ?NS_CHATSTATES). +-spec is_carbon_private(exml:element()) -> boolean(). +is_carbon_private(Packet) -> + [] =/= subelements_with_nss(Packet, <<"private">>, carbon_namespaces()). + -spec is_received(exml:element()) -> boolean(). is_received(Packet) -> - undefined =/= exml_query:subelement_with_name_and_ns(Packet, <<"received">>, ?NS_CC_2). + [] =/= subelements_with_nss(Packet, <<"received">>, carbon_namespaces()). -spec is_sent(exml:element()) -> boolean(). is_sent(Packet) -> - undefined =/= exml_query:subelement_with_name_and_ns(Packet, <<"sent">>, ?NS_CC_2). + [] =/= subelements_with_nss(Packet, <<"sent">>, carbon_namespaces()). + +-spec subelements_with_nss(exml:element(), binary(), [binary()]) -> [exml:element()]. +subelements_with_nss(#xmlel{children = Children}, Name, NSS) -> + lists:filter(fun(#xmlel{name = N} = Child) when N =:= Name -> + NS = exml_query:attr(Child, <<"xmlns">>), + lists:member(NS, NSS); + (_) -> + false + end, Children). + +carbon_namespaces() -> [?NS_CC_1, ?NS_CC_2]. %%%=================================================================== %%% Internal