Prevent duplicate messages by comparing MAM archive id to XEP-0359 stanza ids

CHANGES.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## 4.1.2 (Unreleased)
+
+- Bugfix. Prevent duplicate messages by comparing MAM archive id to XEP-0359 stanza ids.
+
 ## 4.1.1 (2019-02-18)
 
 - Updated translations: af, cz, de, es, eu, ga, he, hi, ja, nb, nl_BE, zh_CN

dev.html

@@ -27,7 +27,6 @@
         // ],
         // websocket_url: 'ws://chat.example.org:5280/xmpp-websocket',
         view_mode: 'fullscreen',
-        archived_messages_page_size: '500',
         notify_all_room_messages: [
             'discuss@conference.conversejs.org'
         ],

dist/converse.js

@@ -55231,7 +55231,7 @@ _converse_headless_converse_core__WEBPACK_IMPORTED_MODULE_3__["default"].plugins
 
         if (!jid || _.compact(jid.split('@')).length < 2) {
           evt.target.outerHTML = templates_chatroom_invite_html__WEBPACK_IMPORTED_MODULE_14___default()({
-            'error_message': __('Please enter a valid XMPP username'),
+            'error_message': __('Please enter a valid XMPP address'),
             'label_invitation': __('Invite')
           });
           this.initInviteWidget();
@@ -61651,6 +61651,26 @@ _converse_core__WEBPACK_IMPORTED_MODULE_2__["default"].plugins.add('converse-cha
         });
       },
 
+      async hasDuplicateArchiveID(stanza) {
+        const result = sizzle(`result[xmlns="${Strophe.NS.MAM}"]`, stanza).pop();
+
+        if (!result) {
+          return false;
+        }
+
+        const by_jid = stanza.getAttribute('from');
+        const supported = await _converse.api.disco.supports(Strophe.NS.MAM, by_jid);
+
+        if (!supported.length) {
+          return false;
+        }
+
+        const query = {};
+        query[`stanza_id ${by_jid}`] = result.getAttribute('id');
+        const msg = this.messages.findWhere(query);
+        return !_.isNil(msg);
+      },
+
       async hasDuplicateStanzaID(stanza) {
         const stanza_id = sizzle(`stanza-id[xmlns="${Strophe.NS.SID}"]`, stanza).pop();
 
@@ -61993,6 +62013,24 @@ _converse_core__WEBPACK_IMPORTED_MODULE_2__["default"].plugins.add('converse-cha
         });
       },
 
