1
0

Merge branch 'develop' into madlittlemods/11025-fix-user-directory-throwing-exception-when-interacting-with-appservice-sender

This commit is contained in:
Eric Eastwood
2021-10-07 19:22:06 -05:00
13 changed files with 81 additions and 23 deletions

1
changelog.d/10995.bugfix Normal file
View File

@@ -0,0 +1 @@
Correct a bugfix introduced in Synapse v1.44.0 that wouldn't catch every error of the connection breaks before a response could be written to it.

1
changelog.d/11002.bugfix Normal file
View File

@@ -0,0 +1 @@
Fix a long-standing bug where local users' per-room nicknames/avatars were visible to anyone who could see you in the user_directory.

1
changelog.d/11010.misc Normal file
View File

@@ -0,0 +1 @@
Clean up some of the federation event authentication code for clarity.

1
changelog.d/11011.misc Normal file
View File

@@ -0,0 +1 @@
Clean up some of the federation event authentication code for clarity.

1
changelog.d/11019.misc Normal file
View File

@@ -0,0 +1 @@
Ensure that cache config tests do not share state.

1
changelog.d/11021.misc Normal file
View File

@@ -0,0 +1 @@
Add additional type hints to `synapse.server_notices`.

View File

@@ -99,6 +99,9 @@ disallow_untyped_defs = True
[mypy-synapse.rest.*]
disallow_untyped_defs = True
[mypy-synapse.server_notices.*]
disallow_untyped_defs = True
[mypy-synapse.state.*]
disallow_untyped_defs = True

View File

@@ -894,6 +894,9 @@ class FederationEventHandler:
backfilled=backfilled,
)
except AuthError as e:
# FIXME richvdh 2021/10/07 I don't think this is reachable. Let's log it
# for now
logger.exception("Unexpected AuthError from _check_event_auth")
raise FederationError("ERROR", e.code, e.msg, affected=event.event_id)
await self._run_push_actions_and_persist_event(event, context, backfilled)
@@ -1158,7 +1161,10 @@ class FederationEventHandler:
return
logger.info(
"Persisting %i of %i remaining events", len(roots), len(event_map)
"Persisting %i of %i remaining outliers: %s",
len(roots),
len(event_map),
shortstr(e.event_id for e in roots),
)
await self._auth_and_persist_fetched_events_inner(origin, room_id, roots)

View File

@@ -203,6 +203,7 @@ class UserDirectoryHandler(StateDeltasHandler):
public_value=Membership.JOIN,
)
is_remote = not self.is_mine_id(state_key)
if change is MatchChange.now_false:
# Need to check if the server left the room entirely, if so
# we might need to remove all the users in that room
@@ -224,15 +225,20 @@ class UserDirectoryHandler(StateDeltasHandler):
else:
logger.debug("Server is still in room: %r", room_id)
include_in_dir = not self.is_mine_id(
state_key
) or await self.store.should_include_local_user_in_dir(state_key)
include_in_dir = (
is_remote
or await self.store.should_include_local_user_in_dir(state_key)
)
if include_in_dir:
if change is MatchChange.no_change:
# Handle any profile changes
await self._handle_profile_change(
state_key, room_id, prev_event_id, event_id
)
# Handle any profile changes for remote users.
# (For local users we are not forced to scan membership
# events; instead the rest of the application calls
# `handle_local_profile_change`.)
if is_remote:
await self._handle_profile_change(
state_key, room_id, prev_event_id, event_id
)
continue
if change is MatchChange.now_true: # The user joined

View File

@@ -563,7 +563,10 @@ class _ByteProducer:
try:
self._request.registerProducer(self, True)
except RuntimeError as e:
except AttributeError as e:
# Calling self._request.registerProducer might raise an AttributeError since
# the underlying Twisted code calls self._request.channel.registerProducer,
# however self._request.channel will be None if the connection was lost.
logger.info("Connection disconnected before response was written: %r", e)
# We drop our references to data we'll not use.

View File

