summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--Swift/Controllers/Chat/UnitTest/ChatsManagerTest.cpp49
-rw-r--r--Swift/Controllers/UnitTest/MockChatWindow.h5
-rw-r--r--Swiften/MUC/MUCImpl.cpp125
3 files changed, 119 insertions, 60 deletions
diff --git a/Swift/Controllers/Chat/UnitTest/ChatsManagerTest.cpp b/Swift/Controllers/Chat/UnitTest/ChatsManagerTest.cpp
index e45bcae..31c9be9 100644
--- a/Swift/Controllers/Chat/UnitTest/ChatsManagerTest.cpp
+++ b/Swift/Controllers/Chat/UnitTest/ChatsManagerTest.cpp
@@ -89,6 +89,7 @@ class ChatsManagerTest : public CppUnit::TestFixture {
CPPUNIT_TEST(testChatControllerHighlightingNotificationTesting);
CPPUNIT_TEST(testChatControllerHighlightingNotificationDeduplicateSounds);
CPPUNIT_TEST(testChatControllerMeMessageHandling);
+ CPPUNIT_TEST(testRestartingMUCComponentCrash);
CPPUNIT_TEST(testChatControllerMeMessageHandlingInMUC);
// Carbons tests
@@ -822,6 +823,54 @@ public:
CPPUNIT_ASSERT_EQUAL(std::string("is feeling delighted."), window->bodyFromMessage(window->lastAddedAction_));
}
+ void testRestartingMUCComponentCrash() {
+ JID mucJID = JID("teaparty@rooms.wonderland.lit");
+ JID self = JID("girl@wonderland.lit/rabbithole");
+ std::string nick = "aLiCe";
+
+ MockChatWindow* window;
+
+ auto genRemoteMUCPresence = [=]() {
+ auto presence = Presence::create();
+ presence->setFrom(mucJID.withResource(nick));
+ presence->setTo(self);
+ return presence;
+ };
+
+ // User rejoins.
+ window = new MockChatWindow();
+ mocks_->ExpectCall(chatWindowFactory_, ChatWindowFactory::createChatWindow).With(mucJID, uiEventStream_).Return(window);
+
+ // Join room
+ {
+ auto joinRoomEvent = std::make_shared<JoinMUCUIEvent>(mucJID, boost::optional<std::string>(), nick);
+ uiEventStream_->send(joinRoomEvent);
+ }
+
+ {
+ auto firstPresence = genRemoteMUCPresence();
+ firstPresence->setType(Presence::Unavailable);
+ auto userPayload = std::make_shared<MUCUserPayload>();
+ userPayload->addItem(MUCItem(MUCOccupant::Owner, MUCOccupant::NoRole));
+ firstPresence->addPayload(userPayload);
+ stanzaChannel_->onPresenceReceived(firstPresence);
+ }
+ CPPUNIT_ASSERT_EQUAL(std::string("Couldn't enter room: Unable to enter this room."), MockChatWindow::bodyFromMessage(window->lastAddedErrorMessage_));
+
+ {
+ auto presence = genRemoteMUCPresence();
+ presence->setType(Presence::Unavailable);
+ auto userPayload = std::make_shared<MUCUserPayload>();
+ userPayload->addStatusCode(303);
+ auto item = MUCItem(MUCOccupant::Owner, self, MUCOccupant::Moderator);
+ item.nick = nick;
+ userPayload->addItem(item);
+ userPayload->addStatusCode(110);
+ presence->addPayload(userPayload);
+ stanzaChannel_->onPresenceReceived(presence);
+ }
+ }
+
void testChatControllerMeMessageHandlingInMUC() {
JID mucJID("mucroom@rooms.test.com");
std::string nickname = "toodles";
diff --git a/Swift/Controllers/UnitTest/MockChatWindow.h b/Swift/Controllers/UnitTest/MockChatWindow.h
index d7942ff..054cd31 100644
--- a/Swift/Controllers/UnitTest/MockChatWindow.h
+++ b/Swift/Controllers/UnitTest/MockChatWindow.h
@@ -40,7 +40,9 @@ namespace Swift {
lastAddedPresence_ = message;
}
- virtual void addErrorMessage(const ChatMessage& /*message*/) {}
+ virtual void addErrorMessage(const ChatMessage& message) {
+ lastAddedErrorMessage_ = message;
+ }
virtual void replaceMessage(const ChatMessage& /*message*/, const std::string& /*id*/, const boost::posix_time::ptime& /*time*/) {}
virtual void replaceWithAction(const ChatMessage& /*message*/, const std::string& /*id*/, const boost::posix_time::ptime& /*time*/) {}
virtual void replaceLastMessage(const ChatMessage& message, const TimestampBehaviour /*timestampBehaviour*/) {
@@ -134,6 +136,7 @@ namespace Swift {
ChatMessage lastReplacedMessage_;
ChatMessage lastAddedSystemMessage_;
ChatMessage lastReplacedSystemMessage_;
+ ChatMessage lastAddedErrorMessage_;
JID lastMUCInvitationJID_;
std::vector<SecurityLabelsCatalog::Item> labels_;
bool labelsEnabled_;
diff --git a/Swiften/MUC/MUCImpl.cpp b/Swiften/MUC/MUCImpl.cpp
index 4bab39d..ba8d03e 100644
--- a/Swiften/MUC/MUCImpl.cpp
+++ b/Swiften/MUC/MUCImpl.cpp
@@ -10,7 +10,6 @@
#include <boost/bind.hpp>
-#include <Swiften/Base/foreach.h>
#include <Swiften/Client/StanzaChannel.h>
#include <Swiften/Elements/CapsInfo.h>
#include <Swiften/Elements/Form.h>
@@ -138,7 +137,7 @@ void MUCImpl::handleIncomingPresence(Presence::ref presence) {
}
MUCUserPayload::ref mucPayload;
- foreach (MUCUserPayload::ref payload, presence->getPayloads<MUCUserPayload>()) {
+ for (MUCUserPayload::ref payload : presence->getPayloads<MUCUserPayload>()) {
if (!payload->getItems().empty() || !payload->getStatusCodes().empty()) {
mucPayload = payload;
}
@@ -148,11 +147,10 @@ void MUCImpl::handleIncomingPresence(Presence::ref presence) {
// (i.e. we start getting non-error presence from the MUC) or not
if (!joinSucceeded_) {
if (presence->getType() == Presence::Error) {
- std::string reason;
onJoinFailed(presence->getPayload<ErrorPayload>());
return;
}
- else {
+ else if (presence->getType() == Presence::Available) {
joinSucceeded_ = true;
presenceSender->addDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::DontSendPresence);
if (presenceSender->getLastSentUndirectedPresence() && !isEqualExceptID(**(presenceSender->getLastSentUndirectedPresence()), *joinRequestPresence_)) {
@@ -162,6 +160,10 @@ void MUCImpl::handleIncomingPresence(Presence::ref presence) {
presenceSender->sendPresence(latestPresence);
}
}
+ else if (presence->getType() == Presence::Unavailable) {
+ onJoinFailed(std::shared_ptr<ErrorPayload>());
+ return;
+ }
}
std::string nick = presence->getFrom().getResource();
@@ -188,19 +190,21 @@ void MUCImpl::handleIncomingPresence(Presence::ref presence) {
if (std::dynamic_pointer_cast<MUCDestroyPayload>(mucPayload->getPayload())) {
type = LeaveDestroy;
}
- else foreach (MUCUserPayload::StatusCode status, mucPayload->getStatusCodes()) {
- if (status.code == 307) {
- type = LeaveKick;
- }
- else if (status.code == 301) {
- type = LeaveBan;
- }
- else if (status.code == 321) {
- type = LeaveNotMember;
- }
- else if (status.code == 303) {
- if (mucPayload->getItems().size() == 1) {
- newNickname = mucPayload->getItems()[0].nick;
+ else {
+ for (MUCUserPayload::StatusCode status : mucPayload->getStatusCodes()) {
+ if (status.code == 307) {
+ type = LeaveKick;
+ }
+ else if (status.code == 301) {
+ type = LeaveBan;
+ }
+ else if (status.code == 321) {
+ type = LeaveNotMember;
+ }
+ else if (status.code == 303) {
+ if (mucPayload->getItems().size() == 1) {
+ newNickname = mucPayload->getItems()[0].nick;
+ }
}
}
}
@@ -254,50 +258,53 @@ void MUCImpl::handleIncomingPresence(Presence::ref presence) {
onOccupantJoined(result.first->second);
}
onOccupantPresenceChange(presence);
- }
- if (mucPayload && !joinComplete_) {
- bool isLocked = false;
- foreach (MUCUserPayload::StatusCode status, mucPayload->getStatusCodes()) {
- if (status.code == 110) {
- /* Simply knowing this is your presence is enough, 210 doesn't seem to be necessary. */
- joinComplete_ = true;
- if (ownMUCJID != presence->getFrom()) {
- presenceSender->removeDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::DontSendPresence);
- ownMUCJID = presence->getFrom();
- presenceSender->addDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::AndSendPresence);
- }
- }
- // MUC status 201: a new room has been created
- if (status.code == 201) {
- isLocked = true;
- /* Room is created and locked */
- /* Currently deal with this by making an instant room */
- if (ownMUCJID != presence->getFrom()) {
- presenceSender->removeDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::DontSendPresence);
- ownMUCJID = presence->getFrom();
- presenceSender->addDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::AndSendPresence);
- }
- if (createAsReservedIfNew) {
- unlocking = true;
- requestConfigurationForm();
+
+ if (mucPayload && !joinComplete_) {
+ bool isLocked = false;
+ for (MUCUserPayload::StatusCode status : mucPayload->getStatusCodes()) {
+ if (status.code == 110) {
+ /* Simply knowing this is your presence is enough, 210 doesn't seem to be necessary. */
+ joinComplete_ = true;
+ if (ownMUCJID != presence->getFrom()) {
+ presenceSender->removeDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::DontSendPresence);
+ ownMUCJID = presence->getFrom();
+ presenceSender->addDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::AndSendPresence);
+ }
}
- else {
- // Accept default room configuration and create an instant room http://xmpp.org/extensions/xep-0045.html#createroom-instant
- MUCOwnerPayload::ref mucPayload(new MUCOwnerPayload());
- presenceSender->addDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::DontSendPresence);
- mucPayload->setPayload(std::make_shared<Form>(Form::SubmitType));
- std::shared_ptr< GenericRequest<MUCOwnerPayload> > request = std::make_shared< GenericRequest<MUCOwnerPayload> >(IQ::Set, getJID(), mucPayload, iqRouter_);
- request->onResponse.connect(boost::bind(&MUCImpl::handleCreationConfigResponse, this, _1, _2));
- request->send();
+ // MUC status 201: a new room has been created
+ if (status.code == 201) {
+ isLocked = true;
+ /* Room is created and locked */
+ /* Currently deal with this by making an instant room */
+ if (ownMUCJID != presence->getFrom()) {
+ presenceSender->removeDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::DontSendPresence);
+ ownMUCJID = presence->getFrom();
+ presenceSender->addDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::AndSendPresence);
+ }
+ if (createAsReservedIfNew) {
+ unlocking = true;
+ requestConfigurationForm();
+ }
+ else {
+ // Accept default room configuration and create an instant room http://xmpp.org/extensions/xep-0045.html#createroom-instant
+ MUCOwnerPayload::ref mucPayload(new MUCOwnerPayload());
+ presenceSender->addDirectedPresenceReceiver(ownMUCJID, DirectedPresenceSender::DontSendPresence);
+ mucPayload->setPayload(std::make_shared<Form>(Form::SubmitType));
+ std::shared_ptr< GenericRequest<MUCOwnerPayload> > request = std::make_shared< GenericRequest<MUCOwnerPayload> >(IQ::Set, getJID(), mucPayload, iqRouter_);
+ request->onResponse.connect(boost::bind(&MUCImpl::handleCreationConfigResponse, this, _1, _2));
+ request->send();
+ }
}
}
- }
- if (joinComplete_ && !isLocked) {
- onJoinComplete(getOwnNick());
- }
- if (!isLocked && !isUnlocked_ && (presence->getFrom() == ownMUCJID)) {
- isUnlocked_ = true;
- onUnlocked();
+
+ if (joinComplete_ && !isLocked) {
+ assert(hasOccupant(getOwnNick()));
+ onJoinComplete(getOwnNick());
+ }
+ if (!isLocked && !isUnlocked_ && (presence->getFrom() == ownMUCJID)) {
+ isUnlocked_ = true;
+ onUnlocked();
+ }
}
}
}
@@ -377,7 +384,7 @@ void MUCImpl::handleAffiliationListResponse(MUCAdminPayload::ref payload, ErrorP
}
else {
std::vector<JID> jids;
- foreach (MUCItem item, payload->getItems()) {
+ for (MUCItem item : payload->getItems()) {
if (item.realJID) {
jids.push_back(*item.realJID);
}