summary refs log tree commit diff
diff options
context:
space:
mode:
-rw-r--r--changelog.d/4699.bugfix1
-rw-r--r--synapse/handlers/federation.py41
-rw-r--r--synapse/storage/event_federation.py22
-rw-r--r--synapse/visibility.py30
4 files changed, 89 insertions, 5 deletions
diff --git a/changelog.d/4699.bugfix b/changelog.d/4699.bugfix
new file mode 100644
index 0000000000..8cd8340cc1
--- /dev/null
+++ b/changelog.d/4699.bugfix
@@ -0,0 +1 @@
+Fix attempting to paginate in rooms where server cannot see any events.
diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py
index f80486102a..0425380e55 100644
--- a/synapse/handlers/federation.py
+++ b/synapse/handlers/federation.py
@@ -858,6 +858,47 @@ class FederationHandler(BaseHandler):
             logger.debug("Not backfilling as no extremeties found.")
             return
 
+        # We only want to paginate if we can actually see the events we'll get,
+        # as otherwise we'll just spend a lot of resources to get redacted
+        # events.
+        #
+        # We do this by filtering all the extremities and seeing if any remain.
+        # Given we don't have the extremity events themselves, we need to
+        # actually check the events that reference them.
+        #
+        # *Note*: the spec wants us to keep backfilling until we reach the start
+        # of the room in case we are allowed to see some of the history. However
+        # in practice that causes more issues than its worth, as a) its
+        # relatively rare for there to be any visible history and b) even when
+        # there is its often sufficiently long ago that clients would stop
+        # attempting to paginate before backfill reached the visible history.
+        #
+        # TODO: If we do do a backfill the we should filter the extremities to
+        #   only include those that point to visible portions of history.
+        #
+        # TODO: Correctly handle the case where we are allowed to see the
+        #   forward event but not the extremity, e.g. in the case of initial
+        #   join of the server where we are allowed to see the join event but
+        #   not anything before it.
+
+        forward_events = yield self.store.get_forward_events(
+            list(extremities),
+        )
+
+        extremities_events = yield self.store.get_events(
+            forward_events,
+            check_redacted=False,
+            get_prev_content=False,
+        )
+
+        filtered_extremities = yield filter_events_for_server(
+            self.store, self.server_name, list(extremities_events.values()),
+            redact=False,
+        )
+
+        if not filtered_extremities:
+            defer.returnValue(False)
+
         # Check if we reached a point where we should start backfilling.
         sorted_extremeties_tuple = sorted(
             extremities.items(),
diff --git a/synapse/storage/event_federation.py b/synapse/storage/event_federation.py
index 38809ed0fc..830b171caa 100644
--- a/synapse/storage/event_federation.py
+++ b/synapse/storage/event_federation.py
@@ -442,6 +442,28 @@ class EventFederationWorkerStore(EventsWorkerStore, SignatureWorkerStore,
         event_results.reverse()
         return event_results
 
+    @defer.inlineCallbacks
+    def get_forward_events(self, event_ids):
+        """Fetch all events that have the given events as a prev event
+
+        Args:
+            event_ids (iterable[str])
+
+        Returns:
+            Deferred[list[str]]
+        """
+        rows = yield self._simple_select_many_batch(
+            table="event_edges",
+            column="prev_event_id",
+            iterable=event_ids,
+            retcols=("event_id",),
+            desc="get_forward_events"
+        )
+
+        defer.returnValue([
+            row["event_id"] for row in rows
+        ])
+
 
 class EventFederationStore(EventFederationWorkerStore):
     """ Responsible for storing and serving up the various graphs associated
diff --git a/synapse/visibility.py b/synapse/visibility.py
index 0281a7c919..f6dcc96630 100644
--- a/synapse/visibility.py
+++ b/synapse/visibility.py
@@ -216,7 +216,20 @@ def filter_events_for_client(store, user_id, events, is_peeking=False,
 
 
 @defer.inlineCallbacks
-def filter_events_for_server(store, server_name, events):
+def filter_events_for_server(store, server_name, events, redact=True):
+    """Filter a list of events based on whether given server is allowed to
+    see them.
+
+    Args:
+        store (DataStore)
+        server_name (str)
+        events (iterable[FrozenEvent])
+        redact (bool): Whether to return a redacted version of the event, or
+            to filter them out entirely.
+
+    Returns
+        Deferred[list[FrozenEvent]]
+    """
     # Whatever else we do, we need to check for senders which have requested
     # erasure of their data.
     erased_senders = yield store.are_users_erased(
@@ -231,7 +244,10 @@ def filter_events_for_server(store, server_name, events):
                 "Sender of %s has been erased, redacting",
                 event.event_id,
             )
-            return prune_event(event)
+            if redact:
+                return prune_event(event)
+            else:
+                return None
 
         # state will be None if we decided we didn't need to filter by
         # room membership.
@@ -265,7 +281,10 @@ def filter_events_for_server(store, server_name, events):
                             return event
                 else:
                     # server has no users in the room: redact
-                    return prune_event(event)
+                    if redact:
+                        return prune_event(event)
+                    else:
+                        return None
 
         return event
 
@@ -361,7 +380,8 @@ def filter_events_for_server(store, server_name, events):
         for e_id, key_to_eid in iteritems(event_to_state_ids)
     }
 
-    defer.returnValue([
+    to_return = (
         redact_disallowed(e, event_to_state[e.event_id])
         for e in events
-    ])
+    )
+    defer.returnValue([e for e in to_return if e is not None])