From 0cdafbd3966e4c1a431916791768ff4dadfe6176 Mon Sep 17 00:00:00 2001 From: Linus Jahn Date: Mon, 28 Oct 2024 20:32:21 +0100 Subject: [PATCH 1/2] tests: mam: Add tests for task API and with fake encryption --- tests/CMakeLists.txt | 2 +- tests/qxmppmammanager/tst_qxmppmammanager.cpp | 153 ++++++++++++++++++ 2 files changed, 154 insertions(+), 1 deletion(-) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 201c3fd54..951f5f71f 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -40,7 +40,7 @@ add_simple_test(qxmpphttpuploadiq) add_simple_test(qxmppiceconnection) add_simple_test(qxmppiq) add_simple_test(qxmppjingleiq) -add_simple_test(qxmppmammanager) +add_simple_test(qxmppmammanager TestClient.h) add_simple_test(qxmppmixinvitation) add_simple_test(qxmppmixitems) add_simple_test(qxmppmessage) diff --git a/tests/qxmppmammanager/tst_qxmppmammanager.cpp b/tests/qxmppmammanager/tst_qxmppmammanager.cpp index b00748b71..bcc788314 100644 --- a/tests/qxmppmammanager/tst_qxmppmammanager.cpp +++ b/tests/qxmppmammanager/tst_qxmppmammanager.cpp @@ -2,12 +2,45 @@ // // SPDX-License-Identifier: LGPL-2.1-or-later +#include "QXmppE2eeExtension.h" +#include "QXmppFutureUtils_p.h" #include "QXmppMamManager.h" #include "QXmppMessage.h" +#include "TestClient.h" #include "util.h" #include +using namespace QXmpp::Private; + +class EncryptionExtension : public QXmppE2eeExtension +{ +public: + QXmppTask encryptMessage(QXmppMessage &&, const std::optional &) override + { + return makeReadyTask(QXmppError { "it's only a test", QXmpp::SendError::EncryptionError }); + } + QXmppTask decryptMessage(QXmppMessage &&m) override + { + m.setBody(m.e2eeFallbackBody()); + m.setE2eeFallbackBody({}); + return makeReadyTask(std::move(m)); + } + + QXmppTask encryptIq(QXmppIq &&, const std::optional &) override + { + return makeReadyTask(QXmppError { "it's only a test", QXmpp::SendError::EncryptionError }); + } + + QXmppTask decryptIq(const QDomElement &) override + { + return makeReadyTask(QXmppError { "it's only a test", QXmpp::SendError::EncryptionError }); + } + + bool isEncrypted(const QDomElement &e) override { return !e.firstChildElement("test-encrypted").isNull(); }; + bool isEncrypted(const QXmppMessage &) override { return false; }; +}; + class QXmppMamTestHelper : public QObject { Q_OBJECT @@ -39,6 +72,10 @@ class tst_QXmppMamManager : public QObject Q_SLOT void testHandleResultIq_data(); Q_SLOT void testHandleResultIq(); + // test for task-based API + Q_SLOT void retrieveMessagesUnencrypted(); + Q_SLOT void retrieveMessagesEncrypted(); + QXmppMamTestHelper m_helper; QXmppMamManager m_manager; }; @@ -205,6 +242,122 @@ void tst_QXmppMamManager::testHandleResultIq() QCOMPARE(m_helper.m_signalTriggered, accept); } +void tst_QXmppMamManager::retrieveMessagesUnencrypted() +{ + TestClient test; + auto *mam = test.addNewExtension(); + auto task = mam->retrieveMessages("mam.server.org"); + test.expect("" + "" + "" + "urn:xmpp:mam:2" + "" + "" + ""); + mam->handleStanza(xmlToDom("" + "" + "" + "" + "" + "Call me but love, and I'll be new baptized; Henceforth I never will be Romeo." + "" + "" + "" + "")); + mam->handleStanza(xmlToDom("" + "" + "" + "" + "" + "What man art thou that thus bescreen'd in night so stumblest on my counsel?" + "" + "" + "" + "")); + test.inject("" + "" + "" + "28482-98726-73623" + "09af3-cc343-b409f" + "" + "" + ""); + + auto retrieved = expectFutureVariant(task); + QCOMPARE(retrieved.messages.size(), 2); + QCOMPARE(retrieved.messages.at(0).body(), "Call me but love, and I'll be new baptized; Henceforth I never will be Romeo."); + QCOMPARE(retrieved.messages.at(1).body(), "What man art thou that thus bescreen'd in night so stumblest on my counsel?"); + QCOMPARE(retrieved.result.resultSetReply().first(), "28482-98726-73623"); +} + +void tst_QXmppMamManager::retrieveMessagesEncrypted() +{ + TestClient test; + // e2ee + auto e2ee = std::make_unique(); + test.setEncryptionExtension(e2ee.get()); + // mam manager + auto *mam = test.addNewExtension(); + + // start request + auto task = mam->retrieveMessages("mam.server.org"); + test.expect("" + "" + "" + "urn:xmpp:mam:2" + "" + "" + ""); + mam->handleStanza(xmlToDom("" + "" + "" + "" + "" + "" + "Call me but love, and I'll be new baptized; Henceforth I never will be Romeo." + "" + "" + "" + "")); + mam->handleStanza(xmlToDom("" + "" + "" + "" + "" + "What man art thou that thus bescreen'd in night so stumblest on my counsel?" + "" + "" + "" + "")); + test.inject("" + "" + "" + "28482-98726-73623" + "09af3-cc343-b409f" + "" + "" + ""); + + // check results + auto retrieved = expectFutureVariant(task); + QCOMPARE(retrieved.messages.size(), 2); + QCOMPARE(retrieved.messages.at(0).body(), "Call me but love, and I'll be new baptized; Henceforth I never will be Romeo."); + QCOMPARE(retrieved.messages.at(1).body(), "What man art thou that thus bescreen'd in night so stumblest on my counsel?"); + QCOMPARE(retrieved.result.resultSetReply().first(), "28482-98726-73623"); +} + void QXmppMamTestHelper::archivedMessageReceived(const QString &queryId, const QXmppMessage &message) { m_signalTriggered = true; From c58b68171917b27d4e156afb4d02543ffa542a1f Mon Sep 17 00:00:00 2001 From: Linus Jahn Date: Mon, 28 Oct 2024 21:30:59 +0100 Subject: [PATCH 2/2] MamManager: Fix unencrypted messages not decrypted with e2ee There were also some async issues that are now solved, especially when all decryption jobs finished instantly (or no message were encrypted). --- src/client/QXmppMamManager.cpp | 87 +++++++++++++++++----------------- 1 file changed, 43 insertions(+), 44 deletions(-) diff --git a/src/client/QXmppMamManager.cpp b/src/client/QXmppMamManager.cpp index 9ffe3df9a..1ec690460 100644 --- a/src/client/QXmppMamManager.cpp +++ b/src/client/QXmppMamManager.cpp @@ -321,58 +321,57 @@ QXmppTask QXmppMamManager::retrieveMessages(con // 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.element); - }); - 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; - - int size = state.messages.size(); - for (auto i = 0; i < size; i++) { - if (!messagesEncrypted[i]) { - continue; - } - - 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(result)) { - state.processedMessages[i] = std::get(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 = state.messages.size(); + + const auto size = state.messages.size(); + for (qsizetype i = 0; i < size; i++) { + const auto &message = state.messages.at(i); + + // decrypt message if needed + if (e2eeExt->isEncrypted(message.element)) { + e2eeExt->decryptMessage(parseMamMessage(state.messages.at(i), Encrypted)).then(this, [this, i, queryId](auto result) { + // find state (again) + 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(result)) { + state.processedMessages[i] = std::get(std::move(result)); + } else { + warning("Error decrypting message."); + state.processedMessages[i] = parseMamMessage(state.messages[i], Unencrypted); + } + + // finish promise on last job + state.runningDecryptionJobs--; + if (state.runningDecryptionJobs == 0) { + state.finish(); + d->ongoingRequests.erase(itr); + } + }); + } else { + state.processedMessages[i] = parseMamMessage(state.messages.at(i), Unencrypted); + + // finish promise on last job (may be needed if no messages are encrypted or + // decryption finishes instantly) state.runningDecryptionJobs--; if (state.runningDecryptionJobs == 0) { state.finish(); d->ongoingRequests.erase(itr); } - }); + } } + } else { + // for the case without decryption + state.processedMessages = transform(state.messages, [](const auto &m) { + return parseMamMessage(m, Unencrypted); + }); - // finishing the promise is done after decryptMessage() - if (encryptedCount > 0) { - return; - } + state.finish(); + d->ongoingRequests.erase(itr); } - - // 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); }); return task;