From e21a6d2103f1df744767ad57efa464c3b52d886a Mon Sep 17 00:00:00 2001 From: David Robertson Date: Mon, 28 Feb 2022 12:10:32 +0000 Subject: [PATCH 1/6] Actually fix bad logging rejecting incoming transactions --- synapse/federation/transport/server/federation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/federation/transport/server/federation.py b/synapse/federation/transport/server/federation.py index 9cc9a7339d19..23ce34305705 100644 --- a/synapse/federation/transport/server/federation.py +++ b/synapse/federation/transport/server/federation.py @@ -110,7 +110,7 @@ async def on_PUT( if issue_8631_logger.isEnabledFor(logging.DEBUG): DEVICE_UPDATE_EDUS = ["m.device_list_update", "m.signing_key_update"] device_list_updates = [ - edu.content + edu.get("content", {}) for edu in transaction_data.get("edus", []) if edu.get("edu_type") in DEVICE_UPDATE_EDUS ] From 2e36f230606d09a1d84f28236ae62908e4a9a4bf Mon Sep 17 00:00:00 2001 From: David Robertson Date: Mon, 28 Feb 2022 12:19:20 +0000 Subject: [PATCH 2/6] Changelog --- changelog.d/12098.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/12098.bugfix diff --git a/changelog.d/12098.bugfix b/changelog.d/12098.bugfix new file mode 100644 index 000000000000..10e603c7ed8e --- /dev/null +++ b/changelog.d/12098.bugfix @@ -0,0 +1 @@ +Fix a bug introduced in Synapse 1.51.0rc1 where incoming federation transactions containing at least one EDU would be dropped if debug logging was enabled for `synapse.8631_debug`. Synapse 1.53.0rc1 included a partial fix this ([#11890](https://github.com/matrix-org/synapse/pull/11890)), but the fix was incorrect: EDUs containing device list or signing key updates would still be dropped. \ No newline at end of file From 39022bde0bd051fad5ec158f30a0a63db7691b41 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Mon, 28 Feb 2022 12:32:17 +0000 Subject: [PATCH 3/6] Test case --- tests/federation/transport/test_server.py | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/tests/federation/transport/test_server.py b/tests/federation/transport/test_server.py index eb62addda8c6..6bc63e6a6b68 100644 --- a/tests/federation/transport/test_server.py +++ b/tests/federation/transport/test_server.py @@ -13,7 +13,7 @@ # limitations under the License. from tests import unittest -from tests.unittest import override_config +from tests.unittest import override_config, DEBUG class RoomDirectoryFederationTests(unittest.FederatingHomeserverTestCase): @@ -38,3 +38,19 @@ def test_open_public_room_list_over_federation(self): "/_matrix/federation/v1/publicRooms", ) self.assertEquals(200, channel.code) + + @DEBUG + def test_edu_debugging_doesnt_explode(self): + """Sanity check fed. RX succeeds with `synapse.debug_8631` logging enabled. + + Remove this when we strip out issue_8631_logger. + """ + channel = self.make_signed_federation_request( + "PUT", + "/_matrix/federation/v1/send/txn_id_1234/", + content={ + "edus": [{"edu_type": "m.device_list_update", "content": {"foo": "bar"}}], + "pdus": [], + }, + ) + self.assertEquals(200, channel.code) From 4f4127bdf3cf33ea72c0d5a22929ef08e149746d Mon Sep 17 00:00:00 2001 From: David Robertson Date: Mon, 28 Feb 2022 12:45:42 +0000 Subject: [PATCH 4/6] Don't use RX --- tests/federation/transport/test_server.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/federation/transport/test_server.py b/tests/federation/transport/test_server.py index 36e93c069fc2..b1fa2e04c91a 100644 --- a/tests/federation/transport/test_server.py +++ b/tests/federation/transport/test_server.py @@ -41,7 +41,7 @@ def test_open_public_room_list_over_federation(self): @DEBUG def test_edu_debugging_doesnt_explode(self): - """Sanity check fed. RX succeeds with `synapse.debug_8631` logging enabled. + """Sanity check incoming federation succeeds with `synapse.debug_8631` enabled. Remove this when we strip out issue_8631_logger. """ From 1cfd4e55c291bf9130acdd856060eaa1e821e55c Mon Sep 17 00:00:00 2001 From: David Robertson Date: Mon, 28 Feb 2022 12:46:42 +0000 Subject: [PATCH 5/6] Simpler changelog --- changelog.d/12098.bugfix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/12098.bugfix b/changelog.d/12098.bugfix index 10e603c7ed8e..6b696692e332 100644 --- a/changelog.d/12098.bugfix +++ b/changelog.d/12098.bugfix @@ -1 +1 @@ -Fix a bug introduced in Synapse 1.51.0rc1 where incoming federation transactions containing at least one EDU would be dropped if debug logging was enabled for `synapse.8631_debug`. Synapse 1.53.0rc1 included a partial fix this ([#11890](https://github.com/matrix-org/synapse/pull/11890)), but the fix was incorrect: EDUs containing device list or signing key updates would still be dropped. \ No newline at end of file +Fix a bug introduced in Synapse 1.51.0rc1 where incoming federation transactions containing at least one EDU would be dropped if debug logging was enabled for `synapse.8631_debug`. \ No newline at end of file From d950e3bd9acdc03723990e41203fdf18559871cb Mon Sep 17 00:00:00 2001 From: David Robertson Date: Mon, 28 Feb 2022 13:32:37 +0000 Subject: [PATCH 6/6] Linter script --- tests/federation/transport/test_server.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/federation/transport/test_server.py b/tests/federation/transport/test_server.py index b1fa2e04c91a..5f001c33b05e 100644 --- a/tests/federation/transport/test_server.py +++ b/tests/federation/transport/test_server.py @@ -13,7 +13,7 @@ # limitations under the License. from tests import unittest -from tests.unittest import override_config, DEBUG +from tests.unittest import DEBUG, override_config class RoomDirectoryFederationTests(unittest.FederatingHomeserverTestCase): @@ -49,7 +49,9 @@ def test_edu_debugging_doesnt_explode(self): "PUT", "/_matrix/federation/v1/send/txn_id_1234/", content={ - "edus": [{"edu_type": "m.device_list_update", "content": {"foo": "bar"}}], + "edus": [ + {"edu_type": "m.device_list_update", "content": {"foo": "bar"}} + ], "pdus": [], }, )