Discard retention policies when retrieving state

Purge jobs don't delete the latest event in a room in order to keep the forward extremity and not break the room. On the other hand, get_state_events, when given an at_token argument calls filter_events_for_client to know if the user can see the event that matches that (sync) token. That function uses the retention policies of the events it's given to filter out those that are too old from a client's view.

Some clients, such as Riot, when loading a room, request the list of members for the latest sync token it knows about, and get confused to the point of refusing to send any message if the server tells it that it can't get that information. This can happen very easily with the message retention feature turned on and a room with low activity so that the last event sent becomes too old according to the room's retention policy.

An easy and clean fix for that issue is to discard the room's retention policies when retrieving state.

2 files changed, 15 insertions, 9 deletions

diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py
index 155ed6e06a..3b0156f516 100644
--- a/synapse/handlers/message.py
+++ b/synapse/handlers/message.py
@@ -138,7 +138,7 @@ class MessageHandler(object):
                 raise NotFoundError("Can't find event for token %s" % (at_token,))
 
             visible_events = yield filter_events_for_client(
-                self.storage, user_id, last_events
+                self.storage, user_id, last_events, apply_retention_policies=False
             )
 
             event = last_events[0]
diff --git a/synapse/visibility.py b/synapse/visibility.py
index 4d4141dacc..7b037eeb0c 100644
--- a/synapse/visibility.py
+++ b/synapse/visibility.py
@@ -44,7 +44,8 @@ MEMBERSHIP_PRIORITY = (
 
 @defer.inlineCallbacks
 def filter_events_for_client(
-    storage: Storage, user_id, events, is_peeking=False, always_include_ids=frozenset()
+    storage: Storage, user_id, events, is_peeking=False, always_include_ids=frozenset(),
+    apply_retention_policies=True,
 ):
     """
     Check which events a user is allowed to see
@@ -59,6 +60,10 @@ def filter_events_for_client(
             events
         always_include_ids (set(event_id)): set of event ids to specifically
             include (unless sender is ignored)
+        apply_retention_policies (bool): Whether to filter out events that's older than
+            allowed by the room's retention policy. Useful when this function is called
+            to e.g. check whether a user should be allowed to see the state at a given
+            event rather than to know if it should send an event to a user's client(s).
 
     Returns:
         Deferred[list[synapse.events.EventBase]]
@@ -86,13 +91,14 @@ def filter_events_for_client(
 
     erased_senders = yield storage.main.are_users_erased((e.sender for e in events))
 
-    room_ids = set(e.room_id for e in events)
-    retention_policies = {}
+    if apply_retention_policies:
+        room_ids = set(e.room_id for e in events)
+        retention_policies = {}
 
-    for room_id in room_ids:
-        retention_policies[room_id] = yield storage.main.get_retention_policy_for_room(
-            room_id
-        )
+        for room_id in room_ids:
+            retention_policies[room_id] = (
+                yield storage.main.get_retention_policy_for_room(room_id)
+            )
 
     def allowed(event):
         """
@@ -113,7 +119,7 @@ def filter_events_for_client(
 
         # Don't try to apply the room's retention policy if the event is a state event, as
         # MSC1763 states that retention is only considered for non-state events.
-        if not event.is_state():
+        if apply_retention_policies and not event.is_state():
             retention_policy = retention_policies[event.room_id]
             max_lifetime = retention_policy.get("max_lifetime")