Skip to content

Commit

Permalink
Stop get_joined_users corruption from custom statuses (matrix-org#7376
Browse files Browse the repository at this point in the history
)

Fix a bug where the `get_joined_users` cache could be corrupted by custom
status events (or other state events with a state_key matching the user ID).

The bug was introduced by matrix-org#2229, but has largely gone unnoticed since then.

Fixes matrix-org#7099, matrix-org#7373.
  • Loading branch information
richvdh authored and phil-flex committed Jun 16, 2020
1 parent a5e4310 commit a1651e5
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 2 deletions.
1 change: 1 addition & 0 deletions changelog.d/7376.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix a bug which could cause messages not to be sent over federation, when state events with state keys matching user IDs (such as custom user statuses) were received.
3 changes: 2 additions & 1 deletion synapse/storage/data_stores/main/roommember.py
Original file line number Diff line number Diff line change
Expand Up @@ -566,7 +566,8 @@ def _get_joined_users_from_context(
if key[0] == EventTypes.Member
]
for etype, state_key in context.delta_ids:
users_in_room.pop(state_key, None)
if etype == EventTypes.Member:
users_in_room.pop(state_key, None)

# We check if we have any of the member event ids in the event cache
# before we ask the DB
Expand Down
50 changes: 49 additions & 1 deletion tests/storage/test_roommember.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
from synapse.types import Requester, UserID

from tests import unittest
from tests.test_utils import event_injection
from tests.utils import TestHomeServer


class RoomMemberStoreTestCase(unittest.HomeserverTestCase):
Expand All @@ -38,7 +40,7 @@ def make_homeserver(self, reactor, clock):
)
return hs

def prepare(self, reactor, clock, hs):
def prepare(self, reactor, clock, hs: TestHomeServer):

# We can't test the RoomMemberStore on its own without the other event
# storage logic
Expand Down Expand Up @@ -114,6 +116,52 @@ def test_count_known_servers_stat_counter_enabled(self):
# It now knows about Charlie's server.
self.assertEqual(self.store._known_servers_count, 2)

def test_get_joined_users_from_context(self):
room = self.helper.create_room_as(self.u_alice, tok=self.t_alice)
bob_event = event_injection.inject_member_event(
self.hs, room, self.u_bob, Membership.JOIN
)

# first, create a regular event
event, context = event_injection.create_event(
self.hs,
room_id=room,
sender=self.u_alice,
prev_event_ids=[bob_event.event_id],
type="m.test.1",
content={},
)

users = self.get_success(
self.store.get_joined_users_from_context(event, context)
)
self.assertEqual(users.keys(), {self.u_alice, self.u_bob})

# Regression test for #7376: create a state event whose key matches bob's
# user_id, but which is *not* a membership event, and persist that; then check
# that `get_joined_users_from_context` returns the correct users for the next event.
non_member_event = event_injection.inject_event(
self.hs,
room_id=room,
sender=self.u_bob,
prev_event_ids=[bob_event.event_id],
type="m.test.2",
state_key=self.u_bob,
content={},
)
event, context = event_injection.create_event(
self.hs,
room_id=room,
sender=self.u_alice,
prev_event_ids=[non_member_event.event_id],
type="m.test.3",
content={},
)
users = self.get_success(
self.store.get_joined_users_from_context(event, context)
)
self.assertEqual(users.keys(), {self.u_alice, self.u_bob})


class CurrentStateMembershipUpdateTestCase(unittest.HomeserverTestCase):
def prepare(self, reactor, clock, homeserver):
Expand Down

0 comments on commit a1651e5

Please sign in to comment.