+      getStanzaIDs(stanza) {
+        const attrs = {};
+        const stanza_ids = sizzle(`stanza-id[xmlns="${Strophe.NS.SID}"]`, stanza);
+
+        if (stanza_ids.length) {
+          stanza_ids.forEach(s => attrs[`stanza_id ${s.getAttribute('by')}`] = s.getAttribute('id'));
+        }
+
+        const result = sizzle(`message > result[xmlns="${Strophe.NS.MAM}"]`, stanza).pop();
+
+        if (result) {
+          const by_jid = stanza.getAttribute('from');
+          attrs[`stanza_id ${by_jid}`] = result.getAttribute('id');
+        }
+
+        return attrs;
+      },
+
       getMessageAttributesFromStanza(stanza, original_stanza) {
         /* Parses a passed in message stanza and returns an object
          * of attributes.
@@ -62010,7 +62048,7 @@ _converse_core__WEBPACK_IMPORTED_MODULE_2__["default"].plugins.add('converse-cha
               delay = sizzle(`delay[xmlns="${Strophe.NS.DELAY}"]`, original_stanza).pop(),
               chat_state = stanza.getElementsByTagName(_converse.COMPOSING).length && _converse.COMPOSING || stanza.getElementsByTagName(_converse.PAUSED).length && _converse.PAUSED || stanza.getElementsByTagName(_converse.INACTIVE).length && _converse.INACTIVE || stanza.getElementsByTagName(_converse.ACTIVE).length && _converse.ACTIVE || stanza.getElementsByTagName(_converse.GONE).length && _converse.GONE;
 
-        const attrs = {
+        const attrs = _.extend({
           'chat_state': chat_state,
           'is_archived': !_.isNil(archive),
           'is_delayed': !_.isNil(delay),
@@ -62022,9 +62060,7 @@ _converse_core__WEBPACK_IMPORTED_MODULE_2__["default"].plugins.add('converse-cha
           'thread': _.propertyOf(stanza.querySelector('thread'))('textContent'),
           'time': delay ? delay.getAttribute('stamp') : moment().format(),
           'type': stanza.getAttribute('type')
-        };
+        }, this.getStanzaIDs(original_stanza));
-        const stanza_ids = sizzle(`stanza-id[xmlns="${Strophe.NS.SID}"]`, stanza);
-        stanza_ids.forEach(s => attrs[`stanza_id ${s.getAttribute('by')}`] = s.getAttribute('id'));
 
         if (attrs.type === 'groupchat') {
           attrs.from = stanza.getAttribute('from');
@@ -62279,7 +62315,7 @@ _converse_core__WEBPACK_IMPORTED_MODULE_2__["default"].plugins.add('converse-cha
         },
               chatbox = this.getChatBox(contact_jid, chatbox_attrs, has_body);
 
-        if (chatbox && !chatbox.findDuplicateFromOriginID(stanza) && !(await chatbox.hasDuplicateStanzaID(stanza)) && !chatbox.handleMessageCorrection(stanza) && !chatbox.handleReceipt(stanza, from_jid, is_carbon, is_me) && !chatbox.handleChatMarker(stanza, from_jid, is_carbon, is_roster_contact)) {
+        if (chatbox && !chatbox.findDuplicateFromOriginID(stanza) && !(await chatbox.hasDuplicateArchiveID(original_stanza)) && !(await chatbox.hasDuplicateStanzaID(stanza)) && !chatbox.handleMessageCorrection(stanza) && !chatbox.handleReceipt(stanza, from_jid, is_carbon, is_me) && !chatbox.handleChatMarker(stanza, from_jid, is_carbon, is_roster_contact)) {
           const attrs = await chatbox.getMessageAttributesFromStanza(stanza, original_stanza);
 
           if (attrs['chat_state'] || !u.isEmptyMessage(attrs)) {
@@ -67068,7 +67104,7 @@ _converse_core__WEBPACK_IMPORTED_MODULE_6__["default"].plugins.add('converse-muc
           stanza = forwarded.querySelector('message');
         }
 
-        if (this.handleReflection(stanza) || (await this.hasDuplicateStanzaID(stanza)) || this.handleMessageCorrection(stanza) || this.isReceipt(stanza) || this.isChatMarker(stanza)) {
+        if (this.handleReflection(stanza) || (await this.hasDuplicateArchiveID(original_stanza)) || (await this.hasDuplicateStanzaID(stanza)) || this.handleMessageCorrection(stanza) || this.isReceipt(stanza) || this.isChatMarker(stanza)) {
           return _converse.emit('message', {
             'stanza': original_stanza
           });
@@ -92585,7 +92621,7 @@ __p += '\n                        ';
 __p += '\n                        <li class="feature" ><span class="fa fa-id-card"></span>' +
 __e( o.__('Not anonymous') ) +
 ' - <em>' +
-__e( o.__('All other groupchat participants can see your XMPP username') ) +
+__e( o.__('All other groupchat participants can see your XMPP address') ) +
 '</em></li>\n                        ';
  } ;
 __p += '\n                        ';
@@ -92593,7 +92629,7 @@ __p += '\n                        ';
 __p += '\n                        <li class="feature" ><span class="fa fa-user-secret"></span>' +
 __e( o.__('Semi-anonymous') ) +
 ' - <em>' +
-__e( o.__('Only moderators can see your XMPP username') ) +
+__e( o.__('Only moderators can see your XMPP address') ) +
 '</em></li>\n                        ';
  } ;
 __p += '\n                        ';
@@ -92733,7 +92769,7 @@ __e( o.__('Temporary') ) +
 __p += '\n';
  if (o.nonanonymous) { ;
 __p += '\n<li class="feature" title="' +
-__e( o.__('All other groupchat participants can see your XMPP username') ) +
+__e( o.__('All other groupchat participants can see your XMPP address') ) +
 '"><span class="fa fa-id-card"></span>' +
 __e( o.__('Not anonymous') ) +
 '</li>\n';
@@ -92741,7 +92777,7 @@ __e( o.__('Not anonymous') ) +
 __p += '\n';
  if (o.semianonymous) { ;
 __p += '\n<li class="feature" title="' +
-__e( o.__('Only moderators can see your XMPP username') ) +
+__e( o.__('Only moderators can see your XMPP address') ) +
 '"><span class="fa fa-user-secret"></span>' +
 __e( o.__('Semi-anonymous') ) +
 '</li>\n';
@@ -93628,7 +93664,7 @@ __p += '\n            <span class="spinner fa fa-spinner centered"/>\n        ';
 __p += '\n            ';
  if (o.authentication == o.LOGIN || o.authentication == o.EXTERNAL) { ;
 __p += '\n                <div class="form-group">\n                    <label for="converse-login-jid">' +
-__e(o.__("XMPP Username:")) +
+__e(o.__("XMPP Address:")) +
 '</label>\n                    <input id="converse-login-jid" class="form-control" autofocus required="required" type="text" name="jid" placeholder="' +
 __e(o.placeholder_username) +
 '"/>\n                </div>\n                ';

spec/mam.js

@@ -16,7 +16,7 @@
 
         describe("Archived Messages", function () {
 
-           it("aren't shown as duplicates by comparing their stanza-id attribute", 
+           it("aren't shown as duplicates by comparing their stanza id and archive id",
                 mock.initConverse(
                     null, ['discoInitialized'], {},
                     async function (done, _converse) {
@@ -33,7 +33,9 @@
                 // Not sure whether such a race-condition might pose a problem
                 // in "real-world" situations.
                 stanza = u.toStanza(
-                    `<message xmlns="jabber:client" to="jcbrand@lightwitch.org/converse.js-73057452">
+                    `<message xmlns="jabber:client"
+                              to="jcbrand@lightwitch.org/converse.js-73057452"
+                              from="trek-radio@conference.lightwitch.org">
                         <result xmlns="urn:xmpp:mam:2" queryid="82d9db27-6cf8-4787-8c2c-5a560263d823" id="45fbbf2a-1059-479d-9283-c8effaf05621">
                             <forwarded xmlns="urn:xmpp:forward:0">
                                 <delay xmlns="urn:xmpp:delay" stamp="2018-01-09T06:17:23Z"/>
@@ -43,15 +45,17 @@
                             </forwarded>
                         </result>
                     </message>`);
-
+                spyOn(view.model, 'hasDuplicateArchiveID').and.callThrough();
-                spyOn(view.model, 'hasDuplicateStanzaID').and.callThrough();
                 view.model.onMessage(stanza);
-                await test_utils.waitUntil(() => view.model.hasDuplicateStanzaID.calls.count());
+                await test_utils.waitUntil(() => view.model.hasDuplicateArchiveID.calls.count());
+                expect(view.model.hasDuplicateArchiveID.calls.count()).toBe(1);
+                const result = await view.model.hasDuplicateArchiveID.calls.all()[0].returnValue
+                expect(result).toBe(true);
                 expect(view.content.querySelectorAll('.chat-msg').length).toBe(1);
                 done();
             }));
 
-           it("aren't shown as duplicates by comparing their queryid attribute", 
+           it("aren't shown as duplicates by comparing only their archive id",
                 mock.initConverse(
                     null, ['discoInitialized'], {},
                     async function (done, _converse) {
@@ -59,20 +63,6 @@
                 await test_utils.openAndEnterChatRoom(_converse, 'discuss', 'conference.conversejs.org', 'dummy');
                 const view = _converse.chatboxviews.get('discuss@conference.conversejs.org');
                 let stanza = u.toStanza(
-                    `<message xmlns="jabber:client"
-                              to="discuss@conference.conversejs.org"
-                              type="groupchat" xml:lang="en"
-                              from="discuss@conference.conversejs.org/prezel">
-                        <stanza-id xmlns="urn:xmpp:sid:0" id="7a9fde91-4387-4bf8-b5d3-978dab8f6bf3" by="discuss@conference.conversejs.org"/>
-                        <body>looks like omemo fails completely with "bundle is undefined" when there is a device in the devicelist that has no keys published</body>
-                        <x xmlns="http://jabber.org/protocol/muc#user">
-                            <item affiliation="none" jid="prezel@blubber.im" role="participant"/>
-                        </x>
-                    </message>`);
-                _converse.connection._dataRecv(test_utils.createRequest(stanza));
-                await test_utils.waitUntil(() => view.content.querySelectorAll('.chat-msg').length);
-
-                stanza = u.toStanza(
                     `<message xmlns="jabber:client" to="dummy@localhost/resource" from="discuss@conference.conversejs.org">
                         <result xmlns="urn:xmpp:mam:2" queryid="06fea9ca-97c9-48c4-8583-009ff54ea2e8" id="7a9fde91-4387-4bf8-b5d3-978dab8f6bf3">
                             <forwarded xmlns="urn:xmpp:forward:0">
@@ -86,11 +76,8 @@
                             </forwarded>
                         </result>
                     </message>`);
-
-                spyOn(view.model, 'hasDuplicateStanzaID').and.callThrough();
                 view.model.onMessage(stanza);
-                await test_utils.waitUntil(() => view.model.hasDuplicateStanzaID.calls.count());
+                await test_utils.waitUntil(() => view.content.querySelectorAll('.chat-msg').length);
-                expect(view.model.hasDuplicateStanzaID.calls.count()).toBe(1);
                 expect(view.content.querySelectorAll('.chat-msg').length).toBe(1);
 
                 stanza = u.toStanza(
@@ -107,8 +94,13 @@
                             </forwarded>
                         </result>
                     </message>`);
+
+                spyOn(view.model, 'hasDuplicateArchiveID').and.callThrough();
                 view.model.onMessage(stanza);
-                expect(view.model.hasDuplicateStanzaID.calls.count()).toBe(2);
+                await test_utils.waitUntil(() => view.model.hasDuplicateArchiveID.calls.count());
+                expect(view.model.hasDuplicateArchiveID.calls.count()).toBe(1);
+                const result = await view.model.hasDuplicateArchiveID.calls.all()[0].returnValue
+                expect(result).toBe(true);
                 expect(view.content.querySelectorAll('.chat-msg').length).toBe(1);
                 done();
             }))

spec/roomslist.js

@@ -220,7 +220,7 @@
                 'Hidden - This groupchat is not publicly searchable'+
                 'Open - Anyone can join this groupchat'+
                 'Temporary - This groupchat will disappear once the last person leaves'+
-                'Not anonymous - All other groupchat participants can see your XMPP username'+
+                'Not anonymous - All other groupchat participants can see your XMPP address'+
                 'Not moderated - Participants entering this groupchat can write right away'
             );
             presence = $pres({

src/headless/converse-chatboxes.js

@@ -325,6 +325,22 @@ converse.plugins.add('converse-chatboxes', {
                 });
             },
 
+            async hasDuplicateArchiveID (stanza) {
+                const result = sizzle(`result[xmlns="${Strophe.NS.MAM}"]`, stanza).pop();
+                if (!result) {
+                    return false;
+                }
+                const by_jid = stanza.getAttribute('from');
+                const supported = await _converse.api.disco.supports(Strophe.NS.MAM, by_jid);
+                if (!supported.length) {
+                    return false;
+                }
+                const query = {};
+                query[`stanza_id ${by_jid}`] = result.getAttribute('id');
+                const msg = this.messages.findWhere(query);
+                return !_.isNil(msg);
+            },
+
             async hasDuplicateStanzaID (stanza) {
                 const stanza_id = sizzle(`stanza-id[xmlns="${Strophe.NS.SID}"]`, stanza).pop();
                 if (!stanza_id) {
@@ -609,6 +625,20 @@ converse.plugins.add('converse-chatboxes', {
                 });
             },
 
+            getStanzaIDs (stanza) {
+                const attrs = {};
+                const stanza_ids = sizzle(`stanza-id[xmlns="${Strophe.NS.SID}"]`, stanza);
+                if (stanza_ids.length) {
+                    stanza_ids.forEach(s => (attrs[`stanza_id ${s.getAttribute('by')}`] = s.getAttribute('id')));
+                }
+                const result = sizzle(`message > result[xmlns="${Strophe.NS.MAM}"]`, stanza).pop();
+                if (result) {
+                    const by_jid = stanza.getAttribute('from');
+                    attrs[`stanza_id ${by_jid}`] = result.getAttribute('id');
+                }
+                return attrs;
+            },
+
             getMessageAttributesFromStanza (stanza, original_stanza) {
                 /* Parses a passed in message stanza and returns an object
                  * of attributes.
@@ -630,7 +660,7 @@ converse.plugins.add('converse-chatboxes', {
                             stanza.getElementsByTagName(_converse.ACTIVE).length && _converse.ACTIVE ||
                             stanza.getElementsByTagName(_converse.GONE).length && _converse.GONE;
 
-                const attrs = {
+                const attrs = _.extend({
                     'chat_state': chat_state,
                     'is_archived': !_.isNil(archive),
                     'is_delayed': !_.isNil(delay),
@@ -642,9 +672,7 @@ converse.plugins.add('converse-chatboxes', {
                     'thread': _.propertyOf(stanza.querySelector('thread'))('textContent'),
                     'time': delay ? delay.getAttribute('stamp') : moment().format(),
                     'type': stanza.getAttribute('type')
-                };
+                }, this.getStanzaIDs(original_stanza));
-                const stanza_ids = sizzle(`stanza-id[xmlns="${Strophe.NS.SID}"]`, stanza);
-                stanza_ids.forEach(s => (attrs[`stanza_id ${s.getAttribute('by')}`] = s.getAttribute('id')));
 
                 if (attrs.type === 'groupchat') {
                     attrs.from = stanza.getAttribute('from');
@@ -880,6 +908,7 @@ converse.plugins.add('converse-chatboxes', {
 
                 if (chatbox &&
                         !chatbox.findDuplicateFromOriginID(stanza) &&
+                        !await chatbox.hasDuplicateArchiveID(original_stanza) &&
                         !await chatbox.hasDuplicateStanzaID(stanza) &&
                         !chatbox.handleMessageCorrection(stanza) &&
                         !chatbox.handleReceipt (stanza, from_jid, is_carbon, is_me) &&

src/headless/converse-muc.js

@@ -1044,6 +1044,7 @@ converse.plugins.add('converse-muc', {
                     stanza = forwarded.querySelector('message');
                 }
                 if (this.handleReflection(stanza) ||
+                        await this.hasDuplicateArchiveID(original_stanza) ||
                         await this.hasDuplicateStanzaID(stanza) ||
                         this.handleMessageCorrection(stanza) ||
                         this.isReceipt(stanza) ||

tests/utils.js

@@ -143,6 +143,7 @@
             'http://jabber.org/protocol/muc',
             'jabber:iq:register',
             Strophe.NS.SID,
+            Strophe.NS.MAM,
             'muc_passwordprotected',
             'muc_hidden',
             'muc_temporary',