Fix content length on federation /thumbnail responses

changelog.d/17532.bugfix

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

synapse/media/_base.py

@@ -25,16 +25,7 @@
 import urllib
 from abc import ABC, abstractmethod
 from types import TracebackType
-from typing import (
-    TYPE_CHECKING,
-    Awaitable,
-    Dict,
-    Generator,
-    List,
-    Optional,
-    Tuple,
-    Type,
-)
+from typing import Awaitable, Dict, Generator, List, Optional, Tuple, Type
 
 import attr
 
@@ -49,10 +40,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 +266,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,24 +292,24 @@ 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"
 
         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 +319,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,))

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

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(