From 73d93039ff6c3addd54bb29a57808a3f2eed7a05 Mon Sep 17 00:00:00 2001
From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
Date: Fri, 2 Oct 2020 12:29:29 +0100
Subject: [PATCH] Fix bug in remote thumbnail search (#8438)

#7124 changed the behaviour of remote thumbnails so that the thumbnailing method was included in the filename of the thumbnail. To support existing files, it included a fallback so that we would check the old filename if the new filename didn't exist.

Unfortunately, it didn't apply this logic to storage providers, so any thumbnails stored on such a storage provider was broken.
---
 changelog.d/8438.bugfix                |  1 +
 synapse/rest/media/v1/media_storage.py | 43 ++++++++++++++------------
 2 files changed, 24 insertions(+), 20 deletions(-)
 create mode 100644 changelog.d/8438.bugfix

diff --git a/changelog.d/8438.bugfix b/changelog.d/8438.bugfix
new file mode 100644
index 000000000..3edc39414
--- /dev/null
+++ b/changelog.d/8438.bugfix
@@ -0,0 +1 @@
+Fix a regression in v1.21.0rc1 which broke thumbnails of remote media.
diff --git a/synapse/rest/media/v1/media_storage.py b/synapse/rest/media/v1/media_storage.py
index 5681677fc..a9586fb0b 100644
--- a/synapse/rest/media/v1/media_storage.py
+++ b/synapse/rest/media/v1/media_storage.py
@@ -141,31 +141,34 @@ class MediaStorage:
         Returns:
             Returns a Responder if the file was found, otherwise None.
         """
+        paths = [self._file_info_to_path(file_info)]
 
-        path = self._file_info_to_path(file_info)
-        local_path = os.path.join(self.local_media_directory, path)
-        if os.path.exists(local_path):
-            return FileResponder(open(local_path, "rb"))
-
-        # Fallback for paths without method names
-        # Should be removed in the future
+        # fallback for remote thumbnails with no method in the filename
         if file_info.thumbnail and file_info.server_name:
-            legacy_path = self.filepaths.remote_media_thumbnail_rel_legacy(
-                server_name=file_info.server_name,
-                file_id=file_info.file_id,
-                width=file_info.thumbnail_width,
-                height=file_info.thumbnail_height,
-                content_type=file_info.thumbnail_type,
+            paths.append(
+                self.filepaths.remote_media_thumbnail_rel_legacy(
+                    server_name=file_info.server_name,
+                    file_id=file_info.file_id,
+                    width=file_info.thumbnail_width,
+                    height=file_info.thumbnail_height,
+                    content_type=file_info.thumbnail_type,
+                )
             )
-            legacy_local_path = os.path.join(self.local_media_directory, legacy_path)
-            if os.path.exists(legacy_local_path):
-                return FileResponder(open(legacy_local_path, "rb"))
+
+        for path in paths:
+            local_path = os.path.join(self.local_media_directory, path)
+            if os.path.exists(local_path):
+                logger.debug("responding with local file %s", local_path)
+                return FileResponder(open(local_path, "rb"))
+            logger.debug("local file %s did not exist", local_path)
 
         for provider in self.storage_providers:
-            res = await provider.fetch(path, file_info)  # type: Any
-            if res:
-                logger.debug("Streaming %s from %s", path, provider)
-                return res
+            for path in paths:
+                res = await provider.fetch(path, file_info)  # type: Any
+                if res:
+                    logger.debug("Streaming %s from %s", path, provider)
+                    return res
+                logger.debug("%s not found on %s", path, provider)
 
         return None