Distinguish b/w nonexistent and unreachable media (#49)

Fixes #46.

I am assuming that after media retention kicks in, the remote homeserver
responds to media requests with 404 M_NOT_FOUND. If so, the content
scanner should do the same.

Mypy should pass once #53 lands.

Author: David Robertson

docs/api.md

@@ -28,6 +28,7 @@ status code of the response for each scenario:
 | 400         | `MCS_MALFORMED_JSON`          | The request body contains malformed JSON.                                                                                                                                         |
 | 400         | `MCS_MEDIA_FAILED_TO_DECRYPT` | The server failed to decrypt the encrypted media downloaded from the media repo.                                                                                                  |
 | 404         | `M_NOT_FOUND`                 | No route could be found at the given path.                                                                                                                                        |
+| 404         | `M_NOT_FOUND`                 | The requested media was not present in the media repo.                                                                                                                            |
 | 403         | `MCS_MEDIA_NOT_CLEAN`         | The server scanned the downloaded media but the antivirus script returned a non-zero exit code.                                                                                   |
 | 403         | `MCS_BAD_DECRYPTION`          | The provided `encrypted_body` could not be decrypted, or the encrypted file could not be decrypted. The client should request the public key of the server and then retry (once). |
 | 500         | `M_UNKNOWN`                   | The server experienced an unexpected error.                                                                                                                                       |

src/matrix_content_scanner/scanner/file_downloader.py

@@ -89,8 +89,8 @@ async def download_file(
             except _PathNotFoundException:
                 # If that still failed, raise an error.
                 raise ContentScannerRestError(
-                    http_status=HTTPStatus.BAD_GATEWAY,
-                    reason=ErrCode.REQUEST_FAILED,
+                    http_status=HTTPStatus.NOT_FOUND,
+                    reason=ErrCode.NOT_FOUND,
                     info="File not found",
                 )
 

src/matrix_content_scanner/utils/constants.py

@@ -17,11 +17,14 @@
 class ErrCode(str, Enum):
     # An unknown error happened.
     UNKNOWN = "M_UNKNOWN"
-    # No route was found with the path and method provided in the request.
+    # One of the following:
+    # - No route was found with the path and method provided in the request.
+    # - The homeserver does not have the requested piece of media.
     NOT_FOUND = "M_NOT_FOUND"
     # The file failed the scan.
     NOT_CLEAN = "MCS_MEDIA_NOT_CLEAN"
     # The file could not be retrieved from the homeserver.
+    # Does NOT cover homeserver responses with M_NOT_FOUND.
     REQUEST_FAILED = "MCS_MEDIA_REQUEST_FAILED"
     # The encrypted file could not be decrypted with the provided metadata.
     FAILED_TO_DECRYPT = "MCS_MEDIA_FAILED_TO_DECRYPT"

tests/scanner/test_file_downloader.py

@@ -124,7 +124,7 @@ async def _test_retry(self) -> None:
         with self.assertRaises(ContentScannerRestError) as cm:
             await self.downloader.download_file(MEDIA_PATH)
 
-        self.assertEqual(cm.exception.http_status, 502)
+        self.assertEqual(cm.exception.http_status, 404)
         self.assertEqual(cm.exception.info, "File not found")
 
         # Check that we sent out two requests: one to the v3 path and one to the r0 path.

tests/servlets/test_scan.py

@@ -0,0 +1,73 @@
+#  Copyright 2023 New Vector Ltd
+#
+#  Licensed under the Apache License, Version 2.0 (the "License");
+#  you may not use this file except in compliance with the License.
+#  You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing, software
+#  distributed under the License is distributed on an "AS IS" BASIS,
+#  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+#  See the License for the specific language governing permissions and
+#  limitations under the License.
+from http import HTTPStatus
+from unittest.mock import patch
+
+from aiohttp.test_utils import AioHTTPTestCase
+from aiohttp.web_app import Application
+from multidict import CIMultiDict
+
+from matrix_content_scanner.httpserver import HTTPServer
+from matrix_content_scanner.utils.constants import ErrCode
+from matrix_content_scanner.utils.errors import ContentScannerRestError
+from tests.testutils import get_content_scanner
+
+SERVER_NAME = "test"
+
+
+class TestScanHandler(AioHTTPTestCase):
+    def setUp(self) -> None:
+        # Bypass well-known lookups.
+        self.scanner = get_content_scanner(
+            {"download": {"base_homeserver_url": "http://my-site.com"}}
+        )
+
+    async def get_application(self) -> Application:
+        return HTTPServer(self.scanner)._app
+
+    async def test_media_not_found_on_remote_homeserver(self) -> None:
+        """Missing media on the remote HS should be presented as a 404 to the client."""
+        patch_downloader = patch.object(
+            self.scanner.file_downloader,
+            "_get",
+            return_value=(HTTPStatus.NOT_FOUND, b"", CIMultiDict()),
+        )
+
+        with patch_downloader:
+            async with self.client.get(
+                f"/_matrix/media_proxy/unstable/download/{SERVER_NAME}/media-does-not-exist"
+            ) as resp:
+                self.assertEqual(resp.status, 404)
+                body = await resp.json()
+                self.assertEqual(body["reason"], "M_NOT_FOUND", body)
+
+    async def test_remote_homeserver_unreachable(self) -> None:
+        """An unreachable HS should be presented as a 502 to the client."""
+        patch_downloader = patch.object(
+            self.scanner.file_downloader,
+            "_get",
+            side_effect=ContentScannerRestError(
+                HTTPStatus.BAD_GATEWAY,
+                ErrCode.REQUEST_FAILED,
+                "dodgy network timeout :(((",
+            ),
+        )
+
+        with patch_downloader:
+            async with self.client.get(
+                f"/_matrix/media_proxy/unstable/download/{SERVER_NAME}/media-does-not-exist"
+            ) as resp:
+                self.assertEqual(resp.status, 502)
+                body = await resp.json()
+                self.assertEqual(body["reason"], "MCS_MEDIA_REQUEST_FAILED", body)