Provide more info why we don't have any thumbnails to serve

changelog.d/13038.feature

@@ -0,0 +1 @@
+Provide more info why we don't have any thumbnails to serve.

synapse/config/repository.py

@@ -42,6 +42,18 @@
  # method: %(method)s
 """
 
+# A map from the given media type to the type of thumbnail we should generate
+# for it.
+THUMBNAIL_SUPPORTED_MEDIA_FORMAT_MAP = {
+ "image/jpeg": "jpeg",
+ "image/jpg": "jpeg",
+ "image/webp": "jpeg",
+ # Thumbnails can only be jpeg or png. We choose png thumbnails for gif
+ # because it can have transparency.
+ "image/gif": "png",
+ "image/png": "png",
+}
+
 HTTP_PROXY_SET_WARNING = """\
 The Synapse config url_preview_ip_range_blacklist will be ignored as an HTTP(s) proxy is configured."""
 
@@ -79,13 +91,22 @@ def parse_thumbnail_requirements(
  width = size["width"]
  height = size["height"]
  method = size["method"]
- jpeg_thumbnail = ThumbnailRequirement(width, height, method, "image/jpeg")
- png_thumbnail = ThumbnailRequirement(width, height, method, "image/png")
- requirements.setdefault("image/jpeg", []).append(jpeg_thumbnail)
- requirements.setdefault("image/jpg", []).append(jpeg_thumbnail)
- requirements.setdefault("image/webp", []).append(jpeg_thumbnail)
- requirements.setdefault("image/gif", []).append(png_thumbnail)
- requirements.setdefault("image/png", []).append(png_thumbnail)
+
+ for format, thumbnail_format in THUMBNAIL_SUPPORTED_MEDIA_FORMAT_MAP.items():
+ requirement = requirements.setdefault(format, [])
+ if thumbnail_format == "jpeg":
+ requirement.append(
+ ThumbnailRequirement(width, height, method, "image/jpeg")
+ )
+ elif thumbnail_format == "png":
+ requirement.append(
+ ThumbnailRequirement(width, height, method, "image/png")
+ )
+ else:
+ raise Exception(
+ "Unknown thumbnail mapping from %s to %s. This is a Synapse problem, please report!"
+ % (format, thumbnail_format)
+ )
  return {
  media_type: tuple(thumbnails) for media_type, thumbnails in requirements.items()
  }

synapse/rest/media/v1/thumbnail_resource.py

@@ -17,9 +17,11 @@
 import logging
 from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple
 
-from synapse.api.errors import SynapseError
+from synapse.api.errors import Codes, SynapseError, cs_error
+from synapse.config.repository import THUMBNAIL_SUPPORTED_MEDIA_FORMAT_MAP
 from synapse.http.server import (
  DirectServeJsonResource,
+ respond_with_json,
  set_corp_headers,
  set_cors_headers,
 )
@@ -309,6 +311,19 @@ async def _select_and_respond_with_thumbnail(
  url_cache: True if this is from a URL cache.
  server_name: The server name, if this is a remote thumbnail.
  """