@@ -41,12 +41,8 @@ class ServerNoticesManager:
self._notifier = hs.get_notifier()
self.server_notices_mxid = self._config.servernotices.server_notices_mxid
def is_enabled(self):
"""Checks if server notices are enabled on this server.
Returns:
bool
"""
def is_enabled(self) -> bool:
"""Checks if server notices are enabled on this server."""
return self.server_notices_mxid is not None
async def send_notice(

View File

@@ -12,12 +12,16 @@
# See the License for the specific language governing permissions and
# limitations under the License.
from unittest.mock import patch
from synapse.config.cache import CacheConfig, add_resizable_cache
from synapse.util.caches.lrucache import LruCache
from tests.unittest import TestCase
# Patch the global _CACHES so that each test runs against its own state.
@patch("synapse.config.cache._CACHES", new_callable=dict)
class CacheConfigTests(TestCase):
def setUp(self):
# Reset caches before each test
@@ -26,7 +30,7 @@ class CacheConfigTests(TestCase):
def tearDown(self):
self.config.reset()
def test_individual_caches_from_environ(self):
def test_individual_caches_from_environ(self, _caches):
"""
Individual cache factors will be loaded from the environment.
"""
@@ -39,7 +43,7 @@ class CacheConfigTests(TestCase):
self.assertEqual(dict(self.config.cache_factors), {"something_or_other": 2.0})
def test_config_overrides_environ(self):
def test_config_overrides_environ(self, _caches):
"""
Individual cache factors defined in the environment will take precedence
over those in the config.
@@ -56,7 +60,7 @@ class CacheConfigTests(TestCase):
{"foo": 1.0, "bar": 3.0, "something_or_other": 2.0},
)
def test_individual_instantiated_before_config_load(self):
def test_individual_instantiated_before_config_load(self, _caches):
"""
If a cache is instantiated before the config is read, it will be given
the default cache size in the interim, and then resized once the config
@@ -72,7 +76,7 @@ class CacheConfigTests(TestCase):
self.assertEqual(cache.max_size, 300)
def test_individual_instantiated_after_config_load(self):
def test_individual_instantiated_after_config_load(self, _caches):
"""
If a cache is instantiated after the config is read, it will be
immediately resized to the correct size given the per_cache_factor if
@@ -85,7 +89,7 @@ class CacheConfigTests(TestCase):
add_resizable_cache("foo", cache_resize_callback=cache.set_cache_factor)
self.assertEqual(cache.max_size, 200)
def test_global_instantiated_before_config_load(self):
def test_global_instantiated_before_config_load(self, _caches):
"""
If a cache is instantiated before the config is read, it will be given
the default cache size in the interim, and then resized to the new
@@ -100,7 +104,7 @@ class CacheConfigTests(TestCase):
self.assertEqual(cache.max_size, 400)
def test_global_instantiated_after_config_load(self):
def test_global_instantiated_after_config_load(self, _caches):
"""
If a cache is instantiated after the config is read, it will be
immediately resized to the correct size given the global factor if there
@@ -113,7 +117,7 @@ class CacheConfigTests(TestCase):
add_resizable_cache("foo", cache_resize_callback=cache.set_cache_factor)
self.assertEqual(cache.max_size, 150)
def test_cache_with_asterisk_in_name(self):
def test_cache_with_asterisk_in_name(self, _caches):
"""Some caches have asterisks in their name, test that they are set correctly."""
config = {
@@ -139,7 +143,7 @@ class CacheConfigTests(TestCase):
add_resizable_cache("*cache_c*", cache_resize_callback=cache_c.set_cache_factor)
self.assertEqual(cache_c.max_size, 200)
def test_apply_cache_factor_from_config(self):
def test_apply_cache_factor_from_config(self, _caches):
"""Caches can disable applying cache factor updates, mainly used by
event cache size.
"""

View File

@@ -423,6 +423,40 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase):
public3 = self.get_success(self.user_dir_helper.get_users_in_public_rooms())
self.assertEqual(set(public3), {(alice, room2), (bob, room2)})
def test_per_room_profile_doesnt_alter_directory_entry(self) -> None:
alice = self.register_user("alice", "pass")
alice_token = self.login(alice, "pass")
bob = self.register_user("bob", "pass")
# Alice should have a user directory entry created at registration.
users = self.get_success(self.user_dir_helper.get_profiles_in_user_directory())
self.assertEqual(
users[alice], ProfileInfo(display_name="alice", avatar_url=None)
)
# Alice makes a room for herself.
room = self.helper.create_room_as(alice, is_public=True, tok=alice_token)
# Alice sets a nickname unique to that room.
self.helper.send_state(
room,
"m.room.member",
{
"displayname": "Freddy Mercury",
"membership": "join",
},
alice_token,
state_key=alice,
)
# Alice's display name remains the same in the user directory.
search_result = self.get_success(self.handler.search_users(bob, alice, 10))
self.assertEqual(
search_result["results"],
[{"display_name": "alice", "avatar_url": None, "user_id": alice}],
0,
)
def test_private_room(self) -> None:
"""
A user can be searched for only by people that are either in a public