From 6bbed5854a01f241971173b95df1e809cab49171 Mon Sep 17 00:00:00 2001 From: Pawel Chmielowski Date: Wed, 11 Mar 2026 10:18:09 +0100 Subject: [PATCH] Fix duplicate stanza-id in muc mam responses generated from local history This fixes issue #4544 --- src/mod_mam.erl | 2 +- src/mod_muc_room.erl | 59 +++++++++++++++++++------------------------- test/mam_tests.erl | 8 +++--- 3 files changed, 32 insertions(+), 37 deletions(-) diff --git a/src/mod_mam.erl b/src/mod_mam.erl index 098f85221..49e780b48 100644 --- a/src/mod_mam.erl +++ b/src/mod_mam.erl @@ -54,7 +54,7 @@ webadmin_page_hostuser/4, get_mam_messages/2, webadmin_user/4, delete_old_messages_batch/5, delete_old_messages_status/1, delete_old_messages_abort/1, - remove_message_from_archive/3]). + remove_message_from_archive/3, strip_my_stanza_id/2]). -import(ejabberd_web_admin, [make_command/4, make_command/2]). diff --git a/src/mod_muc_room.erl b/src/mod_muc_room.erl index d23716626..798a5bfa9 100644 --- a/src/mod_muc_room.erl +++ b/src/mod_muc_room.erl @@ -1097,21 +1097,21 @@ process_groupchat_message(#message{from = From, lang = Lang} = Packet, StateData drop -> {next_state, normal_state, StateData}; NewPacket1 -> - NewPacket = xmpp:put_meta(xmpp:remove_subtag( - add_stanza_id(NewPacket1, StateData), #nick{}), - muc_sender_real_jid, From), + NewPacket2 = xmpp:put_meta(NewPacket1, muc_sender_real_jid, From), + NewPacket3 = xmpp:remove_subtag(NewPacket2, #nick{}), + {ForHistory, ToSend} = add_stanza_id(NewPacket3, StateData), Node = if Subject == [] -> ?NS_MUCSUB_NODES_MESSAGES; true -> ?NS_MUCSUB_NODES_SUBJECT end, - NewStateData2 = check_message_for_retractions(NewPacket1, NewStateData1), + NewStateData2 = check_message_for_retractions(ToSend, NewStateData1), send_wrapped_multiple( - jid:replace_resource(StateData#state.jid, FromNick), - get_users_and_subscribers_with_node(Node, StateData), - NewPacket, Node, NewStateData2), - NewStateData3 = case has_body_or_subject(NewPacket) of + jid:replace_resource(StateData#state.jid, FromNick), + get_users_and_subscribers_with_node(Node, StateData), + ToSend, Node, NewStateData2), + NewStateData3 = case has_body_or_subject(ToSend) of true -> add_message_to_history(FromNick, From, - NewPacket, + ForHistory, NewStateData2); false -> NewStateData2 @@ -1176,28 +1176,20 @@ check_message_for_retractions(Packet, State end. --spec add_stanza_id(Packet :: message(), State :: state()) -> message(). -add_stanza_id(Packet, #state{jid = JID}) -> - {AddId, NewPacket} = - case xmpp:get_meta(Packet, stanza_id, false) of - false -> - GenID = erlang:system_time(microsecond), - {true, xmpp:put_meta(Packet, stanza_id, GenID)}; +-spec add_stanza_id(Packet :: message(), State :: state()) -> {message(), message()}. +add_stanza_id(#message{meta = Meta} = Packet, #state{jid = JID}) -> + case Meta of + #{stanza_id := _StanzaId, mam_archived := _Archived} -> + {Packet, Packet}; + #{stanza_id := StanzaId} -> + ToSend = xmpp:append_subtags(Packet, [#stanza_id{by = JID, id = integer_to_binary(StanzaId)}]), + {Packet, ToSend}; _ -> - StanzaIds = xmpp:get_subtags(Packet, #stanza_id{by = #jid{}}), - HasOurStanzaId = lists:any( - fun(#stanza_id{by = JID2}) when JID == JID2 -> true; - (_) -> false - end, StanzaIds), - {not HasOurStanzaId, Packet} - end, - if - AddId -> - ID = xmpp:get_meta(NewPacket, stanza_id), - IDs = integer_to_binary(ID), - xmpp:append_subtags(NewPacket, [#stanza_id{by = JID, id = IDs}]); - true -> - Packet + StanzaId = xmpp:get_meta(Packet, stanza_id, mod_mam:make_id()), + Stripped = mod_mam:strip_my_stanza_id(Packet, JID#jid.lserver), + ForHistory = xmpp:put_meta(Stripped, stanza_id, StanzaId), + ToSend = xmpp:append_subtags(Packet, [#stanza_id{by = JID, id = integer_to_binary(StanzaId)}]), + {ForHistory, ToSend} end. -spec process_normal_message(jid(), message(), state()) -> state(). @@ -2131,7 +2123,7 @@ calculate_occupant_id(Jid, #state{salt = Salt, jid = RoomJid}) -> Term = <>, misc:term_to_base64(crypto:hash(sha256, Term)). --spec filter_message_hook(state(), binary(), #message{}) -> drop | #message{}. +-spec filter_message_hook(state(), binary(), #message{}) -> drop | stanza(). filter_message_hook(#state{users = Users} = StateData, Nick, #message{from = From} = Message) -> OccupantId = case maps:find(jid:tolower(From), Users) of {ok, #user{occupant_id = Id}} -> Id; @@ -2997,7 +2989,8 @@ add_message_to_history(FromNick, FromJID, Packet, StateData) -> add_to_log(text, {FromNick, Packet}, StateData), case check_subject(Packet) of [] -> - TimeStamp = erlang:timestamp(), + StanzaId = xmpp:get_meta(Packet, stanza_id, mod_mam:make_id()), + TimeStamp = misc:usec_to_now(StanzaId), AddrPacket = case (StateData#state.config)#config.anonymous of true -> Packet; false -> @@ -5837,7 +5830,7 @@ send_wrapped_multiple(From, Users, Packet, Node, State) -> ok end; _ -> - false + ok end; _ -> ok diff --git a/test/mam_tests.erl b/test/mam_tests.erl index 6cf43bade..bbb9a117b 100644 --- a/test/mam_tests.erl +++ b/test/mam_tests.erl @@ -280,9 +280,9 @@ muc_master(Config) -> true = is_feature_advertised(Config, ?NS_MAM_1, Room), true = is_feature_advertised(Config, ?NS_MAM_2, Room), %% We now sending some messages again - send_messages_to_room(Config, lists:seq(1, 5)), + send_messages_to_room(Config, lists:seq(101, 105)), %% And retrieve them via MAM again. - recv_messages_from_room(Config, lists:seq(1, 5)), + recv_messages_from_room(Config, lists:seq(101, 105)), put_event(Config, disconnect), muc_tests:leave(Config), clean(disconnect(Config)). @@ -556,7 +556,9 @@ recv_messages_from_room(Config, Range) -> sub_els = [El]}]}]} = recv_message(Config), #message{from = MyNickJID, type = groupchat, - body = Body} = xmpp:decode(El) + body = Body} = Nested = xmpp:decode(El), + ct:comment("Verify that there is just single stanza-id", []), + ?match([#stanza_id{}], xmpp:get_subtags(Nested, #stanza_id{})) end, Range), #iq{from = Room, id = I, type = result, sub_els = [#mam_fin{xmlns = ?NS_MAM_2,