diff --git a/changelog.d/20368.feature b/changelog.d/20368.feature new file mode 100644 index 0000000000..017f3f7b0b --- /dev/null +++ b/changelog.d/20368.feature @@ -0,0 +1 @@ +Avoid reading media from disk when answering a `HEAD` request to the media download endpoints. diff --git a/synapse/media/_base.py b/synapse/media/_base.py index 7884930876..5f3ebc7796 100644 --- a/synapse/media/_base.py +++ b/synapse/media/_base.py @@ -146,6 +146,11 @@ async def respond_with_file( add_file_headers(request, media_type, file_size, upload_name) + if request.get_method() == "HEAD": + # Twisted does not send a body for HEAD requests. Do not read the media. + finish_request(request) + return + with open(file_path, "rb") as f: await ThreadedFileSender(hs).beginFileTransfer(f, request) @@ -375,6 +380,11 @@ async def respond_with_multipart_responder( b"multipart/mixed; boundary=%s" % multipart_consumer.boundary, ) + if request.get_method() == "HEAD": + # Twisted does not send a body for HEAD requests. Do not read the media. + finish_request(request) + return + try: await responder.write_to_consumer(multipart_consumer) except ConsumerRequestedStopError as e: @@ -422,6 +432,12 @@ async def respond_with_responder( logger.debug("Responding to media request with responder %s", responder) add_file_headers(request, media_type, file_size, upload_name) + + if request.get_method() == "HEAD": + # Twisted does not send a body for HEAD requests. Do not read the media. + finish_request(request) + return + try: await responder.write_to_consumer(request) except ConsumerRequestedStopError as e: diff --git a/tests/federation/test_federation_media.py b/tests/federation/test_federation_media.py index 9c4e813537..666b9fed3a 100644 --- a/tests/federation/test_federation_media.py +++ b/tests/federation/test_federation_media.py @@ -21,6 +21,7 @@ import io import os import shutil import tempfile +from unittest.mock import patch from twisted.internet.testing import MemoryReactor @@ -148,6 +149,34 @@ class FederationMediaDownloadsTest(unittest.FederatingHomeserverTestCase): found_file = any(SMALL_PNG in field for field in stripped_bytes) self.assertTrue(found_file) + def test_head_file_download_does_not_read_file(self) -> None: + content_uri = self.get_success( + self.media_repo.create_or_update_content( + "image/png", + "test_png_upload", + io.BytesIO(SMALL_PNG), + 67, + UserID.from_string("@user_id:whatever.org"), + ) + ) + + with patch( + "synapse.media._base.ThreadedFileSender.beginFileTransfer" + ) as begin_file_transfer: + channel = self.make_signed_federation_request( + "HEAD", + f"/_matrix/federation/v1/media/download/{content_uri.media_id}", + ) + self.pump() + + self.assertEqual(200, channel.code) + content_type = channel.headers.getRawHeaders("content-type") + assert content_type is not None + self.assertIn("multipart/mixed", content_type[0]) + self.assertEqual(channel.is_finished(), True) + self.assertEqual(channel.result.get("body", b""), b"") + begin_file_transfer.assert_not_called() + def test_federation_etag(self) -> None: """Test that federation ETags work""" diff --git a/tests/rest/client/test_media.py b/tests/rest/client/test_media.py index 31d8f3de35..d8d87b7c69 100644 --- a/tests/rest/client/test_media.py +++ b/tests/rest/client/test_media.py @@ -2652,6 +2652,37 @@ class AuthenticatedMediaTestCase(unittest.HomeserverTestCase): resources["/_matrix/media"] = self.hs.get_media_repository_resource() return resources + def test_head_download_does_not_read_file(self) -> None: + channel = self.make_request( + "POST", + "_matrix/media/v3/upload?filename=test_png_upload", + SMALL_PNG, + self.tok, + shorthand=False, + content_type=b"image/png", + ) + self.assertEqual(channel.code, 200) + uri = channel.json_body["content_uri"].split("mxc://")[1] + + with patch( + "synapse.media._base.ThreadedFileSender.beginFileTransfer" + ) as begin_file_transfer: + channel = self.make_request( + "HEAD", + f"_matrix/client/v1/media/download/{uri}", + access_token=self.tok, + shorthand=False, + ) + + self.assertEqual(channel.code, 200) + self.assertEqual( + channel.headers.getRawHeaders(b"Content-Length"), + [str(len(SMALL_PNG)).encode()], + ) + self.assertEqual(channel.headers.getRawHeaders(b"Content-Type"), [b"image/png"]) + self.assertEqual(channel.result.get("body", b""), b"") + begin_file_transfer.assert_not_called() + def test_authenticated_media(self) -> None: # upload some local media with authentication on channel = self.make_request(