Ensure we use a copy of the event content dict before modifying it in serialize_event (#9585)

This bug was discovered by DINUM. We were modifying `serialized_event["content"]`, which - if you've got `USE_FROZEN_DICTS` turned on or are [using a third party rules module](https://github.com/matrix-org/synapse/blob/17cd48fe5171d50da4cb59db647b993168e7dfab/synapse/events/third_party_rules.py#L73-L76) - will raise a 500 if you try to a edit a reply to a message.

`serialized_event["content"]` could be set to the edit event's content, instead of a copy of it, which is bad as we attempt to modify it. Instead, we also end up modifying the original event's content. DINUM uses a third party rules module, which meant the event's content got frozen and thus an exception was raised.

To be clear, the problem is not that the event's content was frozen. In fact doing so helped us uncover the fact we weren't copying event content correctly.

2 files changed, 124 insertions, 0 deletions

diff --git a/tests/rest/client/test_third_party_rules.py b/tests/rest/client/test_third_party_rules.py
index 227fffab58..bf39014277 100644
--- a/tests/rest/client/test_third_party_rules.py
+++ b/tests/rest/client/test_third_party_rules.py
@@ -161,6 +161,68 @@ class ThirdPartyRulesTestCase(unittest.HomeserverTestCase):
         ev = channel.json_body
         self.assertEqual(ev["content"]["x"], "y")
 
+    def test_message_edit(self):
+        """Ensure that the module doesn't cause issues with edited messages."""
+        # first patch the event checker so that it will modify the event
+        async def check(ev: EventBase, state):
+            d = ev.get_dict()
+            d["content"] = {
+                "msgtype": "m.text",
+                "body": d["content"]["body"].upper(),
+            }
+            return d
+
+        current_rules_module().check_event_allowed = check
+
+        # Send an event, then edit it.
+        channel = self.make_request(
+            "PUT",
+            "/_matrix/client/r0/rooms/%s/send/modifyme/1" % self.room_id,
+            {
+                "msgtype": "m.text",
+                "body": "Original body",
+            },
+            access_token=self.tok,
+        )
+        self.assertEqual(channel.result["code"], b"200", channel.result)
+        orig_event_id = channel.json_body["event_id"]
+
+        channel = self.make_request(
+            "PUT",
+            "/_matrix/client/r0/rooms/%s/send/m.room.message/2" % self.room_id,
+            {
+                "m.new_content": {"msgtype": "m.text", "body": "Edited body"},
+                "m.relates_to": {
+                    "rel_type": "m.replace",
+                    "event_id": orig_event_id,
+                },
+                "msgtype": "m.text",
+                "body": "Edited body",
+            },
+            access_token=self.tok,
+        )
+        self.assertEqual(channel.result["code"], b"200", channel.result)
+        edited_event_id = channel.json_body["event_id"]
+
+        # ... and check that they both got modified
+        channel = self.make_request(
+            "GET",
+            "/_matrix/client/r0/rooms/%s/event/%s" % (self.room_id, orig_event_id),
+            access_token=self.tok,
+        )
+        self.assertEqual(channel.result["code"], b"200", channel.result)
+        ev = channel.json_body
+        self.assertEqual(ev["content"]["body"], "ORIGINAL BODY")
+
+        channel = self.make_request(
+            "GET",
+            "/_matrix/client/r0/rooms/%s/event/%s" % (self.room_id, edited_event_id),
+            access_token=self.tok,
+        )
+        self.assertEqual(channel.result["code"], b"200", channel.result)
+        ev = channel.json_body
+        self.assertEqual(ev["content"]["body"], "EDITED BODY")
+
     def test_send_event(self):
         """Tests that the module can send an event into a room via the module api"""
         content = {
diff --git a/tests/rest/client/v2_alpha/test_relations.py b/tests/rest/client/v2_alpha/test_relations.py
index 7c457754f1..e7bb5583fc 100644
--- a/tests/rest/client/v2_alpha/test_relations.py
+++ b/tests/rest/client/v2_alpha/test_relations.py
@@ -39,6 +39,11 @@ class RelationsTestCase(unittest.HomeserverTestCase):
         # We need to enable msc1849 support for aggregations
         config = self.default_config()
         config["experimental_msc1849_support_enabled"] = True
+
+        # We enable frozen dicts as relations/edits change event contents, so we
+        # want to test that we don't modify the events in the caches.
+        config["use_frozen_dicts"] = True
+
         return self.setup_test_homeserver(config=config)
 
     def prepare(self, reactor, clock, hs):
@@ -518,6 +523,63 @@ class RelationsTestCase(unittest.HomeserverTestCase):
             {"event_id": edit_event_id, "sender": self.user_id}, m_replace_dict
         )
 
+    def test_edit_reply(self):
+        """Test that editing a reply works."""
+
+        # Create a reply to edit.
+        channel = self._send_relation(
+            RelationTypes.REFERENCE,
+            "m.room.message",
+            content={"msgtype": "m.text", "body": "A reply!"},
+        )
+        self.assertEquals(200, channel.code, channel.json_body)
+        reply = channel.json_body["event_id"]
+
+        new_body = {"msgtype": "m.text", "body": "I've been edited!"}
+        channel = self._send_relation(
+            RelationTypes.REPLACE,
+            "m.room.message",
+            content={"msgtype": "m.text", "body": "foo", "m.new_content": new_body},
+            parent_id=reply,
+        )
+        self.assertEquals(200, channel.code, channel.json_body)
+
+        edit_event_id = channel.json_body["event_id"]
+
+        channel = self.make_request(
+            "GET",
+            "/rooms/%s/event/%s" % (self.room, reply),
+            access_token=self.user_token,
+        )
+        self.assertEquals(200, channel.code, channel.json_body)
+
+        # We expect to see the new body in the dict, as well as the reference
+        # metadata sill intact.
+        self.assertDictContainsSubset(new_body, channel.json_body["content"])
+        self.assertDictContainsSubset(
+            {
+                "m.relates_to": {
+                    "event_id": self.parent_id,
+                    "key": None,
+                    "rel_type": "m.reference",
+                }
+            },
+            channel.json_body["content"],
+        )
+
+        # We expect that the edit relation appears in the unsigned relations
+        # section.
+        relations_dict = channel.json_body["unsigned"].get("m.relations")
+        self.assertIn(RelationTypes.REPLACE, relations_dict)
+
+        m_replace_dict = relations_dict[RelationTypes.REPLACE]
+        for key in ["event_id", "sender", "origin_server_ts"]:
+            self.assertIn(key, m_replace_dict)
+
+        self.assert_dict(
+            {"event_id": edit_event_id, "sender": self.user_id}, m_replace_dict
+        )
+
     def test_relations_redaction_redacts_edits(self):
         """Test that edits of an event are redacted when the original event
         is redacted.