fix broken avatar checks when server_name contains a port (#13927)

Fixes check_avatar_size_and_mime_type() to successfully update avatars on homeservers running on non-default ports which it would mistakenly treat as remote homeserver while validating the avatar's size and mime type.

Signed-off-by: Ashish Kumar ashfame@users.noreply.github.com
This commit is contained in:
Ashish Kumar 2022-10-26 18:51:23 +04:00 committed by GitHub
parent 86b7d9b886
commit 0cfbb35131
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 55 additions and 1 deletions

1
changelog.d/13927.bugfix Normal file
View file

@ -0,0 +1 @@
Fix a bug which prevented setting an avatar on homeservers which have an explicit port in their `server_name` and have `max_avatar_size` and/or `allowed_avatar_mimetypes` configuration. Contributed by @ashfame.

View file

@ -307,7 +307,11 @@ class ProfileHandler:
if not self.max_avatar_size and not self.allowed_avatar_mimetypes:
return True
server_name, _, media_id = parse_and_validate_mxc_uri(mxc)
host, port, media_id = parse_and_validate_mxc_uri(mxc)
if port is not None:
server_name = host + ":" + str(port)
else:
server_name = host
if server_name == self.server_name:
media_info = await self.store.get_local_media(media_id)

View file

@ -14,6 +14,8 @@
from typing import Any, Awaitable, Callable, Dict
from unittest.mock import Mock
from parameterized import parameterized
from twisted.test.proto_helpers import MemoryReactor
import synapse.types
@ -327,6 +329,53 @@ class ProfileTestCase(unittest.HomeserverTestCase):
)
self.assertFalse(res)
@unittest.override_config(
{"server_name": "test:8888", "allowed_avatar_mimetypes": ["image/png"]}
)
def test_avatar_constraint_on_local_server_with_port(self):
"""Test that avatar metadata is correctly fetched when the media is on a local
server and the server has an explicit port.
(This was previously a bug)
"""
local_server_name = self.hs.config.server.server_name
media_id = "local"
local_mxc = f"mxc://{local_server_name}/{media_id}"
# mock up the existence of the avatar file
self._setup_local_files({media_id: {"mimetype": "image/png"}})
# and now check that check_avatar_size_and_mime_type is happy
self.assertTrue(
self.get_success(self.handler.check_avatar_size_and_mime_type(local_mxc))
)
@parameterized.expand([("remote",), ("remote:1234",)])
@unittest.override_config({"allowed_avatar_mimetypes": ["image/png"]})
def test_check_avatar_on_remote_server(self, remote_server_name: str) -> None:
"""Test that avatar metadata is correctly fetched from a remote server"""
media_id = "remote"
remote_mxc = f"mxc://{remote_server_name}/{media_id}"
# if the media is remote, check_avatar_size_and_mime_type just checks the
# media cache, so we don't need to instantiate a real remote server. It is
# sufficient to poke an entry into the db.
self.get_success(
self.hs.get_datastores().main.store_cached_remote_media(
media_id=media_id,
media_type="image/png",
media_length=50,
origin=remote_server_name,
time_now_ms=self.clock.time_msec(),
upload_name=None,
filesystem_id="xyz",
)
)
self.assertTrue(
self.get_success(self.handler.check_avatar_size_and_mime_type(remote_mxc))
)
def _setup_local_files(self, names_and_props: Dict[str, Dict[str, Any]]):
"""Stores metadata about files in the database.