+ logger.debug(
+ "_select_and_respond_with_thumbnail: media_id=%s desired=%sx%s (%s) thumbnail_infos=%s",
+ media_id,
+ desired_width,
+ desired_height,
+ desired_method,
+ thumbnail_infos,
+ )
+
+ # If `dynamic_thumbnails` is enabled, we expect Synapse to go down a
+ # different code path to handle it.
+ assert not self.dynamic_thumbnails
+
  if thumbnail_infos:
  file_info = self._select_thumbnail(
  desired_width,
@@ -384,8 +399,29 @@ async def _select_and_respond_with_thumbnail(
  file_info.thumbnail.length,
  )
  else:
+ # This might be because:
+ # 1. We can't create thumbnails for the given media (corrupted or
+ # unsupported file type), or
+ # 2. The thumbnailing process never ran or errored out initially
+ # when the media was first uploaded (these bugs should be
+ # reported and fixed).
+ # Note that we don't attempt to generate a thumbnail now because
+ # `dynamic_thumbnails` is disabled.
  logger.info("Failed to find any generated thumbnails")
- respond_404(request)
+
+ respond_with_json(
+ request,
+ 400,
+ cs_error(
+ "Cannot find any thumbnails for the requested media (%r). This might mean the media is not a supported_media_format=(%s) or that thumbnailing failed for some other reason. (Dynamic thumbnails are disabled on this server.)"
+ % (
+ request.postpath,
+ ", ".join(THUMBNAIL_SUPPORTED_MEDIA_FORMAT_MAP.keys()),
+ ),
+ code=Codes.UNKNOWN,
+ ),
+ send_cors=True,
+ )
 
  def _select_thumbnail(
  self,

tests/rest/media/v1/test_media_storage.py

@@ -124,7 +124,9 @@ class _TestImage:
  expected_scaled: The expected bytes from scaled thumbnailing, or None if
  test should just check for a valid image returned.
  expected_found: True if the file should exist on the server, or False if
- a 404 is expected.
+ a 404/400 is expected.
+ unable_to_thumbnail: True if we expect the thumbnailing to fail (400), or
+ False if the thumbnailing should succeed or a normal 404 is expected.
  """
 
  data: bytes
@@ -133,6 +135,7 @@ class _TestImage:
  expected_cropped: Optional[bytes] = None
  expected_scaled: Optional[bytes] = None
  expected_found: bool = True
+ unable_to_thumbnail: bool = False
 
 
 @parameterized_class(
@@ -190,6 +193,7 @@ class _TestImage:
  b"image/gif",
  b".gif",
  expected_found=False,
+ unable_to_thumbnail=True,
  ),
  ),
  ],
@@ -364,18 +368,29 @@ def test_disposition_none(self) -> None:
  def test_thumbnail_crop(self) -> None:
  """Test that a cropped remote thumbnail is available."""
  self._test_thumbnail(
- "crop", self.test_image.expected_cropped, self.test_image.expected_found
+ "crop",
+ self.test_image.expected_cropped,
+ expected_found=self.test_image.expected_found,
+ unable_to_thumbnail=self.test_image.unable_to_thumbnail,
  )
 
  def test_thumbnail_scale(self) -> None:
  """Test that a scaled remote thumbnail is available."""
  self._test_thumbnail(
- "scale", self.test_image.expected_scaled, self.test_image.expected_found
+ "scale",
+ self.test_image.expected_scaled,
+ expected_found=self.test_image.expected_found,
+ unable_to_thumbnail=self.test_image.unable_to_thumbnail,
  )
 
  def test_invalid_type(self) -> None:
  """An invalid thumbnail type is never available."""
- self._test_thumbnail("invalid", None, False)
+ self._test_thumbnail(
+ "invalid",
+ None,
+ expected_found=False,
+ unable_to_thumbnail=self.test_image.unable_to_thumbnail,
+ )
 
  @unittest.override_config(
  {"thumbnail_sizes": [{"width": 32, "height": 32, "method": "scale"}]}
@@ -384,7 +399,12 @@ def test_no_thumbnail_crop(self) -> None:
  """
  Override the config to generate only scaled thumbnails, but request a cropped one.
  """
- self._test_thumbnail("crop", None, False)
+ self._test_thumbnail(
+ "crop",
+ None,
+ expected_found=False,
+ unable_to_thumbnail=self.test_image.unable_to_thumbnail,
+ )
 
  @unittest.override_config(
  {"thumbnail_sizes": [{"width": 32, "height": 32, "method": "crop"}]}
@@ -393,14 +413,22 @@ def test_no_thumbnail_scale(self) -> None:
  """
  Override the config to generate only cropped thumbnails, but request a scaled one.
  """
- self._test_thumbnail("scale", None, False)
+ self._test_thumbnail(
+ "scale",
+ None,
+ expected_found=False,
+ unable_to_thumbnail=self.test_image.unable_to_thumbnail,
+ )
 
  def test_thumbnail_repeated_thumbnail(self) -> None:
  """Test that fetching the same thumbnail works, and deleting the on disk
  thumbnail regenerates it.
  """
  self._test_thumbnail(
- "scale", self.test_image.expected_scaled, self.test_image.expected_found
+ "scale",
+ self.test_image.expected_scaled,
+ expected_found=self.test_image.expected_found,
+ unable_to_thumbnail=self.test_image.unable_to_thumbnail,
  )
 
  if not self.test_image.expected_found:
@@ -457,8 +485,24 @@ def test_thumbnail_repeated_thumbnail(self) -> None:
  )
 
  def _test_thumbnail(
- self, method: str, expected_body: Optional[bytes], expected_found: bool
+ self,
+ method: str,
+ expected_body: Optional[bytes],
+ expected_found: bool,
+ unable_to_thumbnail: bool = False,
  ) -> None:
+ """Test the given thumbnailing method works as expected.
+
+ Args:
+ method: The thumbnailing method to use (crop, scale).
+ expected_body: The expected bytes from thumbnailing, or None if
+ test should just check for a valid image.
+ expected_found: True if the file should exist on the server, or False if
+ a 404/400 is expected.
+ unable_to_thumbnail: True if we expect the thumbnailing to fail (400), or
+ False if the thumbnailing should succeed or a normal 404 is expected.
+ """
+
  params = "?width=32&height=32&method=" + method
  channel = make_request(
  self.reactor,
@@ -494,6 +538,16 @@ def _test_thumbnail(
  else:
  # ensure that the result is at least some valid image
  Image.open(BytesIO(channel.result["body"]))
+ elif unable_to_thumbnail:
+ # A 400 with a JSON body.
+ self.assertEqual(channel.code, 400)
+ self.assertEqual(
+ channel.json_body,
+ {
+ "errcode": "M_UNKNOWN",
+ "error": "Cannot find any thumbnails for the requested media ([b'example.com', b'12345']). This might mean the media is not a supported_media_format=(image/jpeg, image/jpg, image/webp, image/gif, image/png) or that thumbnailing failed for some other reason. (Dynamic thumbnails are disabled on this server.)",
+ },
+ )
  else:
  # A 404 with a JSON body.
  self.assertEqual(channel.code, 404)