diff --git a/changelog.d/17142.bugfix b/changelog.d/17142.bugfix new file mode 100644 index 00000000000..09b617aed11 --- /dev/null +++ b/changelog.d/17142.bugfix @@ -0,0 +1 @@ +Fix bug where push rules would be empty in `/sync` for some accounts. Introduced in v1.93.0. diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index 8ff45a3353b..0332fe66994 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -1945,23 +1945,19 @@ async def _generate_sync_entry_for_account_data( ) if push_rules_changed: - global_account_data = { - AccountDataTypes.PUSH_RULES: await self._push_rules_handler.push_rules_for_user( - sync_config.user - ), - **global_account_data, - } + global_account_data = dict(global_account_data) + global_account_data[AccountDataTypes.PUSH_RULES] = ( + await self._push_rules_handler.push_rules_for_user(sync_config.user) + ) else: all_global_account_data = await self.store.get_global_account_data_for_user( user_id ) - global_account_data = { - AccountDataTypes.PUSH_RULES: await self._push_rules_handler.push_rules_for_user( - sync_config.user - ), - **all_global_account_data, - } + global_account_data = dict(all_global_account_data) + global_account_data[AccountDataTypes.PUSH_RULES] = ( + await self._push_rules_handler.push_rules_for_user(sync_config.user) + ) account_data_for_user = ( await sync_config.filter_collection.filter_global_account_data( diff --git a/tests/handlers/test_sync.py b/tests/handlers/test_sync.py index 2780d29cada..6fce14c1edd 100644 --- a/tests/handlers/test_sync.py +++ b/tests/handlers/test_sync.py @@ -24,7 +24,7 @@ from twisted.test.proto_helpers import MemoryReactor -from synapse.api.constants import EventTypes, JoinRules +from synapse.api.constants import AccountDataTypes, EventTypes, JoinRules from synapse.api.errors import Codes, ResourceLimitError from synapse.api.filtering import FilterCollection, Filtering from synapse.api.room_versions import RoomVersion, RoomVersions @@ -846,6 +846,33 @@ async def _check_sigs_and_hash_for_pulled_events_and_fetch( self.assertIn(private_call_event.event_id, priv_event_ids) + def test_push_rules_with_bad_account_data(self) -> None: + """Some old accounts have managed to set a `m.push_rules` account data, + which we should ignore in /sync response. + """ + + user = self.register_user("alice", "password") + + # Insert the bad account data. + self.get_success( + self.store.add_account_data_for_user(user, AccountDataTypes.PUSH_RULES, {}) + ) + + sync_result: SyncResult = self.get_success( + self.sync_handler.wait_for_sync_for_user( + create_requester(user), generate_sync_config(user) + ) + ) + + for account_dict in sync_result.account_data: + if account_dict["type"] == AccountDataTypes.PUSH_RULES: + # We should have lots of push rules here, rather than the bad + # empty data. + self.assertNotEqual(account_dict["content"], {}) + return + + self.fail("No push rules found") + _request_key = 0