]> granicus.if.org Git - ejabberd/commitdiff
mod_push: Omit summary for outgoing messages
authorHolger Weiss <holger@zedat.fu-berlin.de>
Wed, 13 Jun 2018 19:43:40 +0000 (21:43 +0200)
committerHolger Weiss <holger@zedat.fu-berlin.de>
Wed, 13 Jun 2018 19:43:40 +0000 (21:43 +0200)
Don't include a urn:xmpp:push:summary form in push notifications that
are triggered by outgoing messages.  App servers might use the form
fields to generate user-visible notifications directly (as opposed to
just waking the client app).  This is usually not desired for outgoing
messages.

src/mod_push.erl
src/mod_push_keepalive.erl

index db479cfc7525dd0e1bbce540e7ef1e164260f2f5..90e982840d8b782d5a2304f8621a213e3667e431 100644 (file)
@@ -44,7 +44,7 @@
 -export([get_commands_spec/0, delete_old_sessions/1]).
 
 %% API (used by mod_push_keepalive).
--export([notify/2, notify/4, notify/6, is_message_with_body/1]).
+-export([notify/3, notify/5, notify/7, is_incoming_chat_msg/1]).
 
 %% For IQ callbacks
 -export([delete_session/3]).
@@ -60,6 +60,7 @@
 -type timestamp() :: erlang:timestamp().
 -type push_session() :: {timestamp(), ljid(), binary(), xdata()}.
 -type err_reason() :: notfound | db_failure.
+-type direction() :: send | recv | undefined.
 
 -callback init(binary(), gen_mod:opts())
          -> any().
