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.
This commit is contained in:
Brendan Abolivier 2019-11-28 19:26:13 +00:00
parent a9c44d4008
commit 708cef88cf
No known key found for this signature in database
GPG Key ID: 1E015C145F1916CD
2 changed files with 15 additions and 9 deletions

View File

@ -138,7 +138,7 @@ class MessageHandler(object):
raise NotFoundError("Can't find event for token %s" % (at_token,)) raise NotFoundError("Can't find event for token %s" % (at_token,))
visible_events = yield filter_events_for_client( 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] event = last_events[0]

View File

@ -44,7 +44,8 @@ MEMBERSHIP_PRIORITY = (
@defer.inlineCallbacks @defer.inlineCallbacks
def filter_events_for_client( 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 Check which events a user is allowed to see
@ -59,6 +60,10 @@ def filter_events_for_client(
events events
always_include_ids (set(event_id)): set of event ids to specifically always_include_ids (set(event_id)): set of event ids to specifically
include (unless sender is ignored) 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: Returns:
Deferred[list[synapse.events.EventBase]] Deferred[list[synapse.events.EventBase]]
@ -86,12 +91,13 @@ def filter_events_for_client(
erased_senders = yield storage.main.are_users_erased((e.sender for e in events)) erased_senders = yield storage.main.are_users_erased((e.sender for e in events))
if apply_retention_policies:
room_ids = set(e.room_id for e in events) room_ids = set(e.room_id for e in events)
retention_policies = {} retention_policies = {}
for room_id in room_ids: for room_id in room_ids:
retention_policies[room_id] = yield storage.main.get_retention_policy_for_room( retention_policies[room_id] = (
room_id yield storage.main.get_retention_policy_for_room(room_id)
) )
def allowed(event): 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 # 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. # 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] retention_policy = retention_policies[event.room_id]
max_lifetime = retention_policy.get("max_lifetime") max_lifetime = retention_policy.get("max_lifetime")