From 3e0f759dbc34cb3be0a1946cd36e617fc3c5a17c Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Mon, 17 Jun 2024 18:26:59 -0500 Subject: [PATCH] Strip invite/knock event itself and avoid mutating event `unsigned` Make sure we don't run into https://github.com/element-hq/synapse/issues/14919 (https://github.com/matrix-org/synapse/issues/14919) --- synapse/events/utils.py | 18 ++++++++++++++++++ synapse/handlers/sliding_sync.py | 14 ++++++++------ synapse/rest/client/sync.py | 10 +--------- .../storage/databases/main/events_worker.py | 12 ++---------- synapse/types/handlers/__init__.py | 4 ++-- 5 files changed, 31 insertions(+), 27 deletions(-) diff --git a/synapse/events/utils.py b/synapse/events/utils.py index b997d82d71..f937fd4698 100644 --- a/synapse/events/utils.py +++ b/synapse/events/utils.py @@ -836,3 +836,21 @@ def maybe_upsert_event_field( del container[key] return upsert_okay + + +def strip_event(event: EventBase) -> JsonDict: + """ + Used for "stripped state" events which provide a simplified view of the state of a + room intended to help a potential joiner identify the room (relevant when the user + is invited or knocked). + + Stripped state events can only have the `sender`, `type`, `state_key` and `content` + properties present. + """ + + return { + "type": event.type, + "state_key": event.state_key, + "content": event.content, + "sender": event.sender, + } diff --git a/synapse/handlers/sliding_sync.py b/synapse/handlers/sliding_sync.py index cf448fa3cd..23f971c1f7 100644 --- a/synapse/handlers/sliding_sync.py +++ b/synapse/handlers/sliding_sync.py @@ -25,8 +25,10 @@ from immutabledict import immutabledict from synapse.api.constants import AccountDataTypes, Direction, EventTypes, Membership from synapse.events import EventBase +from synapse.events.utils import strip_event from synapse.storage.roommember import RoomsForUser from synapse.types import ( + JsonDict, PersistedEventPosition, Requester, RoomStreamToken, @@ -793,7 +795,7 @@ class SlidingSyncHandler: ) # Figure out any stripped state events for invite/knocks - stripped_state: List[EventBase] = [] + stripped_state: List[JsonDict] = [] if rooms_for_user_membership_at_to_token.membership in { Membership.INVITE, Membership.KNOCK, @@ -804,15 +806,15 @@ class SlidingSyncHandler: stripped_state = [] if invite_or_knock_event.membership == Membership.INVITE: - stripped_state = invite_or_knock_event.unsigned.get( - "invite_room_state", [] + stripped_state.extend( + invite_or_knock_event.unsigned.get("invite_room_state", []) ) elif invite_or_knock_event.membership == Membership.KNOCK: - stripped_state = invite_or_knock_event.unsigned.get( - "knock_room_state", [] + stripped_state.extend( + invite_or_knock_event.unsigned.get("knock_room_state", []) ) - stripped_state.append(invite_or_knock_event) + stripped_state.append(strip_event(invite_or_knock_event)) return SlidingSyncResult.RoomResult( # TODO: Dummy value diff --git a/synapse/rest/client/sync.py b/synapse/rest/client/sync.py index b261b2dd88..a9be37bbf3 100644 --- a/synapse/rest/client/sync.py +++ b/synapse/rest/client/sync.py @@ -1017,18 +1017,10 @@ class SlidingSyncRestServlet(RestServlet): # Stripped state only applies to invite/knock rooms if room_result.stripped_state: - serialized_stripped_state = ( - await self.event_serializer.serialize_events( - room_result.stripped_state, - time_now, - config=serialize_options, - ) - ) - # TODO: Would be good to rename this to `stripped_state` so it can be # shared between invite and knock rooms, see # https://github.com/matrix-org/matrix-spec-proposals/pull/3575#discussion_r1117629919 - serialized_rooms[room_id]["invite_state"] = serialized_stripped_state + serialized_rooms[room_id]["invite_state"] = room_result.stripped_state return serialized_rooms diff --git a/synapse/storage/databases/main/events_worker.py b/synapse/storage/databases/main/events_worker.py index e264d36f02..f0f390cec4 100644 --- a/synapse/storage/databases/main/events_worker.py +++ b/synapse/storage/databases/main/events_worker.py @@ -55,7 +55,7 @@ from synapse.api.room_versions import ( ) from synapse.events import EventBase, make_event_from_dict from synapse.events.snapshot import EventContext -from synapse.events.utils import prune_event +from synapse.events.utils import prune_event, strip_event from synapse.logging.context import ( PreserveLoggingContext, current_context, @@ -1025,15 +1025,7 @@ class EventsWorkerStore(SQLBaseStore): state_to_include = await self.get_events(selected_state_ids.values()) - return [ - { - "type": e.type, - "state_key": e.state_key, - "content": e.content, - "sender": e.sender, - } - for e in state_to_include.values() - ] + return [strip_event(e) for e in state_to_include.values()] def _maybe_start_fetch_thread(self) -> None: """Starts an event fetch thread if we are not yet at the maximum number.""" diff --git a/synapse/types/handlers/__init__.py b/synapse/types/handlers/__init__.py index b544398a35..04b0ab972b 100644 --- a/synapse/types/handlers/__init__.py +++ b/synapse/types/handlers/__init__.py @@ -31,7 +31,7 @@ else: from pydantic import Extra from synapse.events import EventBase -from synapse.types import JsonMapping, StreamToken, UserID +from synapse.types import JsonDict, JsonMapping, StreamToken, UserID from synapse.types.rest.client import SlidingSyncBody @@ -193,7 +193,7 @@ class SlidingSyncResult: required_state: List[EventBase] timeline: List[EventBase] is_dm: bool - stripped_state: Optional[List[EventBase]] + stripped_state: Optional[List[JsonDict]] prev_batch: StreamToken limited: bool joined_count: int