From 395f77587ac727c585f7fd9ca8b3f0ebe2ff7a68 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 6 Aug 2024 14:29:58 -0700 Subject: [PATCH 1/4] fix content length on federation `/thumbnail` responses --- synapse/media/_base.py | 25 +++++++++++------------- synapse/media/media_repository.py | 6 +++--- synapse/media/thumbnailer.py | 32 +++++++++++++++++++++++++------ 3 files changed, 40 insertions(+), 23 deletions(-) diff --git a/synapse/media/_base.py b/synapse/media/_base.py index 1b268ce4d42..41dafc66ad2 100644 --- a/synapse/media/_base.py +++ b/synapse/media/_base.py @@ -26,7 +26,6 @@ from abc import ABC, abstractmethod from types import TracebackType from typing import ( - TYPE_CHECKING, Awaitable, Dict, Generator, @@ -49,10 +48,6 @@ from synapse.util import Clock from synapse.util.stringutils import is_ascii -if TYPE_CHECKING: - from synapse.storage.databases.main.media_repository import LocalMedia - - logger = logging.getLogger(__name__) # list all text content types that will have the charset default to UTF-8 when @@ -279,7 +274,9 @@ async def respond_with_multipart_responder( clock: Clock, request: SynapseRequest, responder: "Optional[Responder]", - media_info: "LocalMedia", + media_type: str, + media_length: Optional[int], + upload_name: Optional[str], ) -> None: """ Responds to requests originating from the federation media `/download` endpoint by @@ -303,7 +300,7 @@ async def respond_with_multipart_responder( ) return - if media_info.media_type.lower().split(";", 1)[0] in INLINE_CONTENT_TYPES: + if media_type.lower().split(";", 1)[0] in INLINE_CONTENT_TYPES: disposition = "inline" else: disposition = "attachment" @@ -311,16 +308,16 @@ async def respond_with_multipart_responder( def _quote(x: str) -> str: return urllib.parse.quote(x.encode("utf-8")) - if media_info.upload_name: - if _can_encode_filename_as_token(media_info.upload_name): + if upload_name: + if _can_encode_filename_as_token(upload_name): disposition = "%s; filename=%s" % ( disposition, - media_info.upload_name, + upload_name, ) else: disposition = "%s; filename*=utf-8''%s" % ( disposition, - _quote(media_info.upload_name), + _quote(upload_name), ) from synapse.media.media_storage import MultipartFileConsumer @@ -330,14 +327,14 @@ def _quote(x: str) -> str: multipart_consumer = MultipartFileConsumer( clock, request, - media_info.media_type, + media_type, {}, disposition, - media_info.media_length, + media_length, ) logger.debug("Responding to media request with responder %s", responder) - if media_info.media_length is not None: + if media_length is not None: content_length = multipart_consumer.content_length() assert content_length is not None request.setHeader(b"Content-Length", b"%d" % (content_length,)) diff --git a/synapse/media/media_repository.py b/synapse/media/media_repository.py index 8bc92305fe5..0b742092322 100644 --- a/synapse/media/media_repository.py +++ b/synapse/media/media_repository.py @@ -471,7 +471,7 @@ async def get_local_media( responder = await self.media_storage.fetch_media(file_info) if federation: await respond_with_multipart_responder( - self.clock, request, responder, media_info + self.clock, request, responder, media_type, media_length, upload_name ) else: await respond_with_responder( @@ -1008,7 +1008,7 @@ async def generate_local_exact_thumbnail( t_method: str, t_type: str, url_cache: bool, - ) -> Optional[str]: + ) -> Optional[Tuple[str, FileInfo]]: input_path = await self.media_storage.ensure_media_is_in_local_cache( FileInfo(None, media_id, url_cache=url_cache) ) @@ -1070,7 +1070,7 @@ async def generate_local_exact_thumbnail( t_len, ) - return output_path + return output_path, file_info # Could not generate thumbnail. return None diff --git a/synapse/media/thumbnailer.py b/synapse/media/thumbnailer.py index ef6aa8ccf54..78d0d48bd2f 100644 --- a/synapse/media/thumbnailer.py +++ b/synapse/media/thumbnailer.py @@ -347,7 +347,12 @@ async def select_or_generate_local_thumbnail( if responder: if for_federation: await respond_with_multipart_responder( - self.hs.get_clock(), request, responder, media_info + self.hs.get_clock(), + request, + responder, + info.type, + info.length, + None, ) return else: @@ -359,7 +364,7 @@ async def select_or_generate_local_thumbnail( logger.debug("We don't have a thumbnail of that size. Generating") # Okay, so we generate one. - file_path = await self.media_repo.generate_local_exact_thumbnail( + thumbnail_result = await self.media_repo.generate_local_exact_thumbnail( media_id, desired_width, desired_height, @@ -368,13 +373,18 @@ async def select_or_generate_local_thumbnail( url_cache=bool(media_info.url_cache), ) - if file_path: + if thumbnail_result: + file_path, file_info = thumbnail_result + assert file_info.thumbnail is not None + if for_federation: await respond_with_multipart_responder( self.hs.get_clock(), request, FileResponder(open(file_path, "rb")), - media_info, + file_info.thumbnail.type, + file_info.thumbnail.length, + None, ) else: await respond_with_file(request, desired_type, file_path) @@ -579,7 +589,12 @@ async def _select_and_respond_with_thumbnail( if for_federation: assert media_info is not None await respond_with_multipart_responder( - self.hs.get_clock(), request, responder, media_info + self.hs.get_clock(), + request, + responder, + file_info.thumbnail.type, + file_info.thumbnail.length, + None, ) return else: @@ -633,7 +648,12 @@ async def _select_and_respond_with_thumbnail( if for_federation: assert media_info is not None await respond_with_multipart_responder( - self.hs.get_clock(), request, responder, media_info + self.hs.get_clock(), + request, + responder, + file_info.thumbnail.type, + file_info.thumbnail.length, + None, ) else: await respond_with_responder( From 8cb68d06de94b21eb94a5b677b1e9e5d21cdcdcc Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 6 Aug 2024 14:37:26 -0700 Subject: [PATCH 2/4] newsfragment --- changelog.d/17532.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/17532.bugfix diff --git a/changelog.d/17532.bugfix b/changelog.d/17532.bugfix new file mode 100644 index 00000000000..5b05f0f9ba7 --- /dev/null +++ b/changelog.d/17532.bugfix @@ -0,0 +1 @@ +Fix content-length on federation /thumbnail responses. From 5e26ba176b2effa8adf3e3bd118bfaa1fa1321a3 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 6 Aug 2024 14:46:13 -0700 Subject: [PATCH 3/4] lint --- synapse/media/_base.py | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/synapse/media/_base.py b/synapse/media/_base.py index 41dafc66ad2..b0bb2801994 100644 --- a/synapse/media/_base.py +++ b/synapse/media/_base.py @@ -25,15 +25,7 @@ import urllib from abc import ABC, abstractmethod from types import TracebackType -from typing import ( - Awaitable, - Dict, - Generator, - List, - Optional, - Tuple, - Type, -) +from typing import Awaitable, Dict, Generator, List, Optional, Tuple, Type import attr From 1bc4ecdb14bb43bf994eb39196b0b44a176fb4c0 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Thu, 22 Aug 2024 11:10:04 -0700 Subject: [PATCH 4/4] lint --- synapse/media/_base.py | 1 - 1 file changed, 1 deletion(-) diff --git a/synapse/media/_base.py b/synapse/media/_base.py index a2a44573731..7877df62faa 100644 --- a/synapse/media/_base.py +++ b/synapse/media/_base.py @@ -25,7 +25,6 @@ import urllib from abc import ABC, abstractmethod from types import TracebackType - from typing import ( TYPE_CHECKING, Awaitable,