From 460aa0350c22312829d3e2a3d8556221b3f89173 Mon Sep 17 00:00:00 2001
From: Kitsune Ral <Kitsune-Ral@users.sf.net>
Date: Fri, 29 Mar 2019 13:26:36 +0900
Subject: Room::processStateEvent, User: take the previous membership state
 from oldStateEvent

memberJoinState() just happens to return the not-yet-updated state,
making its use around state changes very sensitive to moving things
around. The event's own prevContent is unsigned, therefore untrusted.
---
 lib/room.cpp | 24 ++++++++++++++++--------
 lib/user.cpp | 13 ++++++-------
 lib/user.h   |  6 +++++-
 3 files changed, 27 insertions(+), 16 deletions(-)

(limited to 'lib')

diff --git a/lib/room.cpp b/lib/room.cpp
index 38f5c4ac..0305cf7b 100644
--- a/lib/room.cpp
+++ b/lib/room.cpp
@@ -1177,7 +1177,11 @@ void Room::Private::insertMemberIntoMap(User *u)
     const auto userName = u->name(q);
     // If there is exactly one namesake of the added user, signal member renaming
     // for that other one because the two should be disambiguated now.
-    auto namesakes = membersMap.values(userName);
+    const auto namesakes = membersMap.values(userName);
+
+    // Callers should check they are not adding an existing user once more.
+    Q_ASSERT(!namesakes.contains(u));
+
     if (namesakes.size() == 1)
         emit q->memberAboutToRename(namesakes.front(),
                                     namesakes.front()->fullName(q));
@@ -2189,16 +2193,20 @@ Room::Changes Room::processStateEvent(const RoomEvent& e)
                 emit avatarChanged();
             return AvatarChange;
         }
-        , [this] (const RoomMemberEvent& evt) {
+        , [this,oldStateEvent] (const RoomMemberEvent& evt) {
             auto* u = user(evt.userId());
-            u->processEvent(evt, this);
-            if (u == localUser() && memberJoinState(u) == JoinState::Invite
+            const auto* oldMemberEvent =
+                    static_cast<const RoomMemberEvent*>(oldStateEvent);
+            u->processEvent(evt, this, oldMemberEvent == nullptr);
+            const auto prevMembership = oldMemberEvent
+                    ? oldMemberEvent->membership() : MembershipType::Leave;
+            if (u == localUser() && evt.membership() == MembershipType::Invite
                     && evt.isDirect())
                 connection()->addToDirectChats(this, user(evt.senderId()));
 
-            if( evt.membership() == MembershipType::Join )
+            if (evt.membership() == MembershipType::Join)
             {
-                if (memberJoinState(u) != JoinState::Join)
+                if (prevMembership != MembershipType::Join)
                 {
                     d->insertMemberIntoMap(u);
                     connect(u, &User::nameAboutToChange, this,
@@ -2214,9 +2222,9 @@ Room::Changes Room::processStateEvent(const RoomEvent& e)
                     emit userAdded(u);
                 }
             }
-            else if( evt.membership() != MembershipType::Join )
+            else if (evt.membership() != MembershipType::Join)
             {
-                if (memberJoinState(u) == JoinState::Join)
+                if (prevMembership == MembershipType::Join)
                 {
                     if (evt.membership() == MembershipType::Invite)
                         qCWarning(MAIN) << "Invalid membership change:" << evt;
diff --git a/lib/user.cpp b/lib/user.cpp
index 93cfbffd..c373a067 100644
--- a/lib/user.cpp
+++ b/lib/user.cpp
@@ -379,19 +379,18 @@ QUrl User::avatarUrl(const Room* room) const
     return avatarObject(room).url();
 }
 
-void User::processEvent(const RoomMemberEvent& event, const Room* room)
+void User::processEvent(const RoomMemberEvent& event, const Room* room,
+                        bool firstMention)
 {
     Q_ASSERT(room);
+
+    if (firstMention)
+        ++d->totalRooms;
+
     if (event.membership() != MembershipType::Invite &&
             event.membership() != MembershipType::Join)
         return;
 
-    auto aboutToEnter = room->memberJoinState(this) == JoinState::Leave &&
-            (event.membership() == MembershipType::Join ||
-             event.membership() == MembershipType::Invite);
-    if (aboutToEnter)
-        ++d->totalRooms;
-
     auto newName = event.displayName();
     // `bridged` value uses the same notification signal as the name;
     // it is assumed that first setting of the bridge occurs together with
diff --git a/lib/user.h b/lib/user.h
index 0023b44a..7c9ed55f 100644
--- a/lib/user.h
+++ b/lib/user.h
@@ -105,7 +105,11 @@ namespace QMatrixClient
             QString avatarMediaId(const Room* room = nullptr) const;
             QUrl avatarUrl(const Room* room = nullptr) const;
 
-            void processEvent(const RoomMemberEvent& event, const Room* r);
+            /// This method is for internal use and should not be called
+            /// from client code
+            // FIXME: Move it away to private in lib 0.6
+            void processEvent(const RoomMemberEvent& event, const Room* r,
+                              bool firstMention);
 
         public slots:
             /** Set a new name in the global user profile */
-- 
cgit v1.2.3