@@ -346,20 +347,20 @@ c2s_stanza(State, #stream_error{}, _SendResult) ->
 c2s_stanza(#{push_enabled := true, mgmt_state := pending} = State,
           Pkt, _SendResult) ->
     ?DEBUG("Notifying client of stanza", []),
-    notify(State, xmpp_util:unwrap_carbon(Pkt)),
+    notify(State, unwrap_carbon(Pkt), get_direction(Pkt)),
     State;
 c2s_stanza(State, _Pkt, _SendResult) ->
     State.
 
 -spec mam_message(message() | drop, binary(), binary(), jid(),
                  chat | groupchat, recv | send) -> message().
-mam_message(#message{} = Pkt, LUser, LServer, _Peer, chat, _Dir) ->
+mam_message(#message{} = Pkt, LUser, LServer, _Peer, chat, Dir) ->
     case lookup_sessions(LUser, LServer) of
        {ok, [_|_] = Clients} ->
            case drop_online_sessions(LUser, LServer, Clients) of
                [_|_] = Clients1 ->
                    ?DEBUG("Notifying ~s@~s of MAM message", [LUser, LServer]),
-                   notify(LUser, LServer, Clients1, Pkt);
+                   notify(LUser, LServer, Clients1, Pkt, Dir);
                [] ->
                    ok
            end;
@@ -377,7 +378,7 @@ offline_message(#message{to = #jid{luser = LUser, lserver = LServer}} = Pkt) ->
     case lookup_sessions(LUser, LServer) of
        {ok, [_|_] = Clients} ->
            ?DEBUG("Notifying ~s@~s of offline message", [LUser, LServer]),
-           notify(LUser, LServer, Clients, Pkt);
+           notify(LUser, LServer, Clients, Pkt, recv);
        _ ->
            ok
     end,
@@ -388,8 +389,12 @@ c2s_session_pending(#{push_enabled := true, mgmt_queue := Queue} = State) ->
     case p1_queue:len(Queue) of
        Len when Len > 0 ->
            ?DEBUG("Notifying client of unacknowledged stanza(s)", []),
-           Pkt = mod_stream_mgmt:queue_find(fun is_message_with_body/1, Queue),
-           notify(State, xmpp_util:unwrap_carbon(Pkt)),
+           {Pkt, Dir} = case mod_stream_mgmt:queue_find(
+                               fun is_incoming_chat_msg/1, Queue) of
+                            none -> {none, undefined};
+                            Pkt0 -> {unwrap_carbon(Pkt0), get_direction(Pkt0)}
+                        end,
+           notify(State, Pkt, Dir),
            State;
        0 ->
            State
@@ -421,18 +426,20 @@ remove_user(LUser, LServer) ->
 %%--------------------------------------------------------------------
 %% Generate push notifications.
 %%--------------------------------------------------------------------
--spec notify(c2s_state(), xmpp_element() | xmlel() | none) -> ok.
-notify(#{jid := #jid{luser = LUser, lserver = LServer}, sid := {TS, _}}, Pkt) ->
+-spec notify(c2s_state(), xmpp_element() | xmlel() | none, direction()) -> ok.
+notify(#{jid := #jid{luser = LUser, lserver = LServer},
+        sid := {TS, _}},
+       Pkt, Dir) ->
     case lookup_session(LUser, LServer, TS) of
        {ok, Client} ->
-           notify(LUser, LServer, [Client], Pkt);
+           notify(LUser, LServer, [Client], Pkt, Dir);
        _Err ->
            ok
     end.
 
 -spec notify(binary(), binary(), [push_session()],
-            xmpp_element() | xmlel() | none) -> ok.
-notify(LUser, LServer, Clients, Pkt) ->
+            xmpp_element() | xmlel() | none, direction()) -> ok.
+notify(LUser, LServer, Clients, Pkt, Dir) ->
     lists:foreach(
       fun({TS, PushLJID, Node, XData}) ->
              HandleResponse = fun(#iq{type = result}) ->
@@ -443,15 +450,15 @@ notify(LUser, LServer, Clients, Pkt) ->
                                  (timeout) ->
                                       ok % Hmm.
                               end,
-             notify(LServer, PushLJID, Node, XData, Pkt, HandleResponse)
+             notify(LServer, PushLJID, Node, XData, Pkt, Dir, HandleResponse)
       end, Clients).
 
 -spec notify(binary(), ljid(), binary(), xdata(),
-            xmpp_element() | xmlel() | none,
+            xmpp_element() | xmlel() | none, direction(),
             fun((iq() | timeout) -> any())) -> ok.
-notify(LServer, PushLJID, Node, XData, Pkt, HandleResponse) ->
+notify(LServer, PushLJID, Node, XData, Pkt, Dir, HandleResponse) ->
     From = jid:make(LServer),
-    Summary = make_summary(LServer, Pkt),
+    Summary = make_summary(LServer, Pkt, Dir),
     Item = #ps_item{sub_els = [#push_notification{xdata = Summary}]},
     PubSub = #pubsub{publish = #ps_publish{node = Node, items = [Item]},
                     publish_options = XData},
@@ -465,11 +472,13 @@ notify(LServer, PushLJID, Node, XData, Pkt, HandleResponse) ->
 %%--------------------------------------------------------------------
 %% Miscellaneous.
 %%--------------------------------------------------------------------
--spec is_message_with_body(stanza()) -> boolean().
-is_message_with_body(#message{} = Msg) ->
-    Msg1 = xmpp_util:unwrap_carbon(Msg),
-    get_body_text(Msg1) /= none;
-is_message_with_body(_Stanza) ->
+-spec is_incoming_chat_msg(stanza()) -> boolean().
+is_incoming_chat_msg(#message{} = Msg) ->
+    case get_direction(Msg) of
+       recv -> get_body_text(unwrap_carbon(Msg)) /= none;
+       send -> false
+    end;
+is_incoming_chat_msg(_Stanza) ->
     false.
 
 %%--------------------------------------------------------------------
@@ -593,9 +602,9 @@ drop_online_sessions(LUser, LServer, Clients) ->
     [Client || {TS, _, _, _} = Client <- Clients,
               lists:keyfind(TS, 1, SessIDs) == false].
 
--spec make_summary(binary(), xmpp_element() | xmlel() | none)
+-spec make_summary(binary(), xmpp_element() | xmlel() | none, direction())
       -> xdata() | undefined.
-make_summary(Host, #message{from = From} = Pkt) ->
+make_summary(Host, #message{from = From} = Pkt, recv) ->
     case {gen_mod:get_module_opt(Host, ?MODULE, include_sender),
          gen_mod:get_module_opt(Host, ?MODULE, include_body)} of
        {false, false} ->
@@ -622,7 +631,23 @@ make_summary(Host, #message{from = From} = Pkt) ->
                    #xdata{type = submit, fields = push_summary:encode(Fields2)}
            end
     end;
-make_summary(_Host, _Pkt) ->
+make_summary(_Host, _Pkt, _Dir) ->
+    undefined.
+
+-spec unwrap_carbon(stanza()) -> stanza().
+unwrap_carbon(#message{meta = #{carbon_copy := true}} = Msg) ->
+    xmpp_util:unwrap_carbon(Msg);
+unwrap_carbon(Stanza) ->
+    Stanza.
+
+-spec get_direction(stanza()) -> direction().
+get_direction(#message{meta = #{carbon_copy := true},
+                      from = #jid{luser = U, lserver = S},
+                      to = #jid{luser = U, lserver = S}}) ->
+    send;
+get_direction(#message{}) ->
+    recv;
+get_direction(_Stanza) ->
     undefined.
 
 -spec get_body_text(message()) -> binary() | none.
index 1122d84e1062e0de9899b8469637fd7b3147e5d4..5dc6a20b94dddc09847918acafaa7d85d512d0ea 100644 (file)
@@ -131,7 +131,7 @@ unregister_hooks(Host) ->
 -spec c2s_stanza(c2s_state(), xmpp_element() | xmlel(), term()) -> c2s_state().
 c2s_stanza(#{push_enabled := true, mgmt_state := pending} = State,
           Pkt, _SendResult) ->
-    case mod_push:is_message_with_body(Pkt) of
+    case mod_push:is_incoming_chat_msg(Pkt) of
        true ->
            maybe_restore_resume_timeout(State);
        false ->
@@ -142,7 +142,7 @@ c2s_stanza(State, _Pkt, _SendResult) ->
 
 -spec c2s_session_pending(c2s_state()) -> c2s_state().
 c2s_session_pending(#{push_enabled := true, mgmt_queue := Queue} = State) ->
-    case mod_stream_mgmt:queue_find(fun mod_push:is_message_with_body/1,
+    case mod_stream_mgmt:queue_find(fun mod_push:is_incoming_chat_msg/1,
                                    Queue) of
        none ->
            State1 = maybe_adjust_resume_timeout(State),
@@ -190,7 +190,7 @@ c2s_handle_cast(State, _Msg) ->
 c2s_handle_info(#{push_enabled := true, mgmt_state := pending,
                  jid := JID} = State, {timeout, _, push_keepalive}) ->
     ?INFO_MSG("Waking ~s before session times out", [jid:encode(JID)]),
-    mod_push:notify(State, none),
+    mod_push:notify(State, none, undefined),
     {stop, State};
 c2s_handle_info(State, _) ->
     State.
@@ -235,7 +235,8 @@ wake_all(LServer) ->
            IgnoreResponse = fun(_) -> ok end,
            lists:foreach(fun({_, PushLJID, Node, XData}) ->
                                  mod_push:notify(LServer, PushLJID, Node,
-                                                 XData, none, IgnoreResponse)
+                                                 XData, none, undefined,
+                                                 IgnoreResponse)
                          end, Sessions);
        error ->
            error