From 011cc4df6c62d4749140ea30fa1b1dd114e3ae14 Mon Sep 17 00:00:00 2001 From: Sumner Evans Date: Fri, 25 Mar 2022 01:01:09 -0600 Subject: [PATCH 01/11] Remove re-assignment of self.clock in UploadResource Signed-off-by: Sumner Evans --- synapse/rest/media/v1/upload_resource.py | 1 - 1 file changed, 1 deletion(-) diff --git a/synapse/rest/media/v1/upload_resource.py b/synapse/rest/media/v1/upload_resource.py index e73e431dc9f1..dccada05a41a 100644 --- a/synapse/rest/media/v1/upload_resource.py +++ b/synapse/rest/media/v1/upload_resource.py @@ -42,7 +42,6 @@ def __init__(self, hs: "HomeServer", media_repo: "MediaRepository"): self.server_name = hs.hostname self.auth = hs.get_auth() self.max_upload_size = hs.config.media.max_upload_size - self.clock = hs.get_clock() async def _async_render_OPTIONS(self, request: SynapseRequest) -> None: respond_with_json(request, 200, {}, send_cors=True) From 98b82e04316b8c3acc9472bc31756c0dd3ec75cb Mon Sep 17 00:00:00 2001 From: Sumner Evans Date: Fri, 15 Apr 2022 09:01:08 -0600 Subject: [PATCH 02/11] config: add option to rate limit media creation Signed-off-by: Sumner Evans --- docs/sample_config.yaml | 4 ++++ synapse/config/ratelimiting.py | 10 ++++++++++ 2 files changed, 14 insertions(+) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 56a25c534f24..e1bd87eb4b72 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -964,6 +964,10 @@ log_config: "CONFDIR/SERVERNAME.log.config" #rc_third_party_invite: # per_second: 0.2 # burst_count: 10 +# +#rc_media_create: +# per_second: 10 +# burst_count: 50 # Ratelimiting settings for incoming federation # diff --git a/synapse/config/ratelimiting.py b/synapse/config/ratelimiting.py index 0587f5c10f14..835e9212269a 100644 --- a/synapse/config/ratelimiting.py +++ b/synapse/config/ratelimiting.py @@ -144,6 +144,12 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: }, ) + # Ratelimit create media requests: + self.rc_media_create = RateLimitConfig( + config.get("rc_media_create", {}), + defaults={"per_second": 10, "burst_count": 50}, + ) + def generate_config_section(self, **kwargs: Any) -> str: return """\ ## Ratelimiting ## @@ -234,6 +240,10 @@ def generate_config_section(self, **kwargs: Any) -> str: #rc_third_party_invite: # per_second: 0.2 # burst_count: 10 + # + #rc_media_create: + # per_second: 10 + # burst_count: 50 # Ratelimiting settings for incoming federation # From 9cd13e91ebf0e26502de33c7de837f80554b444f Mon Sep 17 00:00:00 2001 From: Sumner Evans Date: Fri, 15 Apr 2022 09:01:26 -0600 Subject: [PATCH 03/11] config: add option to gate MSC2246 async uploads Signed-off-by: Sumner Evans --- synapse/config/experimental.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index f2dfd49b0777..89b076e42bf6 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -84,3 +84,6 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: # MSC3772: A push rule for mutual relations. self.msc3772_enabled: bool = experimental.get("msc3772_enabled", False) + + # MSC2246 (async media uploads) + self.msc2246_enabled: bool = experimental.get("msc2246_enabled", False) From cb063806a7bb218ea011f5b5ab70282387358a64 Mon Sep 17 00:00:00 2001 From: Sumner Evans Date: Tue, 31 May 2022 09:19:57 -0600 Subject: [PATCH 04/11] config: add option for how long to wait until media ID expires --- docs/sample_config.yaml | 5 +++++ synapse/config/repository.py | 9 +++++++++ 2 files changed, 14 insertions(+) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index e1bd87eb4b72..58d24da5ff54 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -1038,6 +1038,11 @@ media_store_path: "DATADIR/media_store" # #max_image_pixels: 32M +# How long to wait before expiring created media IDs when MSC2246 support is +# enabled. +# +#unused_expiration_time: 1m + # Whether to generate new thumbnails on the fly to precisely match # the resolution requested by the client. If true then whenever # a new resolution is requested by the client the server will diff --git a/synapse/config/repository.py b/synapse/config/repository.py index f9c55143c39d..c09240b9226d 100644 --- a/synapse/config/repository.py +++ b/synapse/config/repository.py @@ -117,6 +117,10 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: self.max_image_pixels = self.parse_size(config.get("max_image_pixels", "32M")) self.max_spider_size = self.parse_size(config.get("max_spider_size", "10M")) + self.unused_expiration_time = self.parse_duration( + config.get("unused_expiration_time", "1m") + ) + self.media_store_path = self.ensure_directory( config.get("media_store_path", "media_store") ) @@ -292,6 +296,11 @@ def generate_config_section(self, data_dir_path: str, **kwargs: Any) -> str: # #max_image_pixels: 32M + # How long to wait before expiring created media IDs when MSC2246 support is + # enabled. + # + #unused_expiration_time: 1m + # Whether to generate new thumbnails on the fly to precisely match # the resolution requested by the client. If true then whenever # a new resolution is requested by the client the server will From 48d3d8d40d6975a16b946e72cb3228d978cc7646 Mon Sep 17 00:00:00 2001 From: Sumner Evans Date: Fri, 15 Apr 2022 09:05:11 -0600 Subject: [PATCH 05/11] media: move muxing of versioned endpoints to MediaRepositoryResource This allows for better handling of all of the versioned media endpoints Signed-off-by: Sumner Evans --- synapse/api/urls.py | 4 +--- synapse/app/generic_worker.py | 8 ++----- synapse/app/homeserver.py | 12 ++-------- synapse/rest/media/v1/media_repository.py | 29 +++++++++++++++++++++-- 4 files changed, 32 insertions(+), 21 deletions(-) diff --git a/synapse/api/urls.py b/synapse/api/urls.py index bd49fa6a5f03..9881d736bd2a 100644 --- a/synapse/api/urls.py +++ b/synapse/api/urls.py @@ -29,9 +29,7 @@ FEDERATION_UNSTABLE_PREFIX = FEDERATION_PREFIX + "/unstable" STATIC_PREFIX = "/_matrix/static" SERVER_KEY_V2_PREFIX = "/_matrix/key/v2" -MEDIA_R0_PREFIX = "/_matrix/media/r0" -MEDIA_V3_PREFIX = "/_matrix/media/v3" -LEGACY_MEDIA_PREFIX = "/_matrix/media/v1" +MEDIA_PREFIX = "/_matrix/media" class ConsentURIBuilder: diff --git a/synapse/app/generic_worker.py b/synapse/app/generic_worker.py index 89f8998f0ea7..4cc56fc3bb1c 100644 --- a/synapse/app/generic_worker.py +++ b/synapse/app/generic_worker.py @@ -27,9 +27,7 @@ from synapse.api.urls import ( CLIENT_API_PREFIX, FEDERATION_PREFIX, - LEGACY_MEDIA_PREFIX, - MEDIA_R0_PREFIX, - MEDIA_V3_PREFIX, + MEDIA_PREFIX, SERVER_KEY_V2_PREFIX, ) from synapse.app import _base @@ -338,9 +336,7 @@ def _listen_http(self, listener_config: ListenerConfig) -> None: resources.update( { - MEDIA_R0_PREFIX: media_repo, - MEDIA_V3_PREFIX: media_repo, - LEGACY_MEDIA_PREFIX: media_repo, + MEDIA_PREFIX: media_repo, "/_synapse/admin": admin_resource, } ) diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index 4c6c0658ab14..6003ddfc0626 100644 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -30,9 +30,7 @@ from synapse.api.urls import ( CLIENT_API_PREFIX, FEDERATION_PREFIX, - LEGACY_MEDIA_PREFIX, - MEDIA_R0_PREFIX, - MEDIA_V3_PREFIX, + MEDIA_PREFIX, SERVER_KEY_V2_PREFIX, STATIC_PREFIX, ) @@ -245,13 +243,7 @@ def _configure_named_resource( if name in ["media", "federation", "client"]: if self.config.server.enable_media_repo: media_repo = self.get_media_repository_resource() - resources.update( - { - MEDIA_R0_PREFIX: media_repo, - MEDIA_V3_PREFIX: media_repo, - LEGACY_MEDIA_PREFIX: media_repo, - } - ) + resources[MEDIA_PREFIX] = media_repo elif name == "media": raise ConfigError( "'media' resource conflicts with enable_media_repo=False" diff --git a/synapse/rest/media/v1/media_repository.py b/synapse/rest/media/v1/media_repository.py index a551458a9fef..e7999e9a7269 100644 --- a/synapse/rest/media/v1/media_repository.py +++ b/synapse/rest/media/v1/media_repository.py @@ -16,6 +16,7 @@ import logging import os import shutil +from enum import Enum from io import BytesIO from typing import IO, TYPE_CHECKING, Dict, List, Optional, Set, Tuple @@ -1035,7 +1036,14 @@ async def _remove_local_media_from_disk( return removed_media, len(removed_media) -class MediaRepositoryResource(Resource): +class MediaVersion(Enum): + R0 = b"r0" + V3 = b"v3" + LEGACY = b"v1" + UNSTABLE = b"unstable" + + +class VersionedMediaRepositoryResource(Resource): """File uploading and downloading. Uploads are POSTed to a resource which returns a token which is used to GET @@ -1080,7 +1088,7 @@ class MediaRepositoryResource(Resource): within a given rectangle. """ - def __init__(self, hs: "HomeServer"): + def __init__(self, hs: "HomeServer", version: MediaVersion): # If we're not configured to use it, raise if we somehow got here. if not hs.config.media.can_load_media_repo: raise ConfigError("Synapse is not configured to use a media repo.") @@ -1099,3 +1107,20 @@ def __init__(self, hs: "HomeServer"): PreviewUrlResource(hs, media_repo, media_repo.media_storage), ) self.putChild(b"config", MediaConfigResource(hs)) + + +class MediaRepositoryResource(Resource): + """ + Base media repository resource. This handles all /_matrix/media requests + and muxes them between the versioned media repository endpoints. + """ + + def __init__(self, hs: "HomeServer"): + # If we're not configured to use it, raise if we somehow got here. + if not hs.config.media.can_load_media_repo: + raise ConfigError("Synapse is not configured to use a media repo.") + + super().__init__() + + for version in MediaVersion: + self.putChild(version.value, VersionedMediaRepositoryResource(hs, version)) From f74e8edc129af4e49e2987342de290695ec2de14 Mon Sep 17 00:00:00 2001 From: Sumner Evans Date: Fri, 15 Apr 2022 09:46:17 -0600 Subject: [PATCH 06/11] media/create: add MSC2246 create endpoint Signed-off-by: Sumner Evans --- synapse/rest/media/v1/create_resource.py | 84 +++++++++++++++++++ synapse/rest/media/v1/media_repository.py | 40 +++++++++ .../databases/main/media_repository.py | 18 ++++ ...sc2246_add_unused_expires_at_for_media.sql | 20 +++++ 4 files changed, 162 insertions(+) create mode 100644 synapse/rest/media/v1/create_resource.py create mode 100644 synapse/storage/schema/main/delta/68/06_msc2246_add_unused_expires_at_for_media.sql diff --git a/synapse/rest/media/v1/create_resource.py b/synapse/rest/media/v1/create_resource.py new file mode 100644 index 000000000000..1dfe09e3904a --- /dev/null +++ b/synapse/rest/media/v1/create_resource.py @@ -0,0 +1,84 @@ +# Copyright 2014-2016 OpenMarket Ltd +# Copyright 2020-2021 The Matrix.org Foundation C.I.C. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import logging +from typing import TYPE_CHECKING + +from synapse.api.errors import LimitExceededError +from synapse.api.ratelimiting import Ratelimiter +from synapse.http.server import DirectServeJsonResource, respond_with_json +from synapse.http.site import SynapseRequest + +if TYPE_CHECKING: + from synapse.rest.media.v1.media_repository import MediaRepository + from synapse.server import HomeServer + + +logger = logging.getLogger(__name__) + + +class CreateResource(DirectServeJsonResource): + isLeaf = True + + def __init__(self, hs: "HomeServer", media_repo: "MediaRepository"): + super().__init__() + + self.media_repo = media_repo + self.clock = hs.get_clock() + self.auth = hs.get_auth() + + # A rate limiter for creating new media IDs. + self._create_media_rate_limiter = Ratelimiter( + store=hs.get_datastores().main, + clock=self.clock, + rate_hz=hs.config.ratelimiting.rc_media_create.per_second, + burst_count=hs.config.ratelimiting.rc_media_create.burst_count, + ) + + async def _async_render_OPTIONS(self, request: SynapseRequest) -> None: + respond_with_json(request, 200, {}, send_cors=True) + + async def _async_render_POST(self, request: SynapseRequest) -> None: + requester = await self.auth.get_user_by_req(request) + + # If the create media requests for the user are over the limit, drop + # them. + allowed, time_allowed = await self._create_media_rate_limiter.can_do_action( + requester + ) + if not allowed: + time_now_s = self.clock.time() + raise LimitExceededError( + retry_after_ms=int(1000 * (time_allowed - time_now_s)) + ) + + content_uri, unused_expires_at = await self.media_repo.create_media_id( + requester.user + ) + + logger.info( + "Created Media URI %r that if unused will expire at %d", + content_uri, + unused_expires_at, + ) + respond_with_json( + request, + 200, + { + "content_uri": content_uri, + "unused_expires_at": unused_expires_at, + }, + send_cors=True, + ) diff --git a/synapse/rest/media/v1/media_repository.py b/synapse/rest/media/v1/media_repository.py index e7999e9a7269..bb2408a78e7c 100644 --- a/synapse/rest/media/v1/media_repository.py +++ b/synapse/rest/media/v1/media_repository.py @@ -37,6 +37,7 @@ from synapse.http.site import SynapseRequest from synapse.logging.context import defer_to_thread from synapse.metrics.background_process_metrics import run_as_background_process +from synapse.rest.media.v1.create_resource import CreateResource from synapse.types import UserID from synapse.util.async_helpers import Linearizer from synapse.util.retryutils import NotRetryingDestination @@ -84,6 +85,7 @@ def __init__(self, hs: "HomeServer"): self.store = hs.get_datastores().main self.max_upload_size = hs.config.media.max_upload_size self.max_image_pixels = hs.config.media.max_image_pixels + self.unused_expiration_time = hs.config.media.unused_expiration_time Thumbnailer.set_limits(self.max_image_pixels) @@ -181,6 +183,27 @@ def mark_recently_accessed(self, server_name: Optional[str], media_id: str) -> N else: self.recently_accessed_locals.add(media_id) + async def create_media_id(self, auth_user: UserID) -> Tuple[str, int]: + """Create and store a media ID for a local user and return the mxc URL + + Args: + auth_user: The user_id of the uploader + + Returns: + The mxc url of the stored content + """ + media_id = random_string(24) + now = self.clock.time_msec() + # After the configured amount of time, don't allow the upload to start. + unused_expires_at = now + self.unused_expiration_time + await self.store.store_local_media_id( + media_id=media_id, + time_now_ms=now, + user_id=auth_user, + unused_expires_at=unused_expires_at, + ) + return f"mxc://{self.server_name}/{media_id}", unused_expires_at + async def create_content( self, media_type: str, @@ -1043,6 +1066,20 @@ class MediaVersion(Enum): UNSTABLE = b"unstable" +class MSC2246MediaRepositoryResource(Resource): + """Media creation and asynchronous uploading. + + This resource implements MSC2246 + https://github.com/matrix-org/matrix-spec-proposals/pull/2246 + """ + + def __init__(self, hs: "HomeServer"): + super().__init__() + media_repo = hs.get_media_repository() + + self.putChild(b"create", CreateResource(hs, media_repo)) + + class VersionedMediaRepositoryResource(Resource): """File uploading and downloading. @@ -1108,6 +1145,9 @@ def __init__(self, hs: "HomeServer", version: MediaVersion): ) self.putChild(b"config", MediaConfigResource(hs)) + if version == MediaVersion.UNSTABLE and hs.config.experimental.msc2246_enabled: + self.putChild(b"fi.mau.msc2246", MSC2246MediaRepositoryResource(hs)) + class MediaRepositoryResource(Resource): """ diff --git a/synapse/storage/databases/main/media_repository.py b/synapse/storage/databases/main/media_repository.py index deffdc19ce9f..a451e78c8190 100644 --- a/synapse/storage/databases/main/media_repository.py +++ b/synapse/storage/databases/main/media_repository.py @@ -302,6 +302,24 @@ def _get_local_media_before_txn(txn: LoggingTransaction) -> List[str]: "get_local_media_before", _get_local_media_before_txn ) + async def store_local_media_id( + self, + media_id: str, + time_now_ms: int, + user_id: UserID, + unused_expires_at: int, + ) -> None: + await self.db_pool.simple_insert( + "local_media_repository", + { + "media_id": media_id, + "created_ts": time_now_ms, + "user_id": user_id.to_string(), + "unused_expires_at": unused_expires_at, + }, + desc="store_local_media_id", + ) + async def store_local_media( self, media_id: str, diff --git a/synapse/storage/schema/main/delta/68/06_msc2246_add_unused_expires_at_for_media.sql b/synapse/storage/schema/main/delta/68/06_msc2246_add_unused_expires_at_for_media.sql new file mode 100644 index 000000000000..8e9438bad7a8 --- /dev/null +++ b/synapse/storage/schema/main/delta/68/06_msc2246_add_unused_expires_at_for_media.sql @@ -0,0 +1,20 @@ +/* Copyright 2022 The Matrix.org Foundation C.I.C + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +-- Add new colums to the `local_media_repository` to keep track of when the +-- media ID must be used by. This is to support MSC2246 async uploads. + +ALTER TABLE local_media_repository + ADD COLUMN unused_expires_at BIGINT DEFAULT NULL; From 46619f3b9aeea2190852cf57e470bbe6f202d769 Mon Sep 17 00:00:00 2001 From: Sumner Evans Date: Fri, 15 Apr 2022 10:12:22 -0600 Subject: [PATCH 07/11] media/upload: add support for async uploads This depends on the new newly-fixed locking mechanism from https://github.com/matrix-org/synapse/pull/12832 to prevent races. Signed-off-by: Sumner Evans --- synapse/rest/media/v1/media_repository.py | 75 ++++++++++++++++- synapse/rest/media/v1/upload_resource.py | 81 +++++++++++++++++-- .../databases/main/media_repository.py | 25 ++++++ 3 files changed, 172 insertions(+), 9 deletions(-) diff --git a/synapse/rest/media/v1/media_repository.py b/synapse/rest/media/v1/media_repository.py index bb2408a78e7c..c4da89d4b74f 100644 --- a/synapse/rest/media/v1/media_repository.py +++ b/synapse/rest/media/v1/media_repository.py @@ -26,6 +26,7 @@ from twisted.web.resource import Resource from synapse.api.errors import ( + Codes, FederationDeniedError, HttpResponseException, NotFoundError, @@ -204,6 +205,77 @@ async def create_media_id(self, auth_user: UserID) -> Tuple[str, int]: ) return f"mxc://{self.server_name}/{media_id}", unused_expires_at + async def verify_can_upload(self, media_id: str, auth_user: UserID) -> None: + """Verify that the media ID can be uploaded to by the given user. This + function checks that: + + * the media ID exists + * the media ID does not already have content + * the user uploading is the same as the one who created the media ID + * the media ID has not expired + + Args: + media_id: The media ID to verify + auth_user: The user_id of the uploader + """ + media = await self.store.get_local_media(media_id) + if media is None: + raise SynapseError(404, "Unknow media ID", errcode=Codes.NOT_FOUND) + + if media["user_id"] != str(auth_user): + raise SynapseError( + 403, + "Only the creator of the media ID can upload to it", + errcode=Codes.FORBIDDEN, + ) + + if media.get("media_length") is not None: + raise SynapseError( + 409, + "Media ID already has content", + errcode="FI.MAU.MSC2246_CANNOT_OVERWRITE_MEDIA", + ) + + if media.get("unused_expires_at", 0) < self.clock.time_msec(): + raise SynapseError( + 409, + "Media ID has expired", + errcode="FI.MAU.MSC2246_CANNOT_OVERWRITE_MEDIA", + ) + + async def update_content( + self, + media_id: str, + media_type: str, + upload_name: Optional[str], + content: IO, + content_length: int, + auth_user: UserID, + ) -> None: + """Update the content of the given media ID. + + Args: + media_id: The media ID to replace. + media_type: The content type of the file. + upload_name: The name of the file, if provided. + content: A file like object that is the content to store + content_length: The length of the content + auth_user: The user_id of the uploader + """ + file_info = FileInfo(server_name=None, file_id=media_id) + fname = await self.media_storage.store_file(content, file_info) + logger.info("Stored local media in file %r", fname) + + await self.store.update_local_media( + media_id=media_id, + media_type=media_type, + upload_name=upload_name, + media_length=content_length, + user_id=auth_user, + ) + + await self._generate_thumbnails(None, media_id, media_id, media_type) + async def create_content( self, media_type: str, @@ -1078,6 +1150,7 @@ def __init__(self, hs: "HomeServer"): media_repo = hs.get_media_repository() self.putChild(b"create", CreateResource(hs, media_repo)) + self.putChild(b"upload", UploadResource(hs, media_repo, True)) class VersionedMediaRepositoryResource(Resource): @@ -1133,7 +1206,7 @@ def __init__(self, hs: "HomeServer", version: MediaVersion): super().__init__() media_repo = hs.get_media_repository() - self.putChild(b"upload", UploadResource(hs, media_repo)) + self.putChild(b"upload", UploadResource(hs, media_repo, False)) self.putChild(b"download", DownloadResource(hs, media_repo)) self.putChild( b"thumbnail", ThumbnailResource(hs, media_repo, media_repo.media_storage) diff --git a/synapse/rest/media/v1/upload_resource.py b/synapse/rest/media/v1/upload_resource.py index dccada05a41a..bea63e521285 100644 --- a/synapse/rest/media/v1/upload_resource.py +++ b/synapse/rest/media/v1/upload_resource.py @@ -14,7 +14,7 @@ # limitations under the License. import logging -from typing import IO, TYPE_CHECKING, Dict, List, Optional +from typing import IO, TYPE_CHECKING, Dict, List, Optional, Tuple from synapse.api.errors import Codes, SynapseError from synapse.http.server import DirectServeJsonResource, respond_with_json @@ -22,32 +22,40 @@ from synapse.http.site import SynapseRequest from synapse.rest.media.v1.media_storage import SpamMediaException +from ._base import parse_media_id + if TYPE_CHECKING: from synapse.rest.media.v1.media_repository import MediaRepository from synapse.server import HomeServer logger = logging.getLogger(__name__) +# The name of the lock to use when uploading media. +_UPLOAD_MEDIA_LOCK_NAME = "upload_media" + class UploadResource(DirectServeJsonResource): isLeaf = True - def __init__(self, hs: "HomeServer", media_repo: "MediaRepository"): + def __init__( + self, + hs: "HomeServer", + media_repo: "MediaRepository", + enable_async_uploads: bool, + ): super().__init__() + self.enable_async_uploads = enable_async_uploads self.media_repo = media_repo self.filepaths = media_repo.filepaths self.store = hs.get_datastores().main - self.clock = hs.get_clock() self.server_name = hs.hostname self.auth = hs.get_auth() self.max_upload_size = hs.config.media.max_upload_size - async def _async_render_OPTIONS(self, request: SynapseRequest) -> None: - respond_with_json(request, 200, {}, send_cors=True) - - async def _async_render_POST(self, request: SynapseRequest) -> None: - requester = await self.auth.get_user_by_req(request) + def _get_file_metadata( + self, request: SynapseRequest + ) -> Tuple[int, Optional[str], str]: raw_content_length = request.getHeader("Content-Length") if raw_content_length is None: raise SynapseError(msg="Request must specify a Content-Length", code=400) @@ -90,6 +98,15 @@ async def _async_render_POST(self, request: SynapseRequest) -> None: # disposition = headers.getRawHeaders(b"Content-Disposition")[0] # TODO(markjh): parse content-dispostion + return content_length, upload_name, media_type + + async def _async_render_OPTIONS(self, request: SynapseRequest) -> None: + respond_with_json(request, 200, {}, send_cors=True) + + async def _async_render_POST(self, request: SynapseRequest) -> None: + requester = await self.auth.get_user_by_req(request) + content_length, upload_name, media_type = self._get_file_metadata(request) + try: content: IO = request.content # type: ignore content_uri = await self.media_repo.create_content( @@ -103,3 +120,51 @@ async def _async_render_POST(self, request: SynapseRequest) -> None: logger.info("Uploaded content with URI %r", content_uri) respond_with_json(request, 200, {"content_uri": content_uri}, send_cors=True) + + async def _async_render_PUT(self, request: SynapseRequest) -> None: + if not self.enable_async_uploads: + raise SynapseError( + 405, + "Asynchronous uploads are not enabled on this homeserver", + errcode=Codes.UNRECOGNIZED, + ) + + requester = await self.auth.get_user_by_req(request) + server_name, media_id, _ = parse_media_id(request) + + if server_name != self.server_name: + raise SynapseError( + 404, + "Non-local server name specified", + errcode=Codes.NOT_FOUND, + ) + + lock = await self.store.try_acquire_lock(_UPLOAD_MEDIA_LOCK_NAME, media_id) + if not lock: + raise SynapseError( + 409, + "Media ID is is locked and cannot be uploaded to", + errcode="FI.MAU.MSC2246_CANNOT_OVERWRITE_MEDIA", + ) + + async with lock: + await self.media_repo.verify_can_upload(media_id, requester.user) + content_length, upload_name, media_type = self._get_file_metadata(request) + + try: + content: IO = request.content # type: ignore + await self.media_repo.update_content( + media_id, + media_type, + upload_name, + content, + content_length, + requester.user, + ) + except SpamMediaException: + # For uploading of media we want to respond with a 400, instead of + # the default 404, as that would just be confusing. + raise SynapseError(400, "Bad content") + + logger.info("Uploaded content to URI %r", media_id) + respond_with_json(request, 200, {}, send_cors=True) diff --git a/synapse/storage/databases/main/media_repository.py b/synapse/storage/databases/main/media_repository.py index a451e78c8190..e18c6ed39a2e 100644 --- a/synapse/storage/databases/main/media_repository.py +++ b/synapse/storage/databases/main/media_repository.py @@ -175,6 +175,7 @@ async def get_local_media(self, media_id: str) -> Optional[Dict[str, Any]]: "quarantined_by", "url_cache", "safe_from_quarantine", + "user_id", ), allow_none=True, desc="get_local_media", @@ -344,6 +345,30 @@ async def store_local_media( desc="store_local_media", ) + async def update_local_media( + self, + media_id: str, + media_type: str, + upload_name: Optional[str], + media_length: int, + user_id: UserID, + url_cache: Optional[str] = None, + ) -> None: + await self.db_pool.simple_update_one( + "local_media_repository", + keyvalues={ + "user_id": user_id.to_string(), + "media_id": media_id, + }, + updatevalues={ + "media_type": media_type, + "upload_name": upload_name, + "media_length": media_length, + "url_cache": url_cache, + }, + desc="update_local_media", + ) + async def mark_local_media_as_safe(self, media_id: str, safe: bool = True) -> None: """Mark a local media as safe or unsafe from quarantining.""" await self.db_pool.simple_update_one( From 91a7ddd39055cef84cb0ead13495135a81cfbbd0 Mon Sep 17 00:00:00 2001 From: Sumner Evans Date: Fri, 15 Apr 2022 15:28:17 -0600 Subject: [PATCH 08/11] media/{download,thumbnail}: add support for stall parameter The stall will default to 20s if MSC2246 is not enabled. For remote media, the max_stall_ms parameter through to remote server. This way, if servers over federation attempt to request the media, the request will only 404 if the media is large (or the client doesn't upload as soon as it can). Signed-off-by: Sumner Evans --- synapse/rest/media/v1/_base.py | 2 + synapse/rest/media/v1/download_resource.py | 20 ++-- synapse/rest/media/v1/media_repository.py | 113 ++++++++++++++---- synapse/rest/media/v1/thumbnail_resource.py | 80 ++++++++----- .../databases/main/media_repository.py | 4 + 5 files changed, 159 insertions(+), 60 deletions(-) diff --git a/synapse/rest/media/v1/_base.py b/synapse/rest/media/v1/_base.py index c35d42fab89d..76a5f89456b7 100644 --- a/synapse/rest/media/v1/_base.py +++ b/synapse/rest/media/v1/_base.py @@ -49,6 +49,8 @@ "text/xml", ] +DEFAULT_MSC2246_DELAY = 20_000 + def parse_media_id(request: Request) -> Tuple[str, str, Optional[str]]: """Parses the server name, media ID and optional file name from the request URI diff --git a/synapse/rest/media/v1/download_resource.py b/synapse/rest/media/v1/download_resource.py index 6180fa575ebc..44d558695cf4 100644 --- a/synapse/rest/media/v1/download_resource.py +++ b/synapse/rest/media/v1/download_resource.py @@ -16,10 +16,10 @@ from typing import TYPE_CHECKING from synapse.http.server import DirectServeJsonResource, set_cors_headers -from synapse.http.servlet import parse_boolean +from synapse.http.servlet import parse_boolean, parse_integer from synapse.http.site import SynapseRequest -from ._base import parse_media_id, respond_404 +from ._base import DEFAULT_MSC2246_DELAY, parse_media_id, respond_404 if TYPE_CHECKING: from synapse.rest.media.v1.media_repository import MediaRepository @@ -35,6 +35,7 @@ def __init__(self, hs: "HomeServer", media_repo: "MediaRepository"): super().__init__() self.media_repo = media_repo self.server_name = hs.hostname + self.enable_msc2246 = hs.config.experimental.msc2246_enabled async def _async_render_GET(self, request: SynapseRequest) -> None: set_cors_headers(request) @@ -50,13 +51,14 @@ async def _async_render_GET(self, request: SynapseRequest) -> None: ) # Limited non-standard form of CSP for IE11 request.setHeader(b"X-Content-Security-Policy", b"sandbox;") - request.setHeader( - b"Referrer-Policy", - b"no-referrer", - ) + request.setHeader(b"Referrer-Policy", b"no-referrer") server_name, media_id, name = parse_media_id(request) + max_stall_ms = parse_integer( + request, "fi.mau.msc2246.max_stall_ms", default=DEFAULT_MSC2246_DELAY + ) + if server_name == self.server_name: - await self.media_repo.get_local_media(request, media_id, name) + await self.media_repo.get_local_media(request, media_id, name, max_stall_ms) else: allow_remote = parse_boolean(request, "allow_remote", default=True) if not allow_remote: @@ -68,4 +70,6 @@ async def _async_render_GET(self, request: SynapseRequest) -> None: respond_404(request) return - await self.media_repo.get_remote_media(request, server_name, media_id, name) + await self.media_repo.get_remote_media( + request, server_name, media_id, name, max_stall_ms + ) diff --git a/synapse/rest/media/v1/media_repository.py b/synapse/rest/media/v1/media_repository.py index c4da89d4b74f..af9ab0792f3d 100644 --- a/synapse/rest/media/v1/media_repository.py +++ b/synapse/rest/media/v1/media_repository.py @@ -18,7 +18,7 @@ import shutil from enum import Enum from io import BytesIO -from typing import IO, TYPE_CHECKING, Dict, List, Optional, Set, Tuple +from typing import IO, TYPE_CHECKING, Any, Dict, List, Optional, Set, Tuple import twisted.internet.error import twisted.web.http @@ -32,13 +32,14 @@ NotFoundError, RequestSendFailed, SynapseError, + cs_error, ) from synapse.config._base import ConfigError from synapse.config.repository import ThumbnailRequirement +from synapse.http.server import respond_with_json from synapse.http.site import SynapseRequest from synapse.logging.context import defer_to_thread from synapse.metrics.background_process_metrics import run_as_background_process -from synapse.rest.media.v1.create_resource import CreateResource from synapse.types import UserID from synapse.util.async_helpers import Linearizer from synapse.util.retryutils import NotRetryingDestination @@ -53,6 +54,7 @@ respond_with_responder, ) from .config_resource import MediaConfigResource +from .create_resource import CreateResource from .download_resource import DownloadResource from .filepath import MediaFilePaths from .media_storage import MediaStorage @@ -318,8 +320,64 @@ async def create_content( return "mxc://%s/%s" % (self.server_name, media_id) + def respond_not_yet_uploaded(self, request: SynapseRequest) -> None: + not_uploaded_error = cs_error( + "Media has not been uploaded yet", + code="FI.MAU.MSC2246_NOT_YET_UPLOADED", + retry_after_ms=5_000, + ) + respond_with_json(request, 404, not_uploaded_error, send_cors=True) + + async def get_local_media_info( + self, request: SynapseRequest, media_id: str, max_stall_ms: int + ) -> Optional[Dict[str, Any]]: + """Gets the info dictionary for given local media ID. If the media has + not been uploaded yet, this function will wait up to ``max_stall_ms`` + milliseconds for the media to be uploaded. + + Args: + request: The incoming request. + media_id: The media ID of the content. (This is the same as + the file_id for local content.) + max_stall_ms: the maximum number of milliseconds to wait for the + media to be uploaded. + + Returns: + Either the info dictionary for the given local media ID or + ``None``. If ``None``, then no further processing is necessary as + this function will send the necessary JSON response. + """ + wait_until = self.clock.time_msec() + max_stall_ms + while True: + # Get the info for the media + media_info = await self.store.get_local_media(media_id) + if not media_info: + respond_404(request) + return None + + if media_info["quarantined_by"]: + logger.info("Media is quarantined") + respond_404(request) + return None + + # The file has been uploaded, so stop looping + if media_info.get("media_length") is not None: + return media_info + + if self.clock.time_msec() >= wait_until: + break + + await self.clock.sleep(0.5) + + self.respond_not_yet_uploaded(request) + return None + async def get_local_media( - self, request: SynapseRequest, media_id: str, name: Optional[str] + self, + request: SynapseRequest, + media_id: str, + name: Optional[str], + max_stall_ms: int, ) -> None: """Responds to requests for local media, if exists, or returns 404. @@ -329,13 +387,14 @@ async def get_local_media( the file_id for local content.) name: Optional name that, if specified, will be used as the filename in the Content-Disposition header of the response. + max_stall_ms: the maximum number of milliseconds to wait for the + media to be uploaded. Returns: Resolves once a response has successfully been written to request """ - media_info = await self.store.get_local_media(media_id) - if not media_info or media_info["quarantined_by"]: - respond_404(request) + media_info = await self.get_local_media_info(request, media_id, max_stall_ms) + if not media_info: return self.mark_recently_accessed(None, media_id) @@ -360,6 +419,7 @@ async def get_remote_media( server_name: str, media_id: str, name: Optional[str], + max_stall_ms: int, ) -> None: """Respond to requests for remote media. @@ -369,6 +429,8 @@ async def get_remote_media( media_id: The media ID of the content (as defined by the remote server). name: Optional name that, if specified, will be used as the filename in the Content-Disposition header of the response. + max_stall_ms: the maximum number of milliseconds to wait for the + media to be uploaded. Returns: Resolves once a response has successfully been written to request @@ -383,14 +445,12 @@ async def get_remote_media( # We linearize here to ensure that we don't try and download remote # media multiple times concurrently - key = (server_name, media_id) - async with self.remote_media_linearizer.queue(key): + async with self.remote_media_linearizer.queue((server_name, media_id)): responder, media_info = await self._get_remote_media_impl( - server_name, media_id + server_name, media_id, max_stall_ms ) - # We deliberately stream the file outside the lock - if responder: + if responder and media_info: media_type = media_info["media_type"] media_length = media_info["media_length"] upload_name = name if name else media_info["upload_name"] @@ -398,18 +458,24 @@ async def get_remote_media( request, responder, media_type, media_length, upload_name ) else: - respond_404(request) + self.respond_not_yet_uploaded(request) + return - async def get_remote_media_info(self, server_name: str, media_id: str) -> dict: + async def get_remote_media_info( + self, server_name: str, media_id: str, max_stall_ms: int + ) -> Optional[dict]: """Gets the media info associated with the remote file, downloading if necessary. Args: server_name: Remote server_name where the media originated. media_id: The media ID of the content (as defined by the remote server). + max_stall_ms: the maximum number of milliseconds to wait for the + media to be uploaded. Returns: - The media info of the file + The media info of the file or ``None`` if the media wasn't uploaded + in time. """ if ( self.federation_domain_whitelist is not None @@ -419,10 +485,9 @@ async def get_remote_media_info(self, server_name: str, media_id: str) -> dict: # We linearize here to ensure that we don't try and download remote # media multiple times concurrently - key = (server_name, media_id) - async with self.remote_media_linearizer.queue(key): + async with self.remote_media_linearizer.queue((server_name, media_id)): responder, media_info = await self._get_remote_media_impl( - server_name, media_id + server_name, media_id, max_stall_ms ) # Ensure we actually use the responder so that it releases resources @@ -433,7 +498,7 @@ async def get_remote_media_info(self, server_name: str, media_id: str) -> dict: return media_info async def _get_remote_media_impl( - self, server_name: str, media_id: str + self, server_name: str, media_id: str, max_stall_ms: int ) -> Tuple[Optional[Responder], dict]: """Looks for media in local cache, if not there then attempt to download from remote server. @@ -442,6 +507,8 @@ async def _get_remote_media_impl( server_name (str): Remote server_name where the media originated. media_id (str): The media ID of the content (as defined by the remote server). + max_stall_ms: the maximum number of milliseconds to wait for the + media to be uploaded. Returns: A tuple of responder and the media info of the file. @@ -472,8 +539,7 @@ async def _get_remote_media_impl( try: media_info = await self._download_remote_file( - server_name, - media_id, + server_name, media_id, max_stall_ms ) except SynapseError: raise @@ -506,6 +572,7 @@ async def _download_remote_file( self, server_name: str, media_id: str, + max_stall_ms: int, ) -> dict: """Attempt to download the remote file from the given server name, using the given file_id as the local id. @@ -515,7 +582,8 @@ async def _download_remote_file( media_id: The media ID of the content (as defined by the remote server). This is different than the file_id, which is locally generated. - file_id: Local file ID + max_stall_ms: the maximum number of milliseconds to wait for the + media to be uploaded. Returns: The media info of the file. @@ -539,7 +607,8 @@ async def _download_remote_file( # tell the remote server to 404 if it doesn't # recognise the server_name, to make sure we don't # end up with a routing loop. - "allow_remote": "false" + "allow_remote": "false", + "fi.mau.msc2246.max_stall_ms": str(max_stall_ms), }, ) except RequestSendFailed as e: diff --git a/synapse/rest/media/v1/thumbnail_resource.py b/synapse/rest/media/v1/thumbnail_resource.py index 53b156524375..6a0b7a1fadce 100644 --- a/synapse/rest/media/v1/thumbnail_resource.py +++ b/synapse/rest/media/v1/thumbnail_resource.py @@ -24,6 +24,7 @@ from synapse.rest.media.v1.media_storage import MediaStorage from ._base import ( + DEFAULT_MSC2246_DELAY, FileInfo, ThumbnailInfo, parse_media_id, @@ -55,6 +56,7 @@ def __init__( self.media_storage = media_storage self.dynamic_thumbnails = hs.config.media.dynamic_thumbnails self.server_name = hs.hostname + self.enable_msc2246 = hs.config.experimental.msc2246_enabled async def _async_render_GET(self, request: SynapseRequest) -> None: set_cors_headers(request) @@ -63,26 +65,36 @@ async def _async_render_GET(self, request: SynapseRequest) -> None: height = parse_integer(request, "height", required=True) method = parse_string(request, "method", "scale") m_type = parse_string(request, "type", "image/png") + max_stall_ms = parse_integer( + request, "fi.mau.msc2246.max_stall_ms", default=DEFAULT_MSC2246_DELAY + ) if server_name == self.server_name: - if self.dynamic_thumbnails: - await self._select_or_generate_local_thumbnail( - request, media_id, width, height, method, m_type - ) - else: - await self._respond_local_thumbnail( - request, media_id, width, height, method, m_type - ) + local_response_function = ( + self._select_or_generate_local_thumbnail + if self.dynamic_thumbnails + else self._respond_local_thumbnail + ) + await local_response_function( + request, media_id, width, height, method, m_type, max_stall_ms + ) self.media_repo.mark_recently_accessed(None, media_id) else: - if self.dynamic_thumbnails: - await self._select_or_generate_remote_thumbnail( - request, server_name, media_id, width, height, method, m_type - ) - else: - await self._respond_remote_thumbnail( - request, server_name, media_id, width, height, method, m_type - ) + remote_response_fn = ( + self._select_or_generate_remote_thumbnail + if self.dynamic_thumbnails + else self._respond_remote_thumbnail + ) + await remote_response_fn( + request, + server_name, + media_id, + width, + height, + method, + m_type, + max_stall_ms, + ) self.media_repo.mark_recently_accessed(server_name, media_id) async def _respond_local_thumbnail( @@ -93,16 +105,14 @@ async def _respond_local_thumbnail( height: int, method: str, m_type: str, + max_stall_ms: int, ) -> None: - media_info = await self.store.get_local_media(media_id) - + media_info = await self.media_repo.get_local_media_info( + request, media_id, max_stall_ms + ) if not media_info: respond_404(request) return - if media_info["quarantined_by"]: - logger.info("Media is quarantined") - respond_404(request) - return thumbnail_infos = await self.store.get_local_media_thumbnails(media_id) await self._select_and_respond_with_thumbnail( @@ -126,16 +136,14 @@ async def _select_or_generate_local_thumbnail( desired_height: int, desired_method: str, desired_type: str, + max_stall_ms: int, ) -> None: - media_info = await self.store.get_local_media(media_id) - + media_info = await self.media_repo.get_local_media_info( + request, media_id, max_stall_ms + ) if not media_info: respond_404(request) return - if media_info["quarantined_by"]: - logger.info("Media is quarantined") - respond_404(request) - return thumbnail_infos = await self.store.get_local_media_thumbnails(media_id) for info in thumbnail_infos: @@ -192,8 +200,14 @@ async def _select_or_generate_remote_thumbnail( desired_height: int, desired_method: str, desired_type: str, + max_stall_ms: int, ) -> None: - media_info = await self.media_repo.get_remote_media_info(server_name, media_id) + media_info = await self.media_repo.get_remote_media_info( + server_name, media_id, max_stall_ms + ) + if not media_info: + respond_404(request) + return thumbnail_infos = await self.store.get_remote_media_thumbnails( server_name, media_id @@ -255,11 +269,17 @@ async def _respond_remote_thumbnail( height: int, method: str, m_type: str, + max_stall_ms: int, ) -> None: # TODO: Don't download the whole remote file # We should proxy the thumbnail from the remote server instead of # downloading the remote file and generating our own thumbnails. - media_info = await self.media_repo.get_remote_media_info(server_name, media_id) + media_info = await self.media_repo.get_remote_media_info( + server_name, media_id, max_stall_ms + ) + if not media_info: + respond_404(request) + return thumbnail_infos = await self.store.get_remote_media_thumbnails( server_name, media_id diff --git a/synapse/storage/databases/main/media_repository.py b/synapse/storage/databases/main/media_repository.py index e18c6ed39a2e..1a916ea623db 100644 --- a/synapse/storage/databases/main/media_repository.py +++ b/synapse/storage/databases/main/media_repository.py @@ -161,6 +161,9 @@ def __init__( async def get_local_media(self, media_id: str) -> Optional[Dict[str, Any]]: """Get the metadata for a local piece of media + Args: + media_id: the media ID to retrieve information for + Returns: None if the media_id doesn't exist. """ @@ -176,6 +179,7 @@ async def get_local_media(self, media_id: str) -> Optional[Dict[str, Any]]: "url_cache", "safe_from_quarantine", "user_id", + "unused_expires_at", ), allow_none=True, desc="get_local_media", From 474ff92058c114611ce0781d777cbc8c86c49b68 Mon Sep 17 00:00:00 2001 From: Sumner Evans Date: Fri, 15 Apr 2022 19:24:54 -0600 Subject: [PATCH 09/11] changelog: add entry for async uploads Signed-off-by: Sumner Evans --- changelog.d/12484.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/12484.feature diff --git a/changelog.d/12484.feature b/changelog.d/12484.feature new file mode 100644 index 000000000000..ec422b407dc2 --- /dev/null +++ b/changelog.d/12484.feature @@ -0,0 +1 @@ +Experimental support for asynchronous uploads as defined by [MSC2246](https://github.com/matrix-org/matrix-spec-proposals/pull/2246). Contributed by @sumnerevans at Beeper. From c45369b7f7ca86f81f365bd1efdba6f86827a000 Mon Sep 17 00:00:00 2001 From: Sumner Evans Date: Fri, 15 Apr 2022 20:28:58 -0600 Subject: [PATCH 10/11] tests: update paths for new media repo structure Signed-off-by: Sumner Evans --- tests/replication/test_multi_media_repo.py | 4 +- tests/rest/admin/test_admin.py | 4 +- tests/rest/admin/test_media.py | 12 ++-- tests/rest/admin/test_statistics.py | 2 +- tests/rest/admin/test_user.py | 4 +- tests/rest/media/v1/test_media_storage.py | 8 +-- tests/rest/media/v1/test_url_preview.py | 66 +++++++++++----------- 7 files changed, 51 insertions(+), 49 deletions(-) diff --git a/tests/replication/test_multi_media_repo.py b/tests/replication/test_multi_media_repo.py index 13aa5eb51aa5..7c4f594115d7 100644 --- a/tests/replication/test_multi_media_repo.py +++ b/tests/replication/test_multi_media_repo.py @@ -65,7 +65,9 @@ def _get_media_req( The channel for the *client* request and the *outbound* request for the media which the caller should respond to. """ - resource = hs.get_media_repository_resource().children[b"download"] + resource = ( + hs.get_media_repository_resource().children[b"r0"].children[b"download"] + ) channel = make_request( self.reactor, FakeSite(resource, self.reactor), diff --git a/tests/rest/admin/test_admin.py b/tests/rest/admin/test_admin.py index 82ac5991e6e4..f914b6d7e4d6 100644 --- a/tests/rest/admin/test_admin.py +++ b/tests/rest/admin/test_admin.py @@ -61,8 +61,8 @@ class QuarantineMediaTestCase(unittest.HomeserverTestCase): def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: # Allow for uploading and downloading to/from the media repo self.media_repo = hs.get_media_repository_resource() - self.download_resource = self.media_repo.children[b"download"] - self.upload_resource = self.media_repo.children[b"upload"] + self.download_resource = self.media_repo.children[b"v3"].children[b"download"] + self.upload_resource = self.media_repo.children[b"v3"].children[b"upload"] def _ensure_quarantined( self, admin_user_tok: str, server_and_media_id: str diff --git a/tests/rest/admin/test_media.py b/tests/rest/admin/test_media.py index e909e444ac1b..d66931b569ff 100644 --- a/tests/rest/admin/test_media.py +++ b/tests/rest/admin/test_media.py @@ -123,8 +123,8 @@ def test_delete_media(self) -> None: Tests that delete a media is successfully """ - download_resource = self.media_repo.children[b"download"] - upload_resource = self.media_repo.children[b"upload"] + download_resource = self.media_repo.children[b"v3"].children[b"download"] + upload_resource = self.media_repo.children[b"v3"].children[b"upload"] # Upload some media into the room response = self.helper.upload_media( @@ -562,7 +562,7 @@ def _create_media(self) -> str: """ Create a media and return media_id and server_and_media_id """ - upload_resource = self.media_repo.children[b"upload"] + upload_resource = self.media_repo.children[b"v3"].children[b"upload"] # Upload some media into the room response = self.helper.upload_media( @@ -586,7 +586,7 @@ def _access_media( """ Try to access a media and check the result """ - download_resource = self.media_repo.children[b"download"] + download_resource = self.media_repo.children[b"v3"].children[b"download"] media_id = server_and_media_id.split("/")[1] local_path = self.filepaths.local_media_filepath(media_id) @@ -641,7 +641,7 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: self.admin_user_tok = self.login("admin", "pass") # Create media - upload_resource = media_repo.children[b"upload"] + upload_resource = media_repo.children[b"v3"].children[b"upload"] # Upload some media into the room response = self.helper.upload_media( @@ -778,7 +778,7 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: self.admin_user_tok = self.login("admin", "pass") # Create media - upload_resource = media_repo.children[b"upload"] + upload_resource = media_repo.children[b"v3"].children[b"upload"] # Upload some media into the room response = self.helper.upload_media( diff --git a/tests/rest/admin/test_statistics.py b/tests/rest/admin/test_statistics.py index 7cb8ec57bad9..497872b0c68f 100644 --- a/tests/rest/admin/test_statistics.py +++ b/tests/rest/admin/test_statistics.py @@ -511,7 +511,7 @@ def _create_media(self, user_token: str, number_media: int) -> None: user_token: Access token of the user number_media: Number of media to be created for the user """ - upload_resource = self.media_repo.children[b"upload"] + upload_resource = self.media_repo.children[b"v3"].children[b"upload"] for _ in range(number_media): # Upload some media into the room self.helper.upload_media( diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index 0d44102237fe..120d6d6c84b7 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -3243,8 +3243,8 @@ def _create_media_and_access( Returns: The ID of the newly created media. """ - upload_resource = self.media_repo.children[b"upload"] - download_resource = self.media_repo.children[b"download"] + upload_resource = self.media_repo.children[b"v3"].children[b"upload"] + download_resource = self.media_repo.children[b"v3"].children[b"download"] # Upload some media into the room response = self.helper.upload_media( diff --git a/tests/rest/media/v1/test_media_storage.py b/tests/rest/media/v1/test_media_storage.py index 7204b2dfe075..952a9202727c 100644 --- a/tests/rest/media/v1/test_media_storage.py +++ b/tests/rest/media/v1/test_media_storage.py @@ -253,8 +253,8 @@ def write_to(r): def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: media_resource = hs.get_media_repository_resource() - self.download_resource = media_resource.children[b"download"] - self.thumbnail_resource = media_resource.children[b"thumbnail"] + self.download_resource = media_resource.children[b"v3"].children[b"download"] + self.thumbnail_resource = media_resource.children[b"v3"].children[b"thumbnail"] self.store = hs.get_datastores().main self.media_repo = hs.get_media_repository() @@ -605,8 +605,8 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: # Allow for uploading and downloading to/from the media repo self.media_repo = hs.get_media_repository_resource() - self.download_resource = self.media_repo.children[b"download"] - self.upload_resource = self.media_repo.children[b"upload"] + self.download_resource = self.media_repo.children[b"v3"].children[b"download"] + self.upload_resource = self.media_repo.children[b"v3"].children[b"upload"] load_legacy_spam_checkers(hs) diff --git a/tests/rest/media/v1/test_url_preview.py b/tests/rest/media/v1/test_url_preview.py index 3b24d0ace622..88754ef09107 100644 --- a/tests/rest/media/v1/test_url_preview.py +++ b/tests/rest/media/v1/test_url_preview.py @@ -120,7 +120,7 @@ def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer: def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: self.media_repo = hs.get_media_repository_resource() - self.preview_url = self.media_repo.children[b"preview_url"] + self.preview_url = self.media_repo.children[b"v3"].children[b"preview_url"] self.lookups: Dict[str, Any] = {} @@ -162,7 +162,7 @@ def test_cache_returns_correct_type(self) -> None: channel = self.make_request( "GET", - "preview_url?url=http://matrix.org", + "v3/preview_url?url=http://matrix.org", shorthand=False, await_result=False, ) @@ -186,7 +186,7 @@ def test_cache_returns_correct_type(self) -> None: # Check the cache returns the correct response channel = self.make_request( - "GET", "preview_url?url=http://matrix.org", shorthand=False + "GET", "v3/preview_url?url=http://matrix.org", shorthand=False ) # Check the cache response has the same content @@ -202,7 +202,7 @@ def test_cache_returns_correct_type(self) -> None: # Check the database cache returns the correct response channel = self.make_request( - "GET", "preview_url?url=http://matrix.org", shorthand=False + "GET", "v3/preview_url?url=http://matrix.org", shorthand=False ) # Check the cache response has the same content @@ -224,7 +224,7 @@ def test_non_ascii_preview_httpequiv(self) -> None: channel = self.make_request( "GET", - "preview_url?url=http://matrix.org", + "v3/preview_url?url=http://matrix.org", shorthand=False, await_result=False, ) @@ -254,7 +254,7 @@ def test_video_rejected(self) -> None: channel = self.make_request( "GET", - "preview_url?url=http://matrix.org", + "v3/preview_url?url=http://matrix.org", shorthand=False, await_result=False, ) @@ -290,7 +290,7 @@ def test_audio_rejected(self) -> None: channel = self.make_request( "GET", - "preview_url?url=http://matrix.org", + "v3/preview_url?url=http://matrix.org", shorthand=False, await_result=False, ) @@ -331,7 +331,7 @@ def test_non_ascii_preview_content_type(self) -> None: channel = self.make_request( "GET", - "preview_url?url=http://matrix.org", + "v3/preview_url?url=http://matrix.org", shorthand=False, await_result=False, ) @@ -366,7 +366,7 @@ def test_overlong_title(self) -> None: channel = self.make_request( "GET", - "preview_url?url=http://matrix.org", + "v3/preview_url?url=http://matrix.org", shorthand=False, await_result=False, ) @@ -399,7 +399,7 @@ def test_ipaddr(self) -> None: channel = self.make_request( "GET", - "preview_url?url=http://example.com", + "v3/preview_url?url=http://example.com", shorthand=False, await_result=False, ) @@ -428,7 +428,7 @@ def test_blacklisted_ip_specific(self) -> None: self.lookups["example.com"] = [(IPv4Address, "192.168.1.1")] channel = self.make_request( - "GET", "preview_url?url=http://example.com", shorthand=False + "GET", "v3/preview_url?url=http://example.com", shorthand=False ) # No requests made. @@ -449,7 +449,7 @@ def test_blacklisted_ip_range(self) -> None: self.lookups["example.com"] = [(IPv4Address, "1.1.1.2")] channel = self.make_request( - "GET", "preview_url?url=http://example.com", shorthand=False + "GET", "v3/preview_url?url=http://example.com", shorthand=False ) self.assertEqual(channel.code, 502) @@ -466,7 +466,7 @@ def test_blacklisted_ip_specific_direct(self) -> None: Blacklisted IP addresses, accessed directly, are not spidered. """ channel = self.make_request( - "GET", "preview_url?url=http://192.168.1.1", shorthand=False + "GET", "v3/preview_url?url=http://192.168.1.1", shorthand=False ) # No requests made. @@ -485,7 +485,7 @@ def test_blacklisted_ip_range_direct(self) -> None: Blacklisted IP ranges, accessed directly, are not spidered. """ channel = self.make_request( - "GET", "preview_url?url=http://1.1.1.2", shorthand=False + "GET", "v3/preview_url?url=http://1.1.1.2", shorthand=False ) self.assertEqual(channel.code, 403) @@ -506,7 +506,7 @@ def test_blacklisted_ip_range_whitelisted_ip(self) -> None: channel = self.make_request( "GET", - "preview_url?url=http://example.com", + "v3/preview_url?url=http://example.com", shorthand=False, await_result=False, ) @@ -542,7 +542,7 @@ def test_blacklisted_ip_with_external_ip(self) -> None: ] channel = self.make_request( - "GET", "preview_url?url=http://example.com", shorthand=False + "GET", "v3/preview_url?url=http://example.com", shorthand=False ) self.assertEqual(channel.code, 502) self.assertEqual( @@ -562,7 +562,7 @@ def test_blacklisted_ipv6_specific(self) -> None: ] channel = self.make_request( - "GET", "preview_url?url=http://example.com", shorthand=False + "GET", "v3/preview_url?url=http://example.com", shorthand=False ) # No requests made. @@ -583,7 +583,7 @@ def test_blacklisted_ipv6_range(self) -> None: self.lookups["example.com"] = [(IPv6Address, "2001:800::1")] channel = self.make_request( - "GET", "preview_url?url=http://example.com", shorthand=False + "GET", "v3/preview_url?url=http://example.com", shorthand=False ) self.assertEqual(channel.code, 502) @@ -600,7 +600,7 @@ def test_OPTIONS(self) -> None: OPTIONS returns the OPTIONS. """ channel = self.make_request( - "OPTIONS", "preview_url?url=http://example.com", shorthand=False + "OPTIONS", "v3/preview_url?url=http://example.com", shorthand=False ) self.assertEqual(channel.code, 200) self.assertEqual(channel.json_body, {}) @@ -614,7 +614,7 @@ def test_accept_language_config_option(self) -> None: # Build and make a request to the server channel = self.make_request( "GET", - "preview_url?url=http://example.com", + "v3/preview_url?url=http://example.com", shorthand=False, await_result=False, ) @@ -672,7 +672,7 @@ def test_data_url(self) -> None: channel = self.make_request( "GET", - f"preview_url?{query_params}", + f"v3/preview_url?{query_params}", shorthand=False, ) self.pump() @@ -693,7 +693,7 @@ def test_inline_data_url(self) -> None: channel = self.make_request( "GET", - "preview_url?url=http://matrix.org", + "v3/preview_url?url=http://matrix.org", shorthand=False, await_result=False, ) @@ -730,7 +730,7 @@ def test_oembed_photo(self) -> None: channel = self.make_request( "GET", - "preview_url?url=http://twitter.com/matrixdotorg/status/12345", + "v3/preview_url?url=http://twitter.com/matrixdotorg/status/12345", shorthand=False, await_result=False, ) @@ -790,7 +790,7 @@ def test_oembed_rich(self) -> None: channel = self.make_request( "GET", - "preview_url?url=http://twitter.com/matrixdotorg/status/12345", + "v3/preview_url?url=http://twitter.com/matrixdotorg/status/12345", shorthand=False, await_result=False, ) @@ -834,7 +834,7 @@ def test_oembed_format(self) -> None: channel = self.make_request( "GET", - "preview_url?url=http://www.hulu.com/watch/12345", + "v3/preview_url?url=http://www.hulu.com/watch/12345", shorthand=False, await_result=False, ) @@ -892,7 +892,7 @@ def test_oembed_autodiscovery(self) -> None: channel = self.make_request( "GET", - "preview_url?url=http://www.twitter.com/matrixdotorg/status/12345", + "v3/preview_url?url=http://www.twitter.com/matrixdotorg/status/12345", shorthand=False, await_result=False, ) @@ -975,7 +975,7 @@ def _download_image(self) -> Tuple[str, str]: channel = self.make_request( "GET", - "preview_url?url=http://cdn.twitter.com/matrixdotorg", + "v3/preview_url?url=http://cdn.twitter.com/matrixdotorg", shorthand=False, await_result=False, ) @@ -1017,7 +1017,7 @@ def test_storage_providers_exclude_files(self) -> None: # Check fetching channel = self.make_request( "GET", - f"download/{host}/{media_id}", + f"v3/download/{host}/{media_id}", shorthand=False, await_result=False, ) @@ -1030,7 +1030,7 @@ def test_storage_providers_exclude_files(self) -> None: channel = self.make_request( "GET", - f"download/{host}/{media_id}", + f"v3/download/{host}/{media_id}", shorthand=False, await_result=False, ) @@ -1065,7 +1065,7 @@ def test_storage_providers_exclude_thumbnails(self) -> None: # Check fetching channel = self.make_request( "GET", - f"thumbnail/{host}/{media_id}?width=32&height=32&method=scale", + f"v3/thumbnail/{host}/{media_id}?width=32&height=32&method=scale", shorthand=False, await_result=False, ) @@ -1083,7 +1083,7 @@ def test_storage_providers_exclude_thumbnails(self) -> None: channel = self.make_request( "GET", - f"thumbnail/{host}/{media_id}?width=32&height=32&method=scale", + f"v3/thumbnail/{host}/{media_id}?width=32&height=32&method=scale", shorthand=False, await_result=False, ) @@ -1135,7 +1135,7 @@ def test_blacklist_port(self) -> None: channel = self.make_request( "GET", - "preview_url?url=" + bad_url, + "v3/preview_url?url=" + bad_url, shorthand=False, await_result=False, ) @@ -1144,7 +1144,7 @@ def test_blacklist_port(self) -> None: channel = self.make_request( "GET", - "preview_url?url=" + good_url, + "v3/preview_url?url=" + good_url, shorthand=False, await_result=False, ) From 0ab87e583facbac6f5af8b44cbb3f134e257408b Mon Sep 17 00:00:00 2001 From: Sumner Evans Date: Mon, 30 May 2022 08:08:37 -0600 Subject: [PATCH 11/11] tests/media_storage: add async media parameter Signed-off-by: Sumner Evans --- tests/rest/media/v1/test_media_storage.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/rest/media/v1/test_media_storage.py b/tests/rest/media/v1/test_media_storage.py index 952a9202727c..e2800e4a2eb5 100644 --- a/tests/rest/media/v1/test_media_storage.py +++ b/tests/rest/media/v1/test_media_storage.py @@ -280,7 +280,13 @@ def _req( self.assertEqual( self.fetches[0][2], "/_matrix/media/r0/download/" + self.media_id ) - self.assertEqual(self.fetches[0][3], {"allow_remote": "false"}) + self.assertEqual( + self.fetches[0][3], + { + "allow_remote": "false", + "fi.mau.msc2246.max_stall_ms": "20000", + }, + ) headers = { b"Content-Length": [b"%d" % (len(self.test_image.data))],