From ebd98c32281e2c2689480357f7e8ce6084e16384 Mon Sep 17 00:00:00 2001 From: Tobias Markmann Date: Mon, 3 Apr 2017 16:02:33 +0200 Subject: Ignore incoming duplicates of messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This might happen with some servers and their MUC implementation which send you not only the original message but also multiple carbon copies of it for MUC PM conversations. This change will ignore any message that has the same non-empty message ID as the previously incoming message. Test-Information: Added unit test to verify new behaviour. Tested in a MUC where the server would send you the original message and multiple carbon copies of the message. Previously the chat view would show and incoming MUC PM message 4 times. Now it’s only shown once. Builds and tests pass on macOS 10.12.4. Change-Id: Ie7bd29dacc00f8f3962131a529b52a69ff09bd6c diff --git a/Swift/Controllers/Chat/ChatController.cpp b/Swift/Controllers/Chat/ChatController.cpp index 36e12e3..3bc9d20 100644 --- a/Swift/Controllers/Chat/ChatController.cpp +++ b/Swift/Controllers/Chat/ChatController.cpp @@ -554,6 +554,18 @@ void ChatController::logMessage(const std::string& message, const JID& fromJID, } } +bool ChatController::shouldIgnoreMessage(std::shared_ptr message) { + if (!message->getID().empty()) { + if (message->getID() == lastHandledMessageID_) { + return true; + } + else { + lastHandledMessageID_ = message->getID(); + } + } + return false; +} + ChatWindow* ChatController::detachChatWindow() { chatWindow_->onUserTyping.disconnect(boost::bind(&ChatStateNotifier::setUserIsTyping, chatStateNotifier_)); chatWindow_->onUserCancelsTyping.disconnect(boost::bind(&ChatStateNotifier::userCancelledNewMessage, chatStateNotifier_)); diff --git a/Swift/Controllers/Chat/ChatController.h b/Swift/Controllers/Chat/ChatController.h index bae00c8..ffc6be9 100644 --- a/Swift/Controllers/Chat/ChatController.h +++ b/Swift/Controllers/Chat/ChatController.h @@ -47,6 +47,7 @@ namespace Swift { virtual void cancelReplaces() SWIFTEN_OVERRIDE; virtual JID getBaseJID() SWIFTEN_OVERRIDE; virtual void logMessage(const std::string& message, const JID& fromJID, const JID& toJID, const boost::posix_time::ptime& timeStamp, bool isIncoming) SWIFTEN_OVERRIDE; + virtual bool shouldIgnoreMessage(std::shared_ptr message) SWIFTEN_OVERRIDE; private: void handlePresenceChange(std::shared_ptr newPresence); @@ -103,6 +104,7 @@ namespace Swift { std::map ftControllers; SettingsProvider* settings_; std::string lastWbID_; + std::string lastHandledMessageID_; ClientBlockListManager* clientBlockListManager_; boost::signals2::scoped_connection blockingOnStateChangedConnection_; diff --git a/Swift/Controllers/Chat/ChatControllerBase.cpp b/Swift/Controllers/Chat/ChatControllerBase.cpp index f5865ea..ecaf186 100644 --- a/Swift/Controllers/Chat/ChatControllerBase.cpp +++ b/Swift/Controllers/Chat/ChatControllerBase.cpp @@ -212,6 +212,11 @@ bool ChatControllerBase::isFromContact(const JID& from) { } void ChatControllerBase::handleIncomingMessage(std::shared_ptr messageEvent) { + std::shared_ptr message = messageEvent->getStanza(); + if (shouldIgnoreMessage(message)) { + return; + } + preHandleIncomingMessage(messageEvent); if (messageEvent->isReadable() && !messageEvent->getConcluded()) { unreadMessages_.push_back(messageEvent); @@ -220,7 +225,6 @@ void ChatControllerBase::handleIncomingMessage(std::shared_ptr mes } } - std::shared_ptr message = messageEvent->getStanza(); ChatWindow::ChatMessage chatMessage; boost::optional optionalBody = message->getBody(); std::string body = optionalBody.get_value_or(""); diff --git a/Swift/Controllers/Chat/ChatControllerBase.h b/Swift/Controllers/Chat/ChatControllerBase.h index 5ddda52..c84c4d4 100644 --- a/Swift/Controllers/Chat/ChatControllerBase.h +++ b/Swift/Controllers/Chat/ChatControllerBase.h @@ -97,6 +97,9 @@ namespace Swift { virtual void logMessage(const std::string& message, const JID& fromJID, const JID& toJID, const boost::posix_time::ptime& timeStamp, bool isIncoming) = 0; ChatWindow::ChatMessage buildChatWindowChatMessage(const std::string& message, const std::string& senderName, bool senderIsSelf); void updateMessageCount(); + virtual bool shouldIgnoreMessage(std::shared_ptr /* message */) { + return false; + } private: IDGenerator idGenerator_; diff --git a/Swift/Controllers/Chat/UnitTest/ChatsManagerTest.cpp b/Swift/Controllers/Chat/UnitTest/ChatsManagerTest.cpp index 2f77ec7..d104fbd 100644 --- a/Swift/Controllers/Chat/UnitTest/ChatsManagerTest.cpp +++ b/Swift/Controllers/Chat/UnitTest/ChatsManagerTest.cpp @@ -135,6 +135,7 @@ class ChatsManagerTest : public CppUnit::TestFixture { // Carbons tests CPPUNIT_TEST(testCarbonsForwardedIncomingMessageToSecondResource); CPPUNIT_TEST(testCarbonsForwardedOutgoingMessageFromSecondResource); + CPPUNIT_TEST(testCarbonsForwardedIncomingDuplicates); CPPUNIT_TEST_SUITE_END(); @@ -1265,6 +1266,45 @@ public: } } + void testCarbonsForwardedIncomingDuplicates() { + JID messageJID("testling@test.com/resource1"); + JID jid2 = jid_.toBare().withResource("someOtherResource"); + + MockChatWindow* window = new MockChatWindow(); + mocks_->ExpectCall(chatWindowFactory_, ChatWindowFactory::createChatWindow).With(messageJID, uiEventStream_).Return(window); + + std::shared_ptr message(new Message()); + message->setFrom(messageJID); + std::string body("This is a legible message. >HEH@)oeueu"); + message->setBody(body); + manager_->handleIncomingMessage(message); + CPPUNIT_ASSERT_EQUAL(body, MockChatWindow::bodyFromMessage(window->lastAddedMessage_)); + + // incoming carbons message from another resource and duplicate of it + { + auto originalMessage = std::make_shared(); + originalMessage->setFrom(messageJID); + originalMessage->setTo(jid2); + originalMessage->setID("BDD82F0B-2523-48BF-B8CA-17B23A314BC2"); + originalMessage->setType(Message::Chat); + std::string forwardedBody = "Some further text."; + originalMessage->setBody(forwardedBody); + + auto messageWrapper = createCarbonsMessage(std::make_shared(), originalMessage); + + manager_->handleIncomingMessage(messageWrapper); + + CPPUNIT_ASSERT_EQUAL(forwardedBody, MockChatWindow::bodyFromMessage(window->lastAddedMessage_)); + CPPUNIT_ASSERT_EQUAL(false, window->lastAddedMessageSenderIsSelf_); + window->resetLastMessages(); + + messageWrapper = createCarbonsMessage(std::make_shared(), originalMessage); + manager_->handleIncomingMessage(messageWrapper); + CPPUNIT_ASSERT_EQUAL(std::string(), MockChatWindow::bodyFromMessage(window->lastAddedMessage_)); + CPPUNIT_ASSERT_EQUAL(false, window->lastAddedMessageSenderIsSelf_); + } + } + private: std::shared_ptr makeDeliveryReceiptTestMessage(const JID& from, const std::string& id) { std::shared_ptr message = std::make_shared(); diff --git a/Swift/Controllers/Chat/UnitTest/MUCControllerTest.cpp b/Swift/Controllers/Chat/UnitTest/MUCControllerTest.cpp index 59c3a87..e7b4b3e 100644 --- a/Swift/Controllers/Chat/UnitTest/MUCControllerTest.cpp +++ b/Swift/Controllers/Chat/UnitTest/MUCControllerTest.cpp @@ -439,7 +439,7 @@ public: message->setType(Message::Groupchat); message->setTo(self_); message->setFrom(mucJID_.withResource("SomeNickname")); - message->setID(iqChannel_->getNewIQID()); + message->setID("3FB99C56-7C92-4755-91B0-9C0098BC7AE0"); message->setSubject("New Room Subject"); controller_->handleIncomingMessage(std::make_shared(message)); -- cgit v0.10.2-6-g49f6