Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Handle missing Content-Type header when accessing remote media #11200

Merged
merged 8 commits into from
Nov 1, 2021
1 change: 1 addition & 0 deletions changelog.d/11200.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixes a long-standing bug wherein a missing Content-Type header when downloading remote media causes Synapse to throw an error.
H-Shay marked this conversation as resolved.
Show resolved Hide resolved
14 changes: 12 additions & 2 deletions synapse/rest/media/v1/media_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,9 @@ async def get_local_media(

self.mark_recently_accessed(None, media_id)

media_type = media_info["media_type"]
media_type = media_info.get("media_type")
squahtx marked this conversation as resolved.
Show resolved Hide resolved
if not media_type:
media_type = "application/octect-stream"
squahtx marked this conversation as resolved.
Show resolved Hide resolved
media_length = media_info["media_length"]
upload_name = name if name else media_info["upload_name"]
url_cache = media_info["url_cache"]
Expand Down Expand Up @@ -333,6 +335,9 @@ async def _get_remote_media_impl(
logger.info("Media is quarantined")
raise NotFoundError()

if not media_info["media_type"]:
media_info["media_type"] = "application/octect-stream"
squahtx marked this conversation as resolved.
Show resolved Hide resolved

responder = await self.media_storage.fetch_media(file_info)
if responder:
return responder, media_info
Expand All @@ -354,6 +359,8 @@ async def _get_remote_media_impl(
raise e

file_id = media_info["filesystem_id"]
if not media_info["media_type"]:
media_info["media_type"] = "application/octect-stream"
squahtx marked this conversation as resolved.
Show resolved Hide resolved
file_info = FileInfo(server_name, file_id)

# We generate thumbnails even if another process downloaded the media
Expand Down Expand Up @@ -445,7 +452,10 @@ async def _download_remote_file(

await finish()

media_type = headers[b"Content-Type"][0].decode("ascii")
if b"Content-Type" in headers:
media_type = headers[b"Content-Type"][0].decode("ascii")
else:
media_type = "application/octet-stream"
upload_name = get_filename_from_headers(headers)
time_now_ms = self.clock.time_msec()

Expand Down
2 changes: 1 addition & 1 deletion synapse/rest/media/v1/upload_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ async def _async_render_POST(self, request: SynapseRequest) -> None:
assert content_type_headers # for mypy
media_type = content_type_headers[0].decode("ascii")
else:
raise SynapseError(msg="Upload request missing 'Content-Type'", code=400)
media_type = "application/octet-stream"

# if headers.hasHeader(b"Content-Disposition"):
# disposition = headers.getRawHeaders(b"Content-Disposition")[0]
Expand Down
24 changes: 19 additions & 5 deletions tests/rest/media/v1/test_media_storage.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ def prepare(self, reactor, clock, hs):

self.media_id = "example.com/12345"

def _req(self, content_disposition):
def _req(self, content_disposition, include_content_type=True):

channel = make_request(
self.reactor,
Expand All @@ -269,10 +269,16 @@ def _req(self, content_disposition):
)
self.assertEqual(self.fetches[0][3], {"allow_remote": "false"})

headers = {
b"Content-Length": [b"%d" % (len(self.test_image.data))],
b"Content-Type": [self.test_image.content_type],
}
if include_content_type:
headers = {
b"Content-Length": [b"%d" % (len(self.test_image.data))],
b"Content-Type": [self.test_image.content_type],
}
else:
headers = {
b"Content-Length": [b"%d" % (len(self.test_image.data))],
}
squahtx marked this conversation as resolved.
Show resolved Hide resolved

if content_disposition:
headers[b"Content-Disposition"] = [content_disposition]

Expand All @@ -285,6 +291,14 @@ def _req(self, content_disposition):

return channel

def test_handle_missing_content_type(self):
channel = self._req(b"inline; filename=out" + self.test_image.extension, False)
squahtx marked this conversation as resolved.
Show resolved Hide resolved
headers = channel.headers
self.assertEqual(channel.code, 200)
self.assertEqual(
headers.getRawHeaders(b"Content-Type"), [b"application/octet-stream"]
)

def test_disposition_filename_ascii(self):
"""
If the filename is filename=<ascii> then Synapse will decode it as an
Expand Down