Fix content length on federation `/thumbnail` responses (#17532)

changelog.d/17532.bugfix

@@ -0,0 +1 @@
+Fix content-length on federation /thumbnail responses.

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,))

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

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(