From 83316865510a2e3c26b44f41840028b28cd440a1 Mon Sep 17 00:00:00 2001
From: Melvin Keskin <melvo@olomono.de>
Date: Sat, 11 Mar 2023 14:38:18 +0100
Subject: OmemoManagerPrivate: Remove 'this->' when not needed

---
 src/omemo/QXmppOmemoManager_p.cpp | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

(limited to 'src')

diff --git a/src/omemo/QXmppOmemoManager_p.cpp b/src/omemo/QXmppOmemoManager_p.cpp
index 303c19dc..070ed5a8 100644
--- a/src/omemo/QXmppOmemoManager_p.cpp
+++ b/src/omemo/QXmppOmemoManager_p.cpp
@@ -923,7 +923,7 @@ bool ManagerPrivate::updatePreKeyPairs(uint32_t count)
         deviceBundle.addPublicPreKey(preKeyId, serializedPublicPreKey);
     }
 
-    this->preKeyPairs.insert(serializedPreKeyPairs);
+    preKeyPairs.insert(serializedPreKeyPairs);
     omemoStorage->addPreKeyPairs(serializedPreKeyPairs);
     ownDevice.latestPreKeyId = latestPreKeyId - 1 + count;
 
@@ -2786,7 +2786,7 @@ void ManagerPrivate::updateDevices(const QString &deviceOwnerJid, const QXmppOme
     // Publish an own correct device list if the PEP service's one is incorrect
     // and the devices are already set up locally.
     if (isOwnDeviceListIncorrect) {
-        if (!this->devices.isEmpty()) {
+        if (!devices.isEmpty()) {
             publishDeviceListItem(true, [=](bool isPublished) {
                 if (!isPublished) {
                     warning("Own device list item could not be published in order to correct the PEP service's one");
@@ -2848,7 +2848,7 @@ void ManagerPrivate::handleIrregularDeviceListChanges(const QString &deviceOwner
             }
         });
     } else {
-        auto &ownerDevices = this->devices[deviceOwnerJid];
+        auto &ownerDevices = devices[deviceOwnerJid];
 
         // Set a timestamp for locally stored contact devices being removed
         // later if their device list item is removed, if their device list node
-- 
cgit v1.2.3


From 64e29c129d7a47e5d128cd097c5c4f6cbd5b4328 Mon Sep 17 00:00:00 2001
From: Melvin Keskin <melvo@olomono.de>
Date: Sat, 11 Mar 2023 14:37:35 +0100
Subject: OmemoManagerPrivate: Fix documentation and warning

---
 src/omemo/QXmppOmemoManager.cpp   | 2 +-
 src/omemo/QXmppOmemoManager_p.cpp | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

(limited to 'src')

diff --git a/src/omemo/QXmppOmemoManager.cpp b/src/omemo/QXmppOmemoManager.cpp
index 0aab152d..d5d3af0a 100644
--- a/src/omemo/QXmppOmemoManager.cpp
+++ b/src/omemo/QXmppOmemoManager.cpp
@@ -340,7 +340,7 @@ QXmppOmemoManager::~QXmppOmemoManager() = default;
 ///
 /// This should be called after starting the client and before the login.
 /// It must only be called after \c setUp() has been called once for the user
-/// during one of the past login session.
+/// during one of the past login sessions.
 /// It does not need to be called if setUp() has been called during the current
 /// login session.
 ///
diff --git a/src/omemo/QXmppOmemoManager_p.cpp b/src/omemo/QXmppOmemoManager_p.cpp
index 070ed5a8..63cb5e00 100644
--- a/src/omemo/QXmppOmemoManager_p.cpp
+++ b/src/omemo/QXmppOmemoManager_p.cpp
@@ -2798,7 +2798,7 @@ void ManagerPrivate::updateDevices(const QString &deviceOwnerJid, const QXmppOme
 
 //
 // Corrects the own device list on the PEP service by the locally stored
-// devices or set a contact device to be removed locally in the future.
+// devices or sets a contact device to be removed locally in the future.
 //
 // \param deviceOwnerJid bare JID of the devices' owner
 //
@@ -2813,7 +2813,7 @@ void ManagerPrivate::handleIrregularDeviceListChanges(const QString &deviceOwner
         auto future = pubSubManager->deleteOwnPepNode(ns_omemo_2_devices);
         future.then(q, [=](QXmppPubSubManager::Result result) {
             if (const auto error = std::get_if<QXmppError>(&result)) {
-                warning("Node '" % QString(ns_omemo_2_devices) % "'  of JID '" % deviceOwnerJid %
+                warning("Node '" % QString(ns_omemo_2_devices) % "' of JID '" % deviceOwnerJid %
                         "' could not be deleted in order to recover from an inconsistent node: " %
                         errorToString(*error));
             } else {
-- 
cgit v1.2.3


From af46ac9487caf54bb0783616af4e209b94b1cde5 Mon Sep 17 00:00:00 2001
From: Melvin Keskin <melvo@olomono.de>
Date: Sat, 11 Mar 2023 20:21:59 +0100
Subject: Fix OMEMO device list processing as specified

---
 src/omemo/QXmppOmemoManager.cpp   | 34 ++++++++++++++------------
 src/omemo/QXmppOmemoManager_p.cpp | 51 +++++++++++++++++++++++++++++++++++----
 src/omemo/QXmppOmemoManager_p.h   |  1 +
 3 files changed, 66 insertions(+), 20 deletions(-)

(limited to 'src')

diff --git a/src/omemo/QXmppOmemoManager.cpp b/src/omemo/QXmppOmemoManager.cpp
index d5d3af0a..a3ad12bb 100644
--- a/src/omemo/QXmppOmemoManager.cpp
+++ b/src/omemo/QXmppOmemoManager.cpp
@@ -1267,29 +1267,33 @@ bool Manager::handlePubSubEvent(const QDomElement &element, const QString &pubSu
         switch (event.eventType()) {
         // Items have been published.
         case QXmppPubSubEventBase::Items: {
-            const auto items = event.items();
-
             // Only process items if the event notification contains one.
-            // That is necessary because PubSub allows publishing without
-            // items leading to notification-only events.
-            if (!items.isEmpty()) {
-                const auto &deviceListItem = items.constFirst();
-                if (deviceListItem.id() == QXmppPubSubManager::standardItemIdToString(QXmppPubSubManager::Current)) {
-                    d->updateDevices(pubSubService, event.items().constFirst());
+            // That is necessary because PubSub allows publishing without items leading to
+            // notification-only events.
+            if (const auto &items = event.items(); !items.isEmpty()) {
+                // Since the usage of the item ID \c QXmppPubSubManager::Current is only RECOMMENDED
+                // by \xep{0060, Publish-Subscribe} (PubSub) but not obligatory, an appropriate
+                // contact device list is determined.
+                // In case of the own device list node, it is sctrictly processed as a recommended
+                // singleton item and changed to fit that if needed.
+                const auto isOwnDeviceListNode = d->ownBareJid() == pubSubService;
+                if (isOwnDeviceListNode) {
+                    const auto &deviceListItem = items.constFirst();
+                    if (deviceListItem.id() == QXmppPubSubManager::standardItemIdToString(QXmppPubSubManager::Current)) {
+                        d->updateDevices(pubSubService, event.items().constFirst());
+                    } else {
+                        d->handleIrregularDeviceListChanges(pubSubService);
+                    }
                 } else {
-                    d->handleIrregularDeviceListChanges(pubSubService);
+                    d->updateContactDevices(pubSubService, items);
                 }
             }
 
             break;
         }
-        // Items have been retracted.
+        // Specific items are deleted.
         case QXmppPubSubEventBase::Retract: {
-            // Specific items are deleted.
-            const auto &retractedItem = event.retractIds().constFirst();
-            if (retractedItem == QXmppPubSubManager::standardItemIdToString(QXmppPubSubManager::Current)) {
-                d->handleIrregularDeviceListChanges(pubSubService);
-            }
+            d->handleIrregularDeviceListChanges(pubSubService);
         }
         // All items are deleted.
         case QXmppPubSubEventBase::Purge:
diff --git a/src/omemo/QXmppOmemoManager_p.cpp b/src/omemo/QXmppOmemoManager_p.cpp
index 63cb5e00..c4d82e78 100644
--- a/src/omemo/QXmppOmemoManager_p.cpp
+++ b/src/omemo/QXmppOmemoManager_p.cpp
@@ -2650,6 +2650,36 @@ void ManagerPrivate::updateOwnDevicesLocally(bool isDeviceListNodeExistent, Func
     }
 }
 
+//
+// Updates all locally stored devices of a contact.
+//
+// \param deviceOwnerJid bare JID of the devices' owner
+// \param deviceListItems PEP items that may contain a device list
+//
+// \returns a found device list item
+//
+std::optional<QXmppOmemoDeviceListItem> QXmppOmemoManagerPrivate::updateContactDevices(const QString &deviceOwnerJid, const QVector<QXmppOmemoDeviceListItem> &deviceListItems)
+{
+    if (deviceListItems.size() > 1) {
+        const auto itr = std::find_if(deviceListItems.cbegin(), deviceListItems.cend(), [=](const QXmppOmemoDeviceListItem &item) {
+            return item.id() == QXmppPubSubManager::Current;
+        });
+
+        if (itr != deviceListItems.cend()) {
+            updateDevices(deviceOwnerJid, *itr);
+            return *itr;
+        } else {
+            warning("Device list for JID '" % deviceOwnerJid % "' could not be updated because the node contains more than one item but none with the singleton node's specific ID '" % QXmppPubSubManager::standardItemIdToString(QXmppPubSubManager::Current) % "'");
+            handleIrregularDeviceListChanges(deviceOwnerJid);
+            return {};
+        }
+    }
+
+    const auto &item = deviceListItems.constFirst();
+    updateDevices(deviceOwnerJid, item);
+    return item;
+}
+
 //
 // Updates all locally stored devices by a passed device list item.
 //
@@ -3058,16 +3088,27 @@ QXmppTask<bool> ManagerPrivate::changeDeviceLabel(const QString &deviceLabel)
 //
 QXmppTask<QXmppPubSubManager::ItemResult<QXmppOmemoDeviceListItem>> ManagerPrivate::requestDeviceList(const QString &jid)
 {
-    auto future = pubSubManager->requestItem<QXmppOmemoDeviceListItem>(jid, ns_omemo_2_devices, QXmppPubSubManager::Current);
-    future.then(q, [this, jid](QXmppPubSubManager::ItemResult<QXmppOmemoDeviceListItem> result) mutable {
+    QXmppPromise<QXmppPubSubManager::ItemResult<QXmppOmemoDeviceListItem>> interface;
+
+    // Since the usage of the item ID \c QXmppPubSubManager::Current is only RECOMMENDED by
+    // \xep{0060, Publish-Subscribe} (PubSub) but not obligatory, all items are requested even if
+    // the node should contain only one item.
+    auto future = pubSubManager->requestItems<QXmppOmemoDeviceListItem>(jid, ns_omemo_2_devices);
+    future.then(q, [this, interface, jid](QXmppPubSubManager::ItemsResult<QXmppOmemoDeviceListItem> result) mutable {
         if (const auto error = std::get_if<QXmppError>(&result)) {
             warning("Device list for JID '" % jid % "' could not be retrieved: " % errorToString(*error));
+            interface.finish(*error);
+        } else if (const auto &items = std::get<QXmppPubSubManager::Items<QXmppOmemoDeviceListItem>>(result).items; items.isEmpty()) {
+            const auto errorMessage = "Device list for JID '" % jid % "' could not be retrieved because the node does not contain any item";
+            warning(errorMessage);
+            interface.finish(QXmppError { errorMessage });
+        } else if (const auto item = updateContactDevices(jid, items); item) {
+            interface.finish(*item);
         } else {
-            const auto &item = std::get<QXmppOmemoDeviceListItem>(result);
-            updateDevices(jid, item);
+            interface.finish(QXmppError { "Device list for JID '" % jid % "' could not be retrieved because the node does not contain an appropriate item" });
         }
     });
-    return future;
+    return interface.task();
 }
 
 //
diff --git a/src/omemo/QXmppOmemoManager_p.h b/src/omemo/QXmppOmemoManager_p.h
index 96f10f94..0792bdf2 100644
--- a/src/omemo/QXmppOmemoManager_p.h
+++ b/src/omemo/QXmppOmemoManager_p.h
@@ -290,6 +290,7 @@ public:
     QXmppOmemoDeviceListItem deviceListItem(bool addOwnDevice = true);
     template<typename Function>
     void updateOwnDevicesLocally(bool isDeviceListNodeExistent, Function continuation);
+    std::optional<QXmppOmemoDeviceListItem> updateContactDevices(const QString &deviceOwnerJid, const QVector<QXmppOmemoDeviceListItem> &deviceListItems);
     void updateDevices(const QString &deviceOwnerJid, const QXmppOmemoDeviceListItem &deviceListItem);
     void handleIrregularDeviceListChanges(const QString &deviceOwnerJid);
     template<typename Function>
-- 
cgit v1.2.3


From 4985e321d57600faf206e22ed1228b9bd442a4f3 Mon Sep 17 00:00:00 2001
From: Linus Jahn <lnj@kaidan.im>
Date: Mon, 13 Mar 2023 19:51:02 +0100
Subject: IqHandling: Fix use-after-move

Fixes #544.
---
 src/client/QXmppIqHandling.h | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

(limited to 'src')

diff --git a/src/client/QXmppIqHandling.h b/src/client/QXmppIqHandling.h
index 7535bc30..63eba306 100644
--- a/src/client/QXmppIqHandling.h
+++ b/src/client/QXmppIqHandling.h
@@ -95,12 +95,10 @@ namespace Private {
             iq.parse(element);
             iq.setE2eeMetadata(e2eeMetadata);
 
-            processHandleIqResult(
-                client,
-                iq.id(),
-                iq.from(),
-                e2eeMetadata,
-                invokeIqHandler(std::forward<Handler>(handler), std::move(iq)));
+            auto id = iq.id(), from = iq.from();
+
+            processHandleIqResult(client, id, from, e2eeMetadata,
+                                  invokeIqHandler(std::forward<Handler>(handler), std::move(iq)));
             return true;
         }
         return false;
-- 
cgit v1.2.3


From bb9f20419c794e4555860e6aeb097229653d5dc9 Mon Sep 17 00:00:00 2001
From: Melvin Keskin <melvo@olomono.de>
Date: Tue, 7 Mar 2023 23:22:06 +0100
Subject: EME: Always send encryption name as fallback

Since QXmpp does not differentiate between different EME versions
receiving clients support, it is better to always send the encryption name.
It ensures that a name is displayed by the receiving client even if it
does not support the latest EME version introducing a new encryption.
---
 src/base/QXmppMessage.cpp               | 2 +-
 tests/qxmppmessage/tst_qxmppmessage.cpp | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

(limited to 'src')

diff --git a/src/base/QXmppMessage.cpp b/src/base/QXmppMessage.cpp
index c0923d22..9527f5ed 100644
--- a/src/base/QXmppMessage.cpp
+++ b/src/base/QXmppMessage.cpp
@@ -1659,7 +1659,7 @@ void QXmppMessage::serializeExtensions(QXmlStreamWriter *writer, QXmpp::SceMode
             writer->writeStartElement(QStringLiteral("encryption"));
             writer->writeDefaultNamespace(ns_eme);
             writer->writeAttribute(QStringLiteral("namespace"), d->encryptionMethod);
-            helperToXmlAddAttribute(writer, QStringLiteral("name"), d->encryptionName);
+            helperToXmlAddAttribute(writer, QStringLiteral("name"), encryptionName());
             writer->writeEndElement();
         }
 
diff --git a/tests/qxmppmessage/tst_qxmppmessage.cpp b/tests/qxmppmessage/tst_qxmppmessage.cpp
index deca437e..98ca5670 100644
--- a/tests/qxmppmessage/tst_qxmppmessage.cpp
+++ b/tests/qxmppmessage/tst_qxmppmessage.cpp
@@ -783,7 +783,7 @@ void tst_QXmppMessage::testEme()
     // test standard encryption: OMEMO
     const QByteArray xmlOmemo(
         "<message to=\"foo@example.com/QXmpp\" from=\"bar@example.com/QXmpp\" type=\"normal\">"
-        "<encryption xmlns=\"urn:xmpp:eme:0\" namespace=\"eu.siacs.conversations.axolotl\"/>"
+        "<encryption xmlns=\"urn:xmpp:eme:0\" namespace=\"eu.siacs.conversations.axolotl\" name=\"OMEMO\"/>"
         "<body>This message is encrypted with OMEMO, but your client doesn&apos;t seem to support that.</body>"
         "</message>");
 
-- 
cgit v1.2.3


From dfe4b2d2759e5a1af43f5f2a74b6416ce5dab229 Mon Sep 17 00:00:00 2001
From: Linus Jahn <lnj@kaidan.im>
Date: Tue, 14 Mar 2023 20:40:06 +0100
Subject: MamManager: Create task before sending

---
 src/client/QXmppMamManager.cpp | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

(limited to 'src')

diff --git a/src/client/QXmppMamManager.cpp b/src/client/QXmppMamManager.cpp
index c39a3411..a63c997d 100644
--- a/src/client/QXmppMamManager.cpp
+++ b/src/client/QXmppMamManager.cpp
@@ -243,6 +243,9 @@ QXmppTask<QXmppMamManager::RetrieveResult> QXmppMamManager::retrieveMessages(con
 
     auto [itr, _] = d->ongoingRequests.insert({ queryIq.queryId().toStdString(), RetrieveRequestState() });
 
+    // create task here; promise could finish immediately after client()->sendIq()
+    auto task = itr->second.promise.task();
+
     // retrieve messages
     client()->sendIq(std::move(queryIq)).then(this, [this, queryId = queryIq.queryId()](QXmppClient::IqResult result) {
         auto itr = d->ongoingRequests.find(queryId.toStdString());
@@ -309,5 +312,5 @@ QXmppTask<QXmppMamManager::RetrieveResult> QXmppMamManager::retrieveMessages(con
         }
     });
 
-    return itr->second.promise.task();
+    return task;
 }
-- 
cgit v1.2.3


From 567b58a7a6dabf105a4d013b07c9937fbd68fd24 Mon Sep 17 00:00:00 2001
From: Linus Jahn <lnj@kaidan.im>
Date: Tue, 14 Mar 2023 20:42:50 +0100
Subject: MamManager: Avoid possible use-after-move

---
 src/client/QXmppMamManager.cpp | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

(limited to 'src')

diff --git a/src/client/QXmppMamManager.cpp b/src/client/QXmppMamManager.cpp
index a63c997d..a5b1ce5c 100644
--- a/src/client/QXmppMamManager.cpp
+++ b/src/client/QXmppMamManager.cpp
@@ -240,14 +240,16 @@ QString QXmppMamManager::retrieveArchivedMessages(const QString &to,
 QXmppTask<QXmppMamManager::RetrieveResult> QXmppMamManager::retrieveMessages(const QString &to, const QString &node, const QString &jid, const QDateTime &start, const QDateTime &end, const QXmppResultSetQuery &resultSetQuery)
 {
     auto queryIq = buildRequest(to, node, jid, start, end, resultSetQuery);
+    auto queryId = queryIq.queryId();
 
-    auto [itr, _] = d->ongoingRequests.insert({ queryIq.queryId().toStdString(), RetrieveRequestState() });
+    auto [itr, inserted] = d->ongoingRequests.insert({ queryIq.queryId().toStdString(), RetrieveRequestState() });
+    Q_ASSERT(inserted);
 
     // create task here; promise could finish immediately after client()->sendIq()
     auto task = itr->second.promise.task();
 
     // retrieve messages
-    client()->sendIq(std::move(queryIq)).then(this, [this, queryId = queryIq.queryId()](QXmppClient::IqResult result) {
+    client()->sendIq(std::move(queryIq)).then(this, [this, queryId](QXmppClient::IqResult result) {
         auto itr = d->ongoingRequests.find(queryId.toStdString());
         if (itr == d->ongoingRequests.end()) {
             return;
-- 
cgit v1.2.3


From fa45d75aaadfe9e1d07911cd9483932abaa55117 Mon Sep 17 00:00:00 2001
From: Linus Jahn <lnj@kaidan.im>
Date: Tue, 14 Mar 2023 20:52:40 +0100
Subject: MamManager: Move parsing from handleStanza() into new function

---
 src/client/QXmppMamManager.cpp | 55 +++++++++++++++++++++++++-----------------
 1 file changed, 33 insertions(+), 22 deletions(-)

(limited to 'src')

diff --git a/src/client/QXmppMamManager.cpp b/src/client/QXmppMamManager.cpp
index a5b1ce5c..d96b2791 100644
--- a/src/client/QXmppMamManager.cpp
+++ b/src/client/QXmppMamManager.cpp
@@ -20,6 +20,37 @@
 
 using namespace QXmpp::Private;
 
+std::optional<std::tuple<QXmppMessage, QString>> parseMamMessageResult(const QDomElement &messageEl)
+{
+    auto resultElement = messageEl.firstChildElement("result");
+    if (resultElement.isNull() || resultElement.namespaceURI() != ns_mam) {
+        return {};
+    }
+
+    auto forwardedElement = resultElement.firstChildElement("forwarded");
+    if (forwardedElement.isNull() || forwardedElement.namespaceURI() != ns_forwarding) {
+        return {};
+    }
+
+    auto queryId = resultElement.attribute("queryid");
+
+    auto messageElement = forwardedElement.firstChildElement("message");
+    if (messageElement.isNull()) {
+        return {};
+    }
+
+    QXmppMessage message;
+    message.parse(messageElement);
+
+    auto delayElement = forwardedElement.firstChildElement("delay");
+    if (!delayElement.isNull() && delayElement.namespaceURI() == ns_delayed_delivery) {
+        const auto stamp = delayElement.attribute("stamp");
+        message.setStamp(QXmppUtils::datetimeFromString(stamp));
+    }
+
+    return { { message, queryId } };
+}
+
 struct RetrieveRequestState
 {
     QXmppPromise<QXmppMamManager::RetrieveResult> promise;
@@ -87,28 +118,8 @@ QStringList QXmppMamManager::discoveryFeatures() const
 bool QXmppMamManager::handleStanza(const QDomElement &element)
 {
     if (element.tagName() == "message") {
-        QDomElement resultElement = element.firstChildElement("result");
-        if (!resultElement.isNull() && resultElement.namespaceURI() == ns_mam) {
-            QDomElement forwardedElement = resultElement.firstChildElement("forwarded");
-            QString queryId = resultElement.attribute("queryid");
-
-            if (forwardedElement.isNull() || forwardedElement.namespaceURI() != ns_forwarding) {
-                return false;
-            }
-
-            auto messageElement = forwardedElement.firstChildElement("message");
-            auto delayElement = forwardedElement.firstChildElement("delay");
-
-            if (messageElement.isNull()) {
-                return false;
-            }
-
-            QXmppMessage message;
-            message.parse(messageElement);
-            if (!delayElement.isNull() && delayElement.namespaceURI() == ns_delayed_delivery) {
-                const QString stamp = delayElement.attribute("stamp");
-                message.setStamp(QXmppUtils::datetimeFromString(stamp));
-            }
+        if (auto result = parseMamMessageResult(element)) {
+            auto &[message, queryId] = *result;
 
             auto itr = d->ongoingRequests.find(queryId.toStdString());
             if (itr != d->ongoingRequests.end()) {
-- 
cgit v1.2.3


From 672af91550cbfbe622a058de32df58e805468f1e Mon Sep 17 00:00:00 2001
From: Linus Jahn <lnj@kaidan.im>
Date: Tue, 14 Mar 2023 21:12:43 +0100
Subject: MamManager: Flatten retrieveMessages code

---
 src/client/QXmppMamManager.cpp | 108 ++++++++++++++++++++++-------------------
 1 file changed, 58 insertions(+), 50 deletions(-)

(limited to 'src')

diff --git a/src/client/QXmppMamManager.cpp b/src/client/QXmppMamManager.cpp
index d96b2791..2bd0db23 100644
--- a/src/client/QXmppMamManager.cpp
+++ b/src/client/QXmppMamManager.cpp
@@ -265,64 +265,72 @@ QXmppTask<QXmppMamManager::RetrieveResult> QXmppMamManager::retrieveMessages(con
         if (itr == d->ongoingRequests.end()) {
             return;
         }
+        auto &state = itr->second;
 
-        if (std::holds_alternative<QDomElement>(result)) {
-            auto &iq = itr->second.iq;
-            iq.parse(std::get<QDomElement>(result));
+        // handle IQ sending errors
+        if (std::holds_alternative<QXmppError>(result)) {
+            state.promise.finish(std::get<QXmppError>(result));
+            d->ongoingRequests.erase(itr);
+            return;
+        }
 
-            if (iq.type() == QXmppIq::Error) {
-                itr->second.promise.finish(QXmppError { iq.error().text(), iq.error() });
-                d->ongoingRequests.erase(itr);
-                return;
-            }
+        // parse IQ
+        auto &iq = state.iq;
+        iq.parse(std::get<QDomElement>(result));
+
+        // handle MAM error result IQ
+        if (iq.type() == QXmppIq::Error) {
+            state.promise.finish(QXmppError { iq.error().text(), iq.error() });
+            d->ongoingRequests.erase(itr);
+            return;
+        }
 
-            // decrypt encrypted messages
-            if (auto *e2eeExt = client()->encryptionExtension()) {
-                auto &messages = itr->second.messages;
-                auto running = std::make_shared<uint>(0);
-                // handle case when no message is encrypted
-                auto hasEncryptedMessages = false;
+        // decrypt encrypted messages
+        if (auto *e2eeExt = client()->encryptionExtension()) {
+            auto &messages = state.messages;
+            auto running = std::make_shared<uint>(0);
 
-                for (auto i = 0; i < messages.size(); i++) {
-                    if (!e2eeExt->isEncrypted(messages.at(i))) {
-                        continue;
-                    }
-                    hasEncryptedMessages = true;
-
-                    auto message = messages.at(i);
-                    (*running)++;
-                    e2eeExt->decryptMessage(std::move(message)).then(this, [this, i, running, queryId](auto result) {
-                        (*running)--;
-                        auto itr = d->ongoingRequests.find(queryId.toStdString());
-                        if (itr == d->ongoingRequests.end()) {
-                            return;
-                        }
-
-                        if (std::holds_alternative<QXmppMessage>(result)) {
-                            itr->second.messages[i] = std::get<QXmppMessage>(std::move(result));
-                        } else {
-                            warning(QStringLiteral("Error decrypting message."));
-                        }
-                        if (*running == 0) {
-                            itr->second.finish();
-                            d->ongoingRequests.erase(itr);
-                        }
-                    });
-                }
+            // handle case when no message is encrypted
+            auto hasEncryptedMessages = false;
 
-                if (!hasEncryptedMessages) {
-                    // finish here, no decryptMessage callback will do it
-                    itr->second.finish();
-                    d->ongoingRequests.erase(itr);
+            for (auto i = 0; i < messages.size(); i++) {
+                if (!e2eeExt->isEncrypted(messages.at(i))) {
+                    continue;
                 }
-            } else {
-                itr->second.finish();
-                d->ongoingRequests.erase(itr);
+                hasEncryptedMessages = true;
+
+                auto message = messages.at(i);
+                (*running)++;
+                e2eeExt->decryptMessage(std::move(message)).then(this, [this, i, running, queryId](auto result) {
+                    (*running)--;
+
+                    auto itr = d->ongoingRequests.find(queryId.toStdString());
+                    if (itr == d->ongoingRequests.end()) {
+                        return;
+                    }
+                    auto &state = itr->second;
+
+                    if (std::holds_alternative<QXmppMessage>(result)) {
+                        state.messages[i] = std::get<QXmppMessage>(std::move(result));
+                    } else {
+                        warning(QStringLiteral("Error decrypting message."));
+                    }
+
+                    if (*running == 0) {
+                        state.finish();
+                        d->ongoingRequests.erase(itr);
+                    }
+                });
+            }
+
+            // finishing the promise is done after decryptMessage()
+            if (hasEncryptedMessages) {
+                return;
             }
-        } else {
-            itr->second.promise.finish(std::get<QXmppError>(result));
-            d->ongoingRequests.erase(itr);
         }
+
+        state.finish();
+        d->ongoingRequests.erase(itr);
     });
 
     return task;
-- 
cgit v1.2.3


From 154ac6b989aeee66eb2d3b87802faf009b90a92b Mon Sep 17 00:00:00 2001
From: Linus Jahn <lnj@kaidan.im>
Date: Tue, 14 Mar 2023 21:51:52 +0100
Subject: MamManager: Avoid shared_ptr for counting running jobs

---
 src/client/QXmppMamManager.cpp | 48 ++++++++++++++++++++++++++++++------------
 1 file changed, 34 insertions(+), 14 deletions(-)

(limited to 'src')

diff --git a/src/client/QXmppMamManager.cpp b/src/client/QXmppMamManager.cpp
index 2bd0db23..d7f63eaf 100644
--- a/src/client/QXmppMamManager.cpp
+++ b/src/client/QXmppMamManager.cpp
@@ -20,6 +20,22 @@
 
 using namespace QXmpp::Private;
 
+template<typename T, typename Converter>
+auto transform(const T &input, Converter convert)
+{
+    using Output = std::decay_t<decltype(convert(*input.begin()))>;
+    QVector<Output> output;
+    output.reserve(input.size());
+    std::transform(input.begin(), input.end(), std::back_inserter(output), std::move(convert));
+    return output;
+}
+
+template<typename T>
+auto sum(const T &c)
+{
+    return std::accumulate(c.begin(), c.end(), 0);
+}
+
 std::optional<std::tuple<QXmppMessage, QString>> parseMamMessageResult(const QDomElement &messageEl)
 {
     auto resultElement = messageEl.firstChildElement("result");
@@ -56,6 +72,7 @@ struct RetrieveRequestState
     QXmppPromise<QXmppMamManager::RetrieveResult> promise;
     QXmppMamResultIq iq;
     QVector<QXmppMessage> messages;
+    uint runningDecryptionJobs = 0;
 
     void finish()
     {
@@ -288,26 +305,28 @@ QXmppTask<QXmppMamManager::RetrieveResult> QXmppMamManager::retrieveMessages(con
         // decrypt encrypted messages
         if (auto *e2eeExt = client()->encryptionExtension()) {
             auto &messages = state.messages;
-            auto running = std::make_shared<uint>(0);
 
-            // handle case when no message is encrypted
-            auto hasEncryptedMessages = false;
+            // check for encrypted messages (once)
+            auto messagesEncrypted = transform(state.messages, [&](const auto &m) {
+                return e2eeExt->isEncrypted(m);
+            });
+            auto encryptedCount = sum(messagesEncrypted);
+
+            // We can't do this on the fly (with ++ and --) in the for loop
+            // because some decryptMessage() jobs could finish instantly
+            state.runningDecryptionJobs = encryptedCount;
 
             for (auto i = 0; i < messages.size(); i++) {
-                if (!e2eeExt->isEncrypted(messages.at(i))) {
+                if (!messagesEncrypted[i]) {
                     continue;
                 }
-                hasEncryptedMessages = true;
 
                 auto message = messages.at(i);
-                (*running)++;
-                e2eeExt->decryptMessage(std::move(message)).then(this, [this, i, running, queryId](auto result) {
-                    (*running)--;
-
+                state.runningDecryptionJobs++;
+                e2eeExt->decryptMessage(std::move(message)).then(this, [this, i, queryId](auto result) {
                     auto itr = d->ongoingRequests.find(queryId.toStdString());
-                    if (itr == d->ongoingRequests.end()) {
-                        return;
-                    }
+                    Q_ASSERT(itr != d->ongoingRequests.end());
+
                     auto &state = itr->second;
 
                     if (std::holds_alternative<QXmppMessage>(result)) {
@@ -316,7 +335,8 @@ QXmppTask<QXmppMamManager::RetrieveResult> QXmppMamManager::retrieveMessages(con
                         warning(QStringLiteral("Error decrypting message."));
                     }
 
-                    if (*running == 0) {
+                    state.runningDecryptionJobs--;
+                    if (state.runningDecryptionJobs == 0) {
                         state.finish();
                         d->ongoingRequests.erase(itr);
                     }
@@ -324,7 +344,7 @@ QXmppTask<QXmppMamManager::RetrieveResult> QXmppMamManager::retrieveMessages(con
             }
 
             // finishing the promise is done after decryptMessage()
-            if (hasEncryptedMessages) {
+            if (encryptedCount > 0) {
                 return;
             }
         }
-- 
cgit v1.2.3


From 054b35de3ea9251ca713209ea73b2b814fb6c0bc Mon Sep 17 00:00:00 2001
From: Linus Jahn <lnj@kaidan.im>
Date: Tue, 14 Mar 2023 22:26:50 +0100
Subject: MamManager: Only parse ScePublic when decrypting messages

Message that are end-to-end-encrypted need to be parsed in ScePublic
mode and should then be decrypted.
In case the decryption fails, the messages are parsed in SceAll (the
normal mode).
---
 src/client/QXmppMamManager.cpp | 70 +++++++++++++++++++++++++++++-------------
 1 file changed, 49 insertions(+), 21 deletions(-)

(limited to 'src')

diff --git a/src/client/QXmppMamManager.cpp b/src/client/QXmppMamManager.cpp
index d7f63eaf..fe0735c7 100644
--- a/src/client/QXmppMamManager.cpp
+++ b/src/client/QXmppMamManager.cpp
@@ -9,15 +9,16 @@
 #include "QXmppConstants_p.h"
 #include "QXmppDataForm.h"
 #include "QXmppE2eeExtension.h"
-#include "QXmppFutureUtils_p.h"
 #include "QXmppMamIq.h"
 #include "QXmppMessage.h"
+#include "QXmppPromise.h"
 #include "QXmppUtils.h"
 
 #include <unordered_map>
 
 #include <QDomElement>
 
+using namespace QXmpp;
 using namespace QXmpp::Private;
 
 template<typename T, typename Converter>
@@ -36,7 +37,26 @@ auto sum(const T &c)
     return std::accumulate(c.begin(), c.end(), 0);
 }
 
-std::optional<std::tuple<QXmppMessage, QString>> parseMamMessageResult(const QDomElement &messageEl)
+struct MamMessage
+{
+    QDomElement element;
+    std::optional<QDateTime> delay;
+};
+
+enum EncryptedType { Unencrypted,
+                     Encrypted };
+
+QXmppMessage parseMamMessage(const MamMessage &mamMessage, EncryptedType encrypted)
+{
+    QXmppMessage m;
+    m.parse(mamMessage.element, encrypted == Encrypted ? ScePublic : SceAll);
+    if (mamMessage.delay) {
+        m.setStamp(*mamMessage.delay);
+    }
+    return m;
+}
+
+std::optional<std::tuple<MamMessage, QString>> parseMamMessageResult(const QDomElement &messageEl)
 {
     auto resultElement = messageEl.firstChildElement("result");
     if (resultElement.isNull() || resultElement.namespaceURI() != ns_mam) {
@@ -55,31 +75,32 @@ std::optional<std::tuple<QXmppMessage, QString>> parseMamMessageResult(const QDo
         return {};
     }
 
-    QXmppMessage message;
-    message.parse(messageElement);
-
-    auto delayElement = forwardedElement.firstChildElement("delay");
-    if (!delayElement.isNull() && delayElement.namespaceURI() == ns_delayed_delivery) {
-        const auto stamp = delayElement.attribute("stamp");
-        message.setStamp(QXmppUtils::datetimeFromString(stamp));
-    }
+    auto parseDelay = [](const auto &forwardedEl) -> std::optional<QDateTime> {
+        auto delayEl = forwardedEl.firstChildElement("delay");
+        if (!delayEl.isNull() && delayEl.namespaceURI() == ns_delayed_delivery) {
+            return QXmppUtils::datetimeFromString(delayEl.attribute("stamp"));
+        }
+        return {};
+    };
 
-    return { { message, queryId } };
+    return { { MamMessage { messageElement, parseDelay(forwardedElement) }, queryId } };
 }
 
 struct RetrieveRequestState
 {
     QXmppPromise<QXmppMamManager::RetrieveResult> promise;
     QXmppMamResultIq iq;
-    QVector<QXmppMessage> messages;
+    QVector<MamMessage> messages;
+    QVector<QXmppMessage> processedMessages;
     uint runningDecryptionJobs = 0;
 
     void finish()
     {
+        Q_ASSERT(messages.count() == processedMessages.count());
         promise.finish(
             QXmppMamManager::RetrievedMessages {
                 std::move(iq),
-                std::move(messages) });
+                std::move(processedMessages) });
     }
 };
 
@@ -144,7 +165,7 @@ bool QXmppMamManager::handleStanza(const QDomElement &element)
                 itr->second.messages.append(std::move(message));
             } else {
                 // signal-based API
-                Q_EMIT archivedMessageReceived(queryId, message);
+                Q_EMIT archivedMessageReceived(queryId, parseMamMessage(message, Unencrypted));
             }
             return true;
         }
@@ -304,11 +325,13 @@ QXmppTask<QXmppMamManager::RetrieveResult> QXmppMamManager::retrieveMessages(con
 
         // decrypt encrypted messages
         if (auto *e2eeExt = client()->encryptionExtension()) {
-            auto &messages = state.messages;
+            // initialize processed messages (we need random access because
+            // decryptMessage() may finish in random order)
+            state.processedMessages.resize(state.messages.size());
 
             // check for encrypted messages (once)
             auto messagesEncrypted = transform(state.messages, [&](const auto &m) {
-                return e2eeExt->isEncrypted(m);
+                return e2eeExt->isEncrypted(m.element);
             });
             auto encryptedCount = sum(messagesEncrypted);
 
@@ -316,25 +339,26 @@ QXmppTask<QXmppMamManager::RetrieveResult> QXmppMamManager::retrieveMessages(con
             // because some decryptMessage() jobs could finish instantly
             state.runningDecryptionJobs = encryptedCount;
 
-            for (auto i = 0; i < messages.size(); i++) {
+            for (auto i = 0; i < state.messages.size(); i++) {
                 if (!messagesEncrypted[i]) {
                     continue;
                 }
 
-                auto message = messages.at(i);
-                state.runningDecryptionJobs++;
-                e2eeExt->decryptMessage(std::move(message)).then(this, [this, i, queryId](auto result) {
+                e2eeExt->decryptMessage(parseMamMessage(state.messages.at(i), Encrypted)).then(this, [this, i, queryId](auto result) {
                     auto itr = d->ongoingRequests.find(queryId.toStdString());
                     Q_ASSERT(itr != d->ongoingRequests.end());
 
                     auto &state = itr->second;
 
+                    // store decrypted message, fallback to encrypted message
                     if (std::holds_alternative<QXmppMessage>(result)) {
-                        state.messages[i] = std::get<QXmppMessage>(std::move(result));
+                        state.processedMessages[i] = std::get<QXmppMessage>(std::move(result));
                     } else {
                         warning(QStringLiteral("Error decrypting message."));
+                        state.processedMessages[i] = parseMamMessage(state.messages[i], Unencrypted);
                     }
 
+                    // finish promise if this was the last job
                     state.runningDecryptionJobs--;
                     if (state.runningDecryptionJobs == 0) {
                         state.finish();
@@ -349,6 +373,10 @@ QXmppTask<QXmppMamManager::RetrieveResult> QXmppMamManager::retrieveMessages(con
             }
         }
 
+        // for the case without decryption, finish here
+        state.processedMessages = transform(state.messages, [](const auto &m) {
+            return parseMamMessage(m, Unencrypted);
+        });
         state.finish();
         d->ongoingRequests.erase(itr);
     });
-- 
cgit v1.2.3


From 7bfb39fe1eb853a95929c16a2692cbb648d7387c Mon Sep 17 00:00:00 2001
From: Linus Jahn <lnj@kaidan.im>
Date: Tue, 14 Mar 2023 23:17:55 +0100
Subject: Client: Don't fill empty 'to' attributes of outgoing IQs

---
 src/base/QXmppStream.cpp           | 4 ++--
 src/base/QXmppStream.h             | 2 +-
 src/client/QXmppOutgoingClient.cpp | 7 ++-----
 3 files changed, 5 insertions(+), 8 deletions(-)

(limited to 'src')

diff --git a/src/base/QXmppStream.cpp b/src/base/QXmppStream.cpp
index 428e8e55..a4921a43 100644
--- a/src/base/QXmppStream.cpp
+++ b/src/base/QXmppStream.cpp
@@ -208,7 +208,7 @@ QXmppTask<QXmpp::SendResult> QXmppStream::send(QXmppPacket &&packet, bool &writt
 ///
 /// \since QXmpp 1.5
 ///
-QXmppTask<QXmppStream::IqResult> QXmppStream::sendIq(QXmppIq &&iq)
+QXmppTask<QXmppStream::IqResult> QXmppStream::sendIq(QXmppIq &&iq, const QString &to)
 {
     using namespace QXmpp;
 
@@ -223,7 +223,7 @@ QXmppTask<QXmppStream::IqResult> QXmppStream::sendIq(QXmppIq &&iq)
         iq.setId(QXmppUtils::generateStanzaUuid());
     }
 
-    return sendIq(QXmppPacket(iq), iq.id(), iq.to());
+    return sendIq(QXmppPacket(iq), iq.id(), to);
 }
 
 ///
diff --git a/src/base/QXmppStream.h b/src/base/QXmppStream.h
index 55dc7967..a7584366 100644
--- a/src/base/QXmppStream.h
+++ b/src/base/QXmppStream.h
@@ -47,7 +47,7 @@ public:
     QXmppTask<QXmpp::SendResult> send(QXmppPacket &&);
 
     using IqResult = std::variant<QDomElement, QXmppError>;
-    QXmppTask<IqResult> sendIq(QXmppIq &&);
+    QXmppTask<IqResult> sendIq(QXmppIq &&, const QString &to);
     QXmppTask<IqResult> sendIq(QXmppPacket &&, const QString &id, const QString &to);
     void cancelOngoingIqs();
     bool hasIqId(const QString &id) const;
diff --git a/src/client/QXmppOutgoingClient.cpp b/src/client/QXmppOutgoingClient.cpp
index 20e88b9c..53c02e41 100644
--- a/src/client/QXmppOutgoingClient.cpp
+++ b/src/client/QXmppOutgoingClient.cpp
@@ -328,11 +328,8 @@ bool QXmppOutgoingClient::isStreamResumed() const
 ///
 QXmppTask<QXmppStream::IqResult> QXmppOutgoingClient::sendIq(QXmppIq &&iq)
 {
-    // always set a to address (the QXmppStream needs this for matching)
-    if (iq.to().isEmpty()) {
-        iq.setTo(d->config.domain());
-    }
-    return QXmppStream::sendIq(std::move(iq));
+    auto to = iq.to();
+    return QXmppStream::sendIq(std::move(iq), to.isEmpty() ? d->config.domain() : to);
 }
 
 void QXmppOutgoingClient::_q_socketDisconnected()
-- 
cgit v1.2.3


From 98abdcac77c5fbf47ac955645995d9ff83c2a98b Mon Sep 17 00:00:00 2001
From: Linus Jahn <lnj@kaidan.im>
Date: Tue, 14 Mar 2023 23:18:47 +0100
Subject: Client: Fix empty to in IQs is interpreted as server domain

Empty to means account bare JID.
---
 src/client/QXmppOutgoingClient.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

(limited to 'src')

diff --git a/src/client/QXmppOutgoingClient.cpp b/src/client/QXmppOutgoingClient.cpp
index 53c02e41..4aaed0e7 100644
--- a/src/client/QXmppOutgoingClient.cpp
+++ b/src/client/QXmppOutgoingClient.cpp
@@ -328,8 +328,9 @@ bool QXmppOutgoingClient::isStreamResumed() const
 ///
 QXmppTask<QXmppStream::IqResult> QXmppOutgoingClient::sendIq(QXmppIq &&iq)
 {
+    // If 'to' is empty the user's bare JID is meant implicitly (see RFC6120, section 10.3.3.).
     auto to = iq.to();
-    return QXmppStream::sendIq(std::move(iq), to.isEmpty() ? d->config.domain() : to);
+    return QXmppStream::sendIq(std::move(iq), to.isEmpty() ? d->config.jidBare() : to);
 }
 
 void QXmppOutgoingClient::_q_socketDisconnected()
-- 
cgit v1.2.3


From a811aeb01bb0af1197434f73f6f1136e9cdc4873 Mon Sep 17 00:00:00 2001
From: Linus Jahn <lnj@kaidan.im>
Date: Tue, 14 Mar 2023 23:28:54 +0100
Subject: CarbonManagerV2: Don't set 'to' address of carbon enable IQ

RFC6120 says we MUST NOT set a to address for such stanzas.

From section 8.1.1.1.:

   2.  A stanza sent from a client to a server for direct processing by
       the server (e.g., roster processing as described in [XMPP-IM] or
       presence sent to the server for broadcasting to other entities)
       MUST NOT possess a 'to' attribute.
---
 src/client/QXmppCarbonManagerV2.cpp | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

(limited to 'src')

diff --git a/src/client/QXmppCarbonManagerV2.cpp b/src/client/QXmppCarbonManagerV2.cpp
index 7959cab3..97831731 100644
--- a/src/client/QXmppCarbonManagerV2.cpp
+++ b/src/client/QXmppCarbonManagerV2.cpp
@@ -18,10 +18,9 @@ using namespace QXmpp::Private;
 class CarbonEnableIq : public QXmppIq
 {
 public:
-    CarbonEnableIq(const QString &jid)
+    CarbonEnableIq()
         : QXmppIq()
     {
-        setTo(jid);
         setType(QXmppIq::Set);
     }
 
@@ -165,7 +164,7 @@ void QXmppCarbonManagerV2::enableCarbons()
         return;
     }
 
-    client()->sendIq(CarbonEnableIq(client()->configuration().jidBare())).then(this, [this](QXmppClient::IqResult domResult) {
+    client()->sendIq(CarbonEnableIq()).then(this, [this](QXmppClient::IqResult domResult) {
         if (auto err = parseIq(std::move(domResult))) {
             warning("Could not enable message carbons: " % err->description);
         } else {
-- 
cgit v1.2.3


From 67d75d5adc5915b5fb83fc1578b35724dae6185b Mon Sep 17 00:00:00 2001
From: Melvin Keskin <melvo@olomono.de>
Date: Thu, 16 Mar 2023 21:57:47 +0100
Subject: Stream: IQ handling: Accept responses without 'from' attribute (#556)

See https://xmpp.org/rfcs/rfc6120.html#stanzas-attributes-from-c2s point 3
---
 src/base/QXmppStream.cpp | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

(limited to 'src')

diff --git a/src/base/QXmppStream.cpp b/src/base/QXmppStream.cpp
index a4921a43..4e7c7276 100644
--- a/src/base/QXmppStream.cpp
+++ b/src/base/QXmppStream.cpp
@@ -481,10 +481,18 @@ bool QXmppStream::handleIqResponse(const QDomElement &stanza)
         return false;
     }
 
-    if (auto itr = d->runningIqs.find(stanza.attribute(QStringLiteral("id")));
+    const auto id = stanza.attribute(QStringLiteral("id"));
+    if (auto itr = d->runningIqs.find(id);
         itr != d->runningIqs.end()) {
-        if (stanza.attribute("from") != itr.value().jid) {
-            warning(QStringLiteral("Received IQ response to one of our requests from wrong sender. Ignoring."));
+        const auto expectedFrom = itr.value().jid;
+        // Check that the sender of the response matches the recipient of the request.
+        // Stanzas coming from the server on behalf of the user's account must have no "from"
+        // attribute or have it set to the user's bare JID.
+        // If 'from' is empty, the IQ has been sent by the server. In this case we don't need to
+        // do the check as we trust the server anyways.
+        if (const auto from = stanza.attribute("from"); !from.isEmpty() && from != expectedFrom) {
+            warning(QStringLiteral("Ignored received IQ response to request '%1' because of wrong sender '%2' instead of expected sender '%3'")
+                        .arg(id, from, expectedFrom));
             return false;
         }
 
-- 
cgit v1.2.3