processone / ejabberd

Robust, Ubiquitous and Massively Scalable Messaging Platform (XMPP, MQTT, SIP Server)
https://www.process-one.net/en/ejabberd/
Other
6.11k stars 1.51k forks source link

When subscriber store the presence offline #3959

Closed cisiqo closed 1 year ago

cisiqo commented 1 year ago

Someone presence one muc room, the subscriber may received this message:

#message{
 id = <<"14535871043214929496">>,type = normal,lang = <<>>,
 from =
  #jid{
   user = <<"aaaaaaaaa">>,server = <<"conference.example.com">>,
   resource = <<>>,luser = <<"aaaaaaaaa">>,
   lserver = <<"conference.example.com">>,lresource = <<>>},
 to =
  #jid{
   user = <<"suber_1">>,server = <<"example.com">>,resource = <<>>,
   luser = <<"suber_1">>,lserver = <<"example.com">>,lresource = <<>>},
 subject = [],body = [],thread = undefined,
 sub_els =
  [#ps_event{
    items =
     #ps_items{
      xmlns = <<>>,node = <<"urn:xmpp:mucsub:nodes:presence">>,
      items =
       [#ps_item{
         xmlns = <<>>,id = <<"14535871043214929496">>,
         sub_els =
          [#presence{
            id = <<>>,type = available,lang = <<"en">>,
            from =
             #jid{
              user = <<"aaaaaaaaa">>,server = <<"conference.example.com">>,
              resource = <<"bbb">>,luser = <<"aaaaaaaaa">>,
              lserver = <<"conference.example.com">>,lresource = <<"bbb">>},
            to =
             #jid{
              user = <<"suber_1">>,server = <<"example.com">>,resource = <<>>,
              luser = <<"suber_1">>,lserver = <<"example.com">>,
              lresource = <<>>},
            show = undefined,status = [],priority = undefined,
            sub_els =
             [#muc_user{
               decline = undefined,destroy = undefined,invites = [],
               items =
                [#muc_item{
                  actor = undefined,continue = undefined,reason = <<>>,
                  affiliation = owner,role = moderator,jid = undefined,
                  nick = <<>>}],
               status_codes = [],password = undefined}],
            meta = #{ip => {0,0,0,0,0,65535,49320,8449}}}],
         node = <<>>,publisher = <<>>}],
      max_items = undefined,subid = <<>>,retract = undefined},
    purge = undefined,subscription = undefined,delete = undefined,
    create = undefined,configuration = undefined}],
 meta = #{in_muc_mam => true}}

When the message need to store offline while the option store_empty_body was false, it must throw exception.

coveralls commented 1 year ago

Coverage Status

Coverage increased (+0.01%) to 33.259% when pulling 027ddd7c461962fd360305c0a7bcb6899110f856 on cisiqo:patch-1 into bc063b8f9734e523030290175e61cef2610d6f7e on processone:master.

prefiks commented 1 year ago

Hello,

I don't fully understand what you are trying to do with that patch, is the idea here to prevent storing those mucsub presences in offline storage, or the other way, always storing them?

prefiks commented 1 year ago

After analyzing that patch this should prevent those event from being stored, so i guess that what you are trying to do here. I guess this makes sense for presences, we probably don't want to pollute offline storage with those, but that check is to relax for this i think, we should check explictly if that is mucsub presence (i see some worth in storing other events like subscription or affiliation change, and even more if those are completly different event produced by clients).

So i would preffer to add misc:get_mucsub_event_type() (that should be direct copy of misc:is_mucsub_message, but make it return Node or false, instead of true and false), and use that to just check for NS_MUCSUB_NODES_PRESENCE and filtering it out.

cisiqo commented 1 year ago

According to the previous logic, the presence message should not save the offline message.

If the store_empty_body is true, the presence message will be saved; If the store_empty_body is false, an error will be reported here; If the store_empty_body is empty_chat_state, then the presence will not be saved.

I modified it because I thought there was a flaw in the logic here.

Of course, if you understand the defects here, your modifications will definitely make the program clearer. Thanks.