From 74c01caf085db88257c09823afe937060a14d698 Mon Sep 17 00:00:00 2001 From: Holger Weiss Date: Sun, 12 Jan 2020 14:04:31 +0100 Subject: [PATCH] mod_carboncopy: Improve is_carbon_copy() check Make sure the hook chain is stopped early whenever a carbon copy is processed, not just in some cases. --- src/mod_carboncopy.erl | 47 +++++++++++++++--------------------------- 1 file changed, 17 insertions(+), 30 deletions(-) diff --git a/src/mod_carboncopy.erl b/src/mod_carboncopy.erl index 2377d04c1..c704f91c0 100644 --- a/src/mod_carboncopy.erl +++ b/src/mod_carboncopy.erl @@ -36,8 +36,7 @@ -export([user_send_packet/1, user_receive_packet/1, iq_handler/1, disco_features/5, - is_carbon_copy/1, depends/2, - mod_options/1, mod_doc/0]). + depends/2, mod_options/1, mod_doc/0]). -export([c2s_copy_session/2, c2s_session_opened/1, c2s_session_resumed/1]). %% For debugging purposes -export([list/2]). @@ -49,12 +48,6 @@ -type direction() :: sent | received. -type c2s_state() :: ejabberd_c2s:state(). --spec is_carbon_copy(stanza()) -> boolean(). -is_carbon_copy(#message{meta = #{carbon_copy := true}}) -> - true; -is_carbon_copy(_) -> - false. - start(Host, _Opts) -> ejabberd_hooks:add(disco_local_features, Host, ?MODULE, disco_features, 50), %% why priority 89: to define clearly that we must run BEFORE mod_logdb hook (90) @@ -113,22 +106,24 @@ iq_handler(#iq{type = get, lang = Lang} = IQ)-> -spec user_send_packet({stanza(), ejabberd_c2s:state()}) -> {stanza(), ejabberd_c2s:state()} | {stop, {stanza(), ejabberd_c2s:state()}}. +user_send_packet({#message{meta = #{carbon_copy := true}}, _C2SState} = Acc) -> + %% Stop the hook chain, we don't want logging modules to duplicate this + %% message. + {stop, Acc}; user_send_packet({Packet, C2SState}) -> From = xmpp:get_from(Packet), To = xmpp:get_to(Packet), - case check_and_forward(From, To, Packet, sent) of - {stop, Pkt} -> {stop, {Pkt, C2SState}}; - Pkt -> {Pkt, C2SState} - end. + {check_and_forward(From, To, Packet, sent), C2SState}. -spec user_receive_packet({stanza(), ejabberd_c2s:state()}) -> {stanza(), ejabberd_c2s:state()} | {stop, {stanza(), ejabberd_c2s:state()}}. +user_receive_packet({#message{meta = #{carbon_copy := true}}, _C2SState} = Acc) -> + %% Stop the hook chain, we don't want logging modules to duplicate this + %% message. + {stop, Acc}; user_receive_packet({Packet, #{jid := JID} = C2SState}) -> To = xmpp:get_to(Packet), - case check_and_forward(JID, To, Packet, received) of - {stop, Pkt} -> {stop, {Pkt, C2SState}}; - Pkt -> {Pkt, C2SState} - end. + {check_and_forward(JID, To, Packet, received), C2SState}. -spec c2s_copy_session(c2s_state(), c2s_state()) -> c2s_state(). c2s_copy_session(State, #{user := U, server := S, resource := R}) -> @@ -157,8 +152,7 @@ c2s_session_opened(State) -> % - 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(jid(), jid(), stanza(), direction()) -> - stanza() | {stop, stanza()}. +-spec check_and_forward(jid(), jid(), stanza(), direction()) -> stanza(). check_and_forward(JID, To, Packet, Direction)-> case (is_chat_message(Packet) orelse is_received_muc_invite(Packet, Direction)) andalso @@ -166,18 +160,11 @@ check_and_forward(JID, To, Packet, Direction)-> not xmpp:has_subtag(Packet, #carbons_private{}) andalso not xmpp:has_subtag(Packet, #hint{type = 'no-copy'}) of true -> - case is_carbon_copy(Packet) of - false -> - send_copies(JID, To, Packet, Direction), - Packet; - true -> - %% stop the hook chain, we don't want logging modules to duplicates - %% this message - {stop, Packet} - end; - _ -> - Packet - end. + send_copies(JID, To, Packet, Direction); + false -> + ok + end, + Packet. %%% Internal %% Direction = received | sent