Merge remote-tracking branch 'origin/develop' into rav/cachedlist_error_handling

Author: richvdh

changelog.d/11865.removal

@@ -0,0 +1 @@
+Deprecate using `synctl` with the config option `synctl_cache_factor` and print a warning if a user still uses this option.

changelog.d/12077.bugfix

@@ -0,0 +1 @@
+Fix a long-standing bug where Synapse would make additional failing requests over federation for missing data.

changelog.d/12092.misc

@@ -0,0 +1 @@
+User `assertEqual` instead of the deprecated `assertEquals` in test code.

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`.

changelog.d/12100.bugfix

@@ -0,0 +1 @@
+Fix a long-standing bug which could cause push notifications to malfunction if `use_frozen_dicts` was set in the configuration.

synapse/api/filtering.py

@@ -22,6 +22,7 @@
     Dict,
     Iterable,
     List,
+    Mapping,
     Optional,
     Set,
     TypeVar,
@@ -361,10 +362,10 @@ def _check(self, event: FilterEvent) -> bool:
             return self._check_fields(field_matchers)
         else:
             content = event.get("content")
-            # Content is assumed to be a dict below, so ensure it is. This should
+            # Content is assumed to be a mapping below, so ensure it is. This should
             # always be true for events, but account_data has been allowed to
             # have non-dict content.
-            if not isinstance(content, dict):
+            if not isinstance(content, Mapping):
                 content = {}
 
             sender = event.get("sender", None)

synapse/federation/federation_client.py

@@ -615,11 +615,15 @@ def _is_unknown_endpoint(
             synapse_error = e.to_synapse_error()
         # There is no good way to detect an "unknown" endpoint.
         #
-        # Dendrite returns a 404 (with no body); synapse returns a 400
+        # Dendrite returns a 404 (with a body of "404 page not found");
+        # Conduit returns a 404 (with no body); and Synapse returns a 400
         # with M_UNRECOGNISED.
-        return e.code == 404 or (
-            e.code == 400 and synapse_error.errcode == Codes.UNRECOGNIZED
-        )
+        #
+        # This needs to be rather specific as some endpoints truly do return 404
+        # errors.
+        return (
+            e.code == 404 and (not e.response or e.response == b"404 page not found")
+        ) or (e.code == 400 and synapse_error.errcode == Codes.UNRECOGNIZED)
 
     async def _try_destination_list(
         self,
@@ -1002,7 +1006,7 @@ async def _do_send_join(
             )
         except HttpResponseException as e:
             # If an error is received that is due to an unrecognised endpoint,
-            # fallback to the v1 endpoint. Otherwise consider it a legitmate error
+            # fallback to the v1 endpoint. Otherwise, consider it a legitimate error
             # and raise.
             if not self._is_unknown_endpoint(e):
                 raise
@@ -1071,7 +1075,7 @@ async def _do_send_invite(
         except HttpResponseException as e:
             # If an error is received that is due to an unrecognised endpoint,
             # fallback to the v1 endpoint if the room uses old-style event IDs.
-            # Otherwise consider it a legitmate error and raise.
+            # Otherwise, consider it a legitimate error and raise.
             err = e.to_synapse_error()
             if self._is_unknown_endpoint(e, err):
                 if room_version.event_format != EventFormatVersions.V1:
@@ -1132,7 +1136,7 @@ async def _do_send_leave(self, destination: str, pdu: EventBase) -> JsonDict:
             )
         except HttpResponseException as e:
             # If an error is received that is due to an unrecognised endpoint,
-            # fallback to the v1 endpoint. Otherwise consider it a legitmate error
+            # fallback to the v1 endpoint. Otherwise, consider it a legitimate error
             # and raise.
             if not self._is_unknown_endpoint(e):
                 raise
@@ -1458,8 +1462,8 @@ async def send_request(
                 )
             except HttpResponseException as e:
                 # If an error is received that is due to an unrecognised endpoint,
-                # fallback to the unstable endpoint. Otherwise consider it a
-                # legitmate error and raise.
+                # fallback to the unstable endpoint. Otherwise, consider it a
+                # legitimate error and raise.
                 if not self._is_unknown_endpoint(e):
                     raise
 

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
                 ]

synapse/push/push_rule_evaluator.py

@@ -15,12 +15,12 @@
 
 import logging
 import re
-from typing import Any, Dict, List, Optional, Pattern, Tuple, Union
+from typing import Any, Dict, List, Mapping, Optional, Pattern, Tuple, Union
 
 from matrix_common.regex import glob_to_regex, to_word_pattern
 
 from synapse.events import EventBase
-from synapse.types import JsonDict, UserID
+from synapse.types import UserID
 from synapse.util.caches.lrucache import LruCache
 
 logger = logging.getLogger(__name__)
@@ -223,7 +223,7 @@ def _glob_matches(glob: str, value: str, word_boundary: bool = False) -> bool:
 
 
 def _flatten_dict(
-    d: Union[EventBase, JsonDict],
+    d: Union[EventBase, Mapping[str, Any]],
     prefix: Optional[List[str]] = None,
     result: Optional[Dict[str, str]] = None,
 ) -> Dict[str, str]:
@@ -234,7 +234,7 @@ def _flatten_dict(
     for key, value in d.items():
         if isinstance(value, str):
             result[".".join(prefix + [key])] = value.lower()
-        elif isinstance(value, dict):
+        elif isinstance(value, Mapping):
             _flatten_dict(value, prefix=(prefix + [key]), result=result)
 
     return result

synctl

@@ -37,6 +37,13 @@ YELLOW = "\x1b[1;33m"
 RED = "\x1b[1;31m"
 NORMAL = "\x1b[m"
 
+SYNCTL_CACHE_FACTOR_WARNING = """\
+Setting 'synctl_cache_factor' in the config is deprecated. Instead, please do
+one of the following:
+ - Either set the environment variable 'SYNAPSE_CACHE_FACTOR'
+ - or set 'caches.global_factor' in the homeserver config.
+--------------------------------------------------------------------------------"""
+
 
 def pid_running(pid):
     try:
@@ -228,6 +235,7 @@ def main():
     start_stop_synapse = True
 
     if cache_factor:
+        write(SYNCTL_CACHE_FACTOR_WARNING)
         os.environ["SYNAPSE_CACHE_FACTOR"] = str(cache_factor)
 
     cache_factors = config.get("synctl_cache_factors", {})

tests/api/test_auth.py

@@ -71,7 +71,7 @@ def test_get_user_by_req_user_valid_token(self):
         request.args[b"access_token"] = [self.test_token]
         request.requestHeaders.getRawHeaders = mock_getRawHeaders()
         requester = self.get_success(self.auth.get_user_by_req(request))
-        self.assertEquals(requester.user.to_string(), self.test_user)
+        self.assertEqual(requester.user.to_string(), self.test_user)
 
     def test_get_user_by_req_user_bad_token(self):
         self.store.get_user_by_access_token = simple_async_mock(None)
@@ -109,7 +109,7 @@ def test_get_user_by_req_appservice_valid_token(self):
         request.args[b"access_token"] = [self.test_token]
         request.requestHeaders.getRawHeaders = mock_getRawHeaders()
         requester = self.get_success(self.auth.get_user_by_req(request))
-        self.assertEquals(requester.user.to_string(), self.test_user)
+        self.assertEqual(requester.user.to_string(), self.test_user)
 
     def test_get_user_by_req_appservice_valid_token_good_ip(self):
         from netaddr import IPSet
@@ -128,7 +128,7 @@ def test_get_user_by_req_appservice_valid_token_good_ip(self):
         request.args[b"access_token"] = [self.test_token]
         request.requestHeaders.getRawHeaders = mock_getRawHeaders()
         requester = self.get_success(self.auth.get_user_by_req(request))
-        self.assertEquals(requester.user.to_string(), self.test_user)
+        self.assertEqual(requester.user.to_string(), self.test_user)
 
     def test_get_user_by_req_appservice_valid_token_bad_ip(self):
         from netaddr import IPSet
@@ -195,7 +195,7 @@ def test_get_user_by_req_appservice_valid_token_valid_user_id(self):
         request.args[b"user_id"] = [masquerading_user_id]
         request.requestHeaders.getRawHeaders = mock_getRawHeaders()
         requester = self.get_success(self.auth.get_user_by_req(request))
-        self.assertEquals(
+        self.assertEqual(
             requester.user.to_string(), masquerading_user_id.decode("utf8")
         )
 
@@ -242,10 +242,10 @@ def test_get_user_by_req_appservice_valid_token_valid_device_id(self):
         request.args[b"org.matrix.msc3202.device_id"] = [masquerading_device_id]
         request.requestHeaders.getRawHeaders = mock_getRawHeaders()
         requester = self.get_success(self.auth.get_user_by_req(request))
-        self.assertEquals(
+        self.assertEqual(
             requester.user.to_string(), masquerading_user_id.decode("utf8")
         )
-        self.assertEquals(requester.device_id, masquerading_device_id.decode("utf8"))
+        self.assertEqual(requester.device_id, masquerading_device_id.decode("utf8"))
 
     @override_config({"experimental_features": {"msc3202_device_masquerading": True}})
     def test_get_user_by_req_appservice_valid_token_invalid_device_id(self):
@@ -275,8 +275,8 @@ def test_get_user_by_req_appservice_valid_token_invalid_device_id(self):
         request.requestHeaders.getRawHeaders = mock_getRawHeaders()
 
         failure = self.get_failure(self.auth.get_user_by_req(request), AuthError)
-        self.assertEquals(failure.value.code, 400)
-        self.assertEquals(failure.value.errcode, Codes.EXCLUSIVE)
+        self.assertEqual(failure.value.code, 400)
+        self.assertEqual(failure.value.errcode, Codes.EXCLUSIVE)
 
     def test_get_user_by_req__puppeted_token__not_tracking_puppeted_mau(self):
         self.store.get_user_by_access_token = simple_async_mock(
@@ -309,7 +309,7 @@ def test_get_user_by_req__puppeted_token__tracking_puppeted_mau(self):
         request.args[b"access_token"] = [self.test_token]
         request.requestHeaders.getRawHeaders = mock_getRawHeaders()
         self.get_success(self.auth.get_user_by_req(request))
-        self.assertEquals(self.store.insert_client_ip.call_count, 2)
+        self.assertEqual(self.store.insert_client_ip.call_count, 2)
 
     def test_get_user_from_macaroon(self):
         self.store.get_user_by_access_token = simple_async_mock(
@@ -369,9 +369,9 @@ def test_blocking_mau(self):
         self.store.get_monthly_active_count = simple_async_mock(lots_of_users)
 
         e = self.get_failure(self.auth.check_auth_blocking(), ResourceLimitError)
-        self.assertEquals(e.value.admin_contact, self.hs.config.server.admin_contact)
-        self.assertEquals(e.value.errcode, Codes.RESOURCE_LIMIT_EXCEEDED)
-        self.assertEquals(e.value.code, 403)
+        self.assertEqual(e.value.admin_contact, self.hs.config.server.admin_contact)
+        self.assertEqual(e.value.errcode, Codes.RESOURCE_LIMIT_EXCEEDED)
+        self.assertEqual(e.value.code, 403)
 
         # Ensure does not throw an error
         self.store.get_monthly_active_count = simple_async_mock(small_number_of_users)
@@ -473,9 +473,9 @@ def test_hs_disabled(self):
         self.auth_blocking._hs_disabled = True
         self.auth_blocking._hs_disabled_message = "Reason for being disabled"
         e = self.get_failure(self.auth.check_auth_blocking(), ResourceLimitError)
-        self.assertEquals(e.value.admin_contact, self.hs.config.server.admin_contact)
-        self.assertEquals(e.value.errcode, Codes.RESOURCE_LIMIT_EXCEEDED)
-        self.assertEquals(e.value.code, 403)
+        self.assertEqual(e.value.admin_contact, self.hs.config.server.admin_contact)
+        self.assertEqual(e.value.errcode, Codes.RESOURCE_LIMIT_EXCEEDED)
+        self.assertEqual(e.value.code, 403)
 
     def test_hs_disabled_no_server_notices_user(self):
         """Check that 'hs_disabled_message' works correctly when there is no
@@ -488,9 +488,9 @@ def test_hs_disabled_no_server_notices_user(self):
         self.auth_blocking._hs_disabled = True
         self.auth_blocking._hs_disabled_message = "Reason for being disabled"
         e = self.get_failure(self.auth.check_auth_blocking(), ResourceLimitError)
-        self.assertEquals(e.value.admin_contact, self.hs.config.server.admin_contact)
-        self.assertEquals(e.value.errcode, Codes.RESOURCE_LIMIT_EXCEEDED)
-        self.assertEquals(e.value.code, 403)
+        self.assertEqual(e.value.admin_contact, self.hs.config.server.admin_contact)
+        self.assertEqual(e.value.errcode, Codes.RESOURCE_LIMIT_EXCEEDED)
+        self.assertEqual(e.value.code, 403)
 
     def test_server_notices_mxid_special_cased(self):
         self.auth_blocking._hs_disabled = True

tests/api/test_filtering.py

@@ -18,6 +18,7 @@
 from unittest.mock import patch
 
 import jsonschema
+from frozendict import frozendict
 
 from synapse.api.constants import EventContentFields
 from synapse.api.errors import SynapseError
@@ -327,6 +328,15 @@ def test_filter_labels(self):
 
         self.assertFalse(Filter(self.hs, definition)._check(event))
 
+        # check it works with frozendicts too
+        event = MockEvent(
+            sender="@foo:bar",
+            type="m.room.message",
+            room_id="!secretbase:unknown",
+            content=frozendict({EventContentFields.LABELS: ["#fun"]}),
+        )
+        self.assertTrue(Filter(self.hs, definition)._check(event))
+
     def test_filter_not_labels(self):
         definition = {"org.matrix.not_labels": ["#fun"]}
         event = MockEvent(
@@ -364,7 +374,7 @@ def test_filter_presence_match(self):
         )
 
         results = self.get_success(user_filter.filter_presence(events=events))
-        self.assertEquals(events, results)
+        self.assertEqual(events, results)
 
     def test_filter_presence_no_match(self):
         user_filter_json = {"presence": {"types": ["m.*"]}}
@@ -388,7 +398,7 @@ def test_filter_presence_no_match(self):
         )
 
         results = self.get_success(user_filter.filter_presence(events=events))
-        self.assertEquals([], results)
+        self.assertEqual([], results)
 
     def test_filter_room_state_match(self):
         user_filter_json = {"room": {"state": {"types": ["m.*"]}}}
@@ -407,7 +417,7 @@ def test_filter_room_state_match(self):
         )
 
         results = self.get_success(user_filter.filter_room_state(events=events))
-        self.assertEquals(events, results)
+        self.assertEqual(events, results)
 
     def test_filter_room_state_no_match(self):
         user_filter_json = {"room": {"state": {"types": ["m.*"]}}}
@@ -428,7 +438,7 @@ def test_filter_room_state_no_match(self):
         )
 
         results = self.get_success(user_filter.filter_room_state(events))
-        self.assertEquals([], results)
+        self.assertEqual([], results)
 
     def test_filter_rooms(self):
         definition = {
@@ -444,7 +454,7 @@ def test_filter_rooms(self):
 
         filtered_room_ids = list(Filter(self.hs, definition).filter_rooms(room_ids))
 
-        self.assertEquals(filtered_room_ids, ["!allowed:example.com"])
+        self.assertEqual(filtered_room_ids, ["!allowed:example.com"])
 
     @unittest.override_config({"experimental_features": {"msc3440_enabled": True}})
     def test_filter_relations(self):
@@ -486,7 +496,7 @@ async def events_have_relations(*args, **kwargs):
                     Filter(self.hs, definition)._check_event_relations(events)
                 )
             )
-        self.assertEquals(filtered_events, events[1:])
+        self.assertEqual(filtered_events, events[1:])
 
     def test_add_filter(self):
         user_filter_json = {"room": {"state": {"types": ["m.*"]}}}
@@ -497,8 +507,8 @@ def test_add_filter(self):
             )
         )
 
-        self.assertEquals(filter_id, 0)
-        self.assertEquals(
+        self.assertEqual(filter_id, 0)
+        self.assertEqual(
             user_filter_json,
             (
                 self.get_success(
@@ -524,6 +534,6 @@ def test_get_filter(self):
             )
         )
 
-        self.assertEquals(filter.get_filter_json(), user_filter_json)
+        self.assertEqual(filter.get_filter_json(), user_filter_json)
 
-        self.assertRegexpMatches(repr(filter), r"<FilterCollection \{.*\}>")
+        self.assertRegex(repr(filter), r"<FilterCollection \{.*\}>")