diff --git a/changelog.d/16440.bugfix b/changelog.d/16440.bugfix new file mode 100644 index 0000000000..6ce0b1e4af --- /dev/null +++ b/changelog.d/16440.bugfix @@ -0,0 +1 @@ +Properly return inline media when content types have parameters. diff --git a/synapse/media/_base.py b/synapse/media/_base.py index 13345acf75..860e5ddca2 100644 --- a/synapse/media/_base.py +++ b/synapse/media/_base.py @@ -148,7 +148,9 @@ def add_file_headers( # A strict subset of content types is allowed to be inlined so that they may # be viewed directly in a browser. Other file types are forced to be downloads. - if media_type.lower() in INLINE_CONTENT_TYPES: + # + # Only the type & subtype are important, parameters can be ignored. + if media_type.lower().split(";", 1)[0] in INLINE_CONTENT_TYPES: disposition = "inline" else: disposition = "attachment" diff --git a/tests/media/test_base.py b/tests/media/test_base.py index 119d7ba66f..144948f23c 100644 --- a/tests/media/test_base.py +++ b/tests/media/test_base.py @@ -42,18 +42,35 @@ class GetFileNameFromHeadersTests(unittest.TestCase): class AddFileHeadersTests(unittest.TestCase): TEST_CASES = { + # Safe values use inline. "text/plain": b"inline; filename=file.name", "text/csv": b"inline; filename=file.name", "image/png": b"inline; filename=file.name", + # Unlisted values are set to attachment. "text/html": b"attachment; filename=file.name", "any/thing": b"attachment; filename=file.name", + # Parameters get ignored. + "text/plain; charset=utf-8": b"inline; filename=file.name", + "text/markdown; charset=utf-8; variant=CommonMark": b"attachment; filename=file.name", + # Parsed as lowercase. + "Text/Plain": b"inline; filename=file.name", + # Bad values don't choke. + "": b"attachment; filename=file.name", + ";": b"attachment; filename=file.name", } def test_content_disposition(self) -> None: for media_type, expected in self.TEST_CASES.items(): request = Mock() add_file_headers(request, media_type, 0, "file.name") - request.setHeader.assert_any_call(b"Content-Disposition", expected) + # There should be a single call to set Content-Disposition. + for call in request.setHeader.call_args_list: + args, _ = call + if args[0] == b"Content-Disposition": + break + else: + self.fail(f"No Content-Disposition header found for {media_type}") + self.assertEqual(args[1], expected, media_type) def test_no_filename(self) -> None: request = Mock()