From 71b625d80806886794c5e72f7ff11432e99b736c Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Wed, 20 Feb 2019 16:54:35 +0000
Subject: [PATCH] Stop backpaginating when events not visible

---
 synapse/handlers/federation.py      | 31 +++++++++++++++++++++++++++++
 synapse/storage/event_federation.py | 22 ++++++++++++++++++++
 synapse/visibility.py               | 30 +++++++++++++++++++++++-----
 3 files changed, 78 insertions(+), 5 deletions(-)

diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py
index 083f2e0ac..7b3834a91 100644
--- a/synapse/handlers/federation.py
+++ b/synapse/handlers/federation.py
@@ -830,6 +830,37 @@ 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 references them.
+        #
+        # TODO: Filter the list of extremities if we do do a backfill
+        # 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.
+
+        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 38809ed0f..830b171ca 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 0281a7c91..f6dcc9663 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])