From ed8d0927af0f4e187d148ee352af02c501620d04 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 27 Jul 2023 10:27:16 -0400 Subject: [PATCH 1/5] Add forward compatibility for the redacts property. --- synapse/events/utils.py | 16 +++---- tests/rest/client/test_redactions.py | 67 +++++++++++++++++++++------- 2 files changed, 58 insertions(+), 25 deletions(-) diff --git a/synapse/events/utils.py b/synapse/events/utils.py index c890833b1d1e..1fb0b2c57ea3 100644 --- a/synapse/events/utils.py +++ b/synapse/events/utils.py @@ -475,14 +475,14 @@ def serialize_event( if config.as_client_event: d = config.event_format(d) - # If the event is a redaction, copy the redacts field from the content to - # top-level for backwards compatibility. - if ( - e.type == EventTypes.Redaction - and e.room_version.updated_redaction_rules - and e.redacts is not None - ): - d["redacts"] = e.redacts + # If the event is a redaction, either copy the redacts field from the content to + # top-level for backwards compatibility OR from the redacts field to the content + # field for forwards compatibility. + if e.type == EventTypes.Redaction and e.redacts is not None: + if e.room_version.updated_redaction_rules: + d["redacts"] = e.redacts + else: + d["content"]["redacts"] = e.redacts only_event_fields = config.only_event_fields if only_event_fields: diff --git a/tests/rest/client/test_redactions.py b/tests/rest/client/test_redactions.py index 6028886bd62e..180b635ea694 100644 --- a/tests/rest/client/test_redactions.py +++ b/tests/rest/client/test_redactions.py @@ -13,10 +13,12 @@ # limitations under the License. from typing import List, Optional +from parameterized import parameterized + from twisted.test.proto_helpers import MemoryReactor from synapse.api.constants import EventTypes, RelationTypes -from synapse.api.room_versions import RoomVersions +from synapse.api.room_versions import RoomVersion, RoomVersions from synapse.rest import admin from synapse.rest.client import login, room, sync from synapse.server import HomeServer @@ -569,50 +571,81 @@ def test_redact_relations_txn_id_reuse(self) -> None: self.assertIn("body", event_dict["content"], event_dict) self.assertEqual("I'm in a thread!", event_dict["content"]["body"]) - def test_content_redaction(self) -> None: - """MSC2174 moved the redacts property to the content.""" + @parameterized.expand( + [ + # Tuples of: + # Room version + # Boolean: True if the redaction event content should include the event ID. + # Boolean: true if the resulting redaction event is expected to include the + # event ID in the content. + (RoomVersions.V10, False, False), + (RoomVersions.V11, True, True), + (RoomVersions.V11, False, True), + ] + ) + def test_redaction_content( + self, room_version: RoomVersion, include_content: bool, expect_content: bool + ) -> None: + """ + Room version 11 moved the redacts property to the content. + + Ensure that the event gets created properly and that the Client-Server + API servers the proper backwards-compatible version. + """ # Create a room with the newer room version. room_id = self.helper.create_room_as( self.mod_user_id, tok=self.mod_access_token, - room_version=RoomVersions.V11.identifier, + room_version=room_version.identifier, ) # Create an event. b = self.helper.send(room_id=room_id, tok=self.mod_access_token) event_id = b["event_id"] - # Attempt to redact it with a bogus event ID. - self._redact_event( + # Ensure the event ID in the URL and the content must match. + if include_content: + self._redact_event( + self.mod_access_token, + room_id, + event_id, + expect_code=400, + content={"redacts": "foo"}, + ) + + # Redact it for real. + result = self._redact_event( self.mod_access_token, room_id, event_id, - expect_code=400, - content={"redacts": "foo"}, + content={"redacts": event_id} if include_content else {}, ) - - # Redact it for real. - self._redact_event(self.mod_access_token, room_id, event_id) + redaction_event_id = result["event_id"] # Sync the room, to get the id of the create event timeline = self._sync_room_timeline(self.mod_access_token, room_id) redact_event = timeline[-1] self.assertEqual(redact_event["type"], EventTypes.Redaction) - # The redacts key should be in the content. + # The redacts key should be in the content and the redacts keys. self.assertEquals(redact_event["content"]["redacts"], event_id) - - # It should also be copied as the top-level redacts field for backwards - # compatibility. self.assertEquals(redact_event["redacts"], event_id) # But it isn't actually part of the event. def get_event(txn: LoggingTransaction) -> JsonDict: return db_to_json( - main_datastore._fetch_event_rows(txn, [event_id])[event_id].json + main_datastore._fetch_event_rows(txn, [redaction_event_id])[ + redaction_event_id + ].json ) main_datastore = self.hs.get_datastores().main event_json = self.get_success( main_datastore.db_pool.runInteraction("get_event", get_event) ) - self.assertNotIn("redacts", event_json) + self.assertEquals(event_json["type"], EventTypes.Redaction) + if expect_content: + self.assertNotIn("redacts", event_json) + self.assertEquals(event_json["content"]["redacts"], event_id) + else: + self.assertEquals(event_json["redacts"], event_id) + self.assertNotIn("redacts", event_json["content"]) From a24050d0e3654e221b42b3b340a7610542735cf1 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 27 Jul 2023 10:31:03 -0400 Subject: [PATCH 2/5] Newsfragment --- changelog.d/16013.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/16013.bugfix diff --git a/changelog.d/16013.bugfix b/changelog.d/16013.bugfix new file mode 100644 index 000000000000..5c18a86ea3dc --- /dev/null +++ b/changelog.d/16013.bugfix @@ -0,0 +1 @@ +Fix a bug introduced in Synapse 1.89.0 where the `redacts` property was not properly added to the `content` for forwards-compatibility with room versions 1 through 10. From aa0ff91fca25a47d1ca331fa7001369768efa619 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Thu, 27 Jul 2023 12:50:09 -0400 Subject: [PATCH 3/5] Fix-up frozen events. --- synapse/events/utils.py | 1 + 1 file changed, 1 insertion(+) diff --git a/synapse/events/utils.py b/synapse/events/utils.py index 1fb0b2c57ea3..d88f87da24a3 100644 --- a/synapse/events/utils.py +++ b/synapse/events/utils.py @@ -482,6 +482,7 @@ def serialize_event( if e.room_version.updated_redaction_rules: d["redacts"] = e.redacts else: + d["content"] = dict(d["content"]) d["content"]["redacts"] = e.redacts only_event_fields = config.only_event_fields From 4bf4deb77ff9360113fb794ff3c06e710553d97c Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 1 Aug 2023 11:59:28 -0400 Subject: [PATCH 4/5] Update newsfragment. --- changelog.d/16013.bugfix | 1 - changelog.d/16013.misc | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) delete mode 100644 changelog.d/16013.bugfix create mode 100644 changelog.d/16013.misc diff --git a/changelog.d/16013.bugfix b/changelog.d/16013.bugfix deleted file mode 100644 index 5c18a86ea3dc..000000000000 --- a/changelog.d/16013.bugfix +++ /dev/null @@ -1 +0,0 @@ -Fix a bug introduced in Synapse 1.89.0 where the `redacts` property was not properly added to the `content` for forwards-compatibility with room versions 1 through 10. diff --git a/changelog.d/16013.misc b/changelog.d/16013.misc new file mode 100644 index 000000000000..bd161e13edd0 --- /dev/null +++ b/changelog.d/16013.misc @@ -0,0 +1 @@ +Properly overwrite the `redacts` content-property for forwards-compatibility with room versions 1 through 10. From 35a243c8248ae8031c5b756d687dc6dd4a36d658 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 2 Aug 2023 10:40:38 -0400 Subject: [PATCH 5/5] Clarify comment. --- synapse/events/utils.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/synapse/events/utils.py b/synapse/events/utils.py index d88f87da24a3..967a6c245b6c 100644 --- a/synapse/events/utils.py +++ b/synapse/events/utils.py @@ -475,9 +475,10 @@ def serialize_event( if config.as_client_event: d = config.event_format(d) - # If the event is a redaction, either copy the redacts field from the content to - # top-level for backwards compatibility OR from the redacts field to the content - # field for forwards compatibility. + # If the event is a redaction, the field with the redacted event ID appears + # in a different location depending on the room version. e.redacts handles + # fetching from the proper location; copy it to the other location for forwards- + # and backwards-compatibility with clients. if e.type == EventTypes.Redaction and e.redacts is not None: if e.room_version.updated_redaction_rules: d["redacts"] = e.redacts