mirror of
https://github.com/element-hq/synapse.git
synced 2026-11-04 11:17:51 +00:00
Do not read media from disk when answering a HEAD request (#20368)
Fixes #20367 HEAD requests are routed to the GET handlers, so the media download endpoints read the whole file and stream it to Twisted, which then discards the body. Stop after writing the headers instead. ### Pull Request Checklist <!-- Please read https://element-hq.github.io/synapse/latest/development/contributing_guide.html before submitting your pull request --> * [x] Pull request is based on the develop branch * [x] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). The entry should: - Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from `EventStore` to `EventWorkerStore`.". - Use markdown where necessary, mostly for `code blocks`. - End with either a period (.) or an exclamation mark (!). - Start with a capital letter. - Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry. * [x] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters)) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
ab8f060746
commit
dec679de1c
4 files changed
+77
No files matched your search
@@ -0,0 +1 @@
|
||||
Avoid reading media from disk when answering a `HEAD` request to the media download endpoints.
|
||||
@@ -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:
|
||||
|
||||
@@ -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"""
|
||||
|
||||
|
||||
@@ -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(
|
||||
|
||||
Reference in new issue
Block a user