From e563e4bdf3e7d55d767e24205e594f520732986d Mon Sep 17 00:00:00 2001 From: Shay Date: Wed, 28 Aug 2024 03:29:12 -0700 Subject: [PATCH] Fix content length on federation `/thumbnail` responses (#17532) --- changelog.d/17532.bugfix | 1 + synapse/media/_base.py | 22 ++++++++++----------- synapse/media/media_repository.py | 6 +++--- synapse/media/thumbnailer.py | 32 +++++++++++++++++++++++++------ 4 files changed, 41 insertions(+), 20 deletions(-) create mode 100644 changelog.d/17532.bugfix diff --git a/changelog.d/17532.bugfix b/changelog.d/17532.bugfix new file mode 100644 index 0000000000..5b05f0f9ba --- /dev/null +++ b/changelog.d/17532.bugfix @@ -0,0 +1 @@ +Fix content-length on federation /thumbnail responses. diff --git a/synapse/media/_base.py b/synapse/media/_base.py index 9341d4859e..7877df62fa 100644 --- a/synapse/media/_base.py +++ b/synapse/media/_base.py @@ -60,8 +60,6 @@ from synapse.util.stringutils import is_ascii if TYPE_CHECKING: from synapse.server import HomeServer - from synapse.storage.databases.main.media_repository import LocalMedia - logger = logging.getLogger(__name__) @@ -290,7 +288,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 @@ -314,7 +314,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" @@ -322,16 +322,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 @@ -341,14 +341,14 @@ async def respond_with_multipart_responder( 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 8bc92305fe..0b74209232 100644 --- a/synapse/media/media_repository.py +++ b/synapse/media/media_repository.py @@ -471,7 +471,7 @@ class MediaRepository: 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 @@ class MediaRepository: 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 @@ class MediaRepository: 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 042851021c..ee1118a53a 100644 --- a/synapse/media/thumbnailer.py +++ b/synapse/media/thumbnailer.py @@ -348,7 +348,12 @@ class ThumbnailProvider: 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: @@ -360,7 +365,7 @@ class ThumbnailProvider: 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, @@ -369,13 +374,18 @@ class ThumbnailProvider: 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(self.hs, open(file_path, "rb")), - media_info, + file_info.thumbnail.type, + file_info.thumbnail.length, + None, ) else: await respond_with_file(self.hs, request, desired_type, file_path) @@ -580,7 +590,12 @@ class ThumbnailProvider: 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: @@ -634,7 +649,12 @@ class ThumbnailProvider: 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(