diff options
| -rw-r--r-- | changelog.d/10883.misc | 1 | ||||
| -rw-r--r-- | synapse/events/snapshot.py | 14 | ||||
| -rw-r--r-- | synapse/handlers/federation.py | 9 | ||||
| -rw-r--r-- | synapse/handlers/federation_event.py | 7 | ||||
| -rw-r--r-- | synapse/state/__init__.py | 34 |
5 files changed, 21 insertions, 44 deletions
diff --git a/changelog.d/10883.misc b/changelog.d/10883.misc new file mode 100644 index 0000000000..9a765435db --- /dev/null +++ b/changelog.d/10883.misc @@ -0,0 +1 @@ +Clean up some of the federation event authentication code for clarity. diff --git a/synapse/events/snapshot.py b/synapse/events/snapshot.py index f8d898c3b1..5ba01eeef9 100644 --- a/synapse/events/snapshot.py +++ b/synapse/events/snapshot.py @@ -80,9 +80,7 @@ class EventContext: (type, state_key) -> event_id - FIXME: what is this for an outlier? it seems ill-defined. It seems like - it could be either {}, or the state we were given by the remote - server, depending on $THINGS + For an outlier, this is {} Note that this is a private attribute: it should be accessed via ``get_current_state_ids``. _AsyncEventContext impl calculates this @@ -96,7 +94,7 @@ class EventContext: (type, state_key) -> event_id - FIXME: again, what is this for an outlier? + For an outlier, this is {} As with _current_state_ids, this is a private attribute. It should be accessed via get_prev_state_ids. @@ -130,6 +128,14 @@ class EventContext: delta_ids=delta_ids, ) + @staticmethod + def for_outlier(): + """Return an EventContext instance suitable for persisting an outlier event""" + return EventContext( + current_state_ids={}, + prev_state_ids={}, + ) + async def serialize(self, event: EventBase, store: "DataStore") -> dict: """Converts self to a type that can be serialized as JSON, and then deserialized by `deserialize` diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index a03d77dffd..0befe9ce43 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -624,7 +624,7 @@ class FederationHandler(BaseHandler): # in the invitee's sync stream. It is stripped out for all other local users. event.unsigned["knock_room_state"] = stripped_room_state["knock_state_events"] - context = await self.state_handler.compute_event_context(event) + context = EventContext.for_outlier() stream_id = await self._federation_event_handler.persist_events_and_notify( event.room_id, [(event, context)] ) @@ -814,7 +814,7 @@ class FederationHandler(BaseHandler): ) ) - context = await self.state_handler.compute_event_context(event) + context = EventContext.for_outlier() await self._federation_event_handler.persist_events_and_notify( event.room_id, [(event, context)] ) @@ -843,7 +843,7 @@ class FederationHandler(BaseHandler): await self.federation_client.send_leave(host_list, event) - context = await self.state_handler.compute_event_context(event) + context = EventContext.for_outlier() stream_id = await self._federation_event_handler.persist_events_and_notify( event.room_id, [(event, context)] ) @@ -1115,8 +1115,7 @@ class FederationHandler(BaseHandler): events_to_context = {} for e in itertools.chain(auth_events, state): e.internal_metadata.outlier = True - ctx = await self.state_handler.compute_event_context(e) - events_to_context[e.event_id] = ctx + events_to_context[e.event_id] = EventContext.for_outlier() event_map = { e.event_id: e for e in itertools.chain(auth_events, state, [event]) diff --git a/synapse/handlers/federation_event.py b/synapse/handlers/federation_event.py index 3b95beeb08..10b3fdc222 100644 --- a/synapse/handlers/federation_event.py +++ b/synapse/handlers/federation_event.py @@ -1221,7 +1221,7 @@ class FederationEventHandler: async def prep(ev_info: _NewEventInfo) -> EventContext: event = ev_info.event with nested_logging_context(suffix=event.event_id): - res = await self._state_handler.compute_event_context(event) + res = EventContext.for_outlier() res = await self._check_event_auth( origin, event, @@ -1540,10 +1540,7 @@ class FederationEventHandler: event.event_id, auth_event.event_id, ) - missing_auth_event_context = ( - await self._state_handler.compute_event_context(auth_event) - ) - + missing_auth_event_context = EventContext.for_outlier() missing_auth_event_context = await self._check_event_auth( origin, auth_event, diff --git a/synapse/state/__init__.py b/synapse/state/__init__.py index 463ce58dae..c981df3f18 100644 --- a/synapse/state/__init__.py +++ b/synapse/state/__init__.py @@ -263,7 +263,9 @@ class StateHandler: async def compute_event_context( self, event: EventBase, old_state: Optional[Iterable[EventBase]] = None ) -> EventContext: - """Build an EventContext structure for the event. + """Build an EventContext structure for a non-outlier event. + + (for an outlier, call EventContext.for_outlier directly) This works out what the current state should be for the event, and generates a new state group if necessary. @@ -278,35 +280,7 @@ class StateHandler: The event context. """ - if event.internal_metadata.is_outlier(): - # If this is an outlier, then we know it shouldn't have any current - # state. Certainly store.get_current_state won't return any, and - # persisting the event won't store the state group. - - # FIXME: why do we populate current_state_ids? I thought the point was - # that we weren't supposed to have any state for outliers? - if old_state: - prev_state_ids = {(s.type, s.state_key): s.event_id for s in old_state} - if event.is_state(): - current_state_ids = dict(prev_state_ids) - key = (event.type, event.state_key) - current_state_ids[key] = event.event_id - else: - current_state_ids = prev_state_ids - else: - current_state_ids = {} - prev_state_ids = {} - - # We don't store state for outliers, so we don't generate a state - # group for it. - context = EventContext.with_state( - state_group=None, - state_group_before_event=None, - current_state_ids=current_state_ids, - prev_state_ids=prev_state_ids, - ) - - return context + assert not event.internal_metadata.is_outlier() # # first of all, figure out the state before the event |