From 3c55462282cfa5db9a1865dfcaad874c39ed6ece Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Mon, 24 Jul 2023 10:13:54 +0100 Subject: [PATCH 01/13] Add support for inling media --- synapse/media/_base.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/synapse/media/_base.py b/synapse/media/_base.py index 20cb8b9010bb..5c34cdbc7410 100644 --- a/synapse/media/_base.py +++ b/synapse/media/_base.py @@ -50,6 +50,16 @@ "text/xml", ] +# A list of all content types that are "safe" to be rendered inline in a browser. +INLINE_CONTENT_TYPES = [ + "text/css", + "text/plain", + "text/csv", + "application/json", + "application/ld+json", + "text/xml", +] + def parse_media_id(request: Request) -> Tuple[str, str, Optional[str]]: """Parses the server name, media ID and optional file name from the request URI @@ -155,7 +165,12 @@ def _quote(x: str) -> str: # Use a Content-Disposition of attachment to force download of media. disposition = "attachment" - if upload_name: + + # We allow a strict subset of content types to be inlined + # so that they may be viewed directly in a browser. + if media_type.lower() in INLINE_CONTENT_TYPES: + disposition = "inline" + elif upload_name: # RFC6266 section 4.1 [1] defines both `filename` and `filename*`. # # `filename` is defined to be a `value`, which is defined by RFC2616 From 7ce8b133274e1b75d4ccb8c4a72e493822676b48 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Mon, 24 Jul 2023 10:27:23 +0100 Subject: [PATCH 02/13] Add test for add file header --- tests/media/test_base.py | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/tests/media/test_base.py b/tests/media/test_base.py index 4728c80969a8..caf909f3a488 100644 --- a/tests/media/test_base.py +++ b/tests/media/test_base.py @@ -12,7 +12,9 @@ # See the License for the specific language governing permissions and # limitations under the License. -from synapse.media._base import get_filename_from_headers +from unittest.mock import Mock + +from synapse.media._base import add_file_headers, get_filename_from_headers from tests import unittest @@ -36,3 +38,18 @@ def tests(self) -> None: expected, f"expected output for {hdr!r} to be {expected} but was {res}", ) + + +class AddFileHeadersTests(unittest.TestCase): + TEST_CASES = { + "text/plain": b"inline", + "text/xml": b"inline", + "text/html": b"attachment; filename=file.name", + "any/thing": b"attachment; filename=file.name", + } + + def tests(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) From ff611f28faa9bd5cd3a7f0533a43aa718d3f0cda Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Mon, 24 Jul 2023 10:30:55 +0100 Subject: [PATCH 03/13] Add a changelog --- changelog.d/15988.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/15988.feature diff --git a/changelog.d/15988.feature b/changelog.d/15988.feature new file mode 100644 index 000000000000..7b09a57ec314 --- /dev/null +++ b/changelog.d/15988.feature @@ -0,0 +1 @@ +Render plain, CSS, CSV, JSON and XML media content in the browser (inline) when requested through the /download endpoint. \ No newline at end of file From f0f546fceb507f65289bc6b8d5360eee9962fb30 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Mon, 24 Jul 2023 11:28:10 +0100 Subject: [PATCH 04/13] Drop XML from the list --- synapse/media/_base.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/synapse/media/_base.py b/synapse/media/_base.py index 5c34cdbc7410..2b0a2e422f8f 100644 --- a/synapse/media/_base.py +++ b/synapse/media/_base.py @@ -56,8 +56,7 @@ "text/plain", "text/csv", "application/json", - "application/ld+json", - "text/xml", + "application/ld+json" ] From 7e41e2b6ef67af2ab79712569f7fa03576d50c0b Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Mon, 24 Jul 2023 11:29:01 +0100 Subject: [PATCH 05/13] Test CSV instead --- tests/media/test_base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/media/test_base.py b/tests/media/test_base.py index caf909f3a488..c283b41ba2b4 100644 --- a/tests/media/test_base.py +++ b/tests/media/test_base.py @@ -43,7 +43,7 @@ def tests(self) -> None: class AddFileHeadersTests(unittest.TestCase): TEST_CASES = { "text/plain": b"inline", - "text/xml": b"inline", + "text/csv": b"inline", "text/html": b"attachment; filename=file.name", "any/thing": b"attachment; filename=file.name", } From 260893cb228df0850a549b21ef41acfc4d48bd13 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Mon, 24 Jul 2023 11:50:59 +0100 Subject: [PATCH 06/13] Support common image formats --- synapse/media/_base.py | 11 ++++++++++- tests/media/test_base.py | 1 + 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/synapse/media/_base.py b/synapse/media/_base.py index 2b0a2e422f8f..489babc329ee 100644 --- a/synapse/media/_base.py +++ b/synapse/media/_base.py @@ -56,7 +56,16 @@ "text/plain", "text/csv", "application/json", - "application/ld+json" + "application/ld+json", + # We allow images listed under the common image list + # https://developer.mozilla.org/en-US/docs/Web/Media/Formats/Image_types#common_image_file_types + # except SVGs + "image/apng", + "image/avif", + "image/gif", + "image/jpeg", + "image/png", + "image/webp", ] diff --git a/tests/media/test_base.py b/tests/media/test_base.py index c283b41ba2b4..b141ae88fdba 100644 --- a/tests/media/test_base.py +++ b/tests/media/test_base.py @@ -44,6 +44,7 @@ class AddFileHeadersTests(unittest.TestCase): TEST_CASES = { "text/plain": b"inline", "text/csv": b"inline", + "text/png": b"inline", "text/html": b"attachment; filename=file.name", "any/thing": b"attachment; filename=file.name", } From 6632045718e722654ad63cd7315a6f293dec79b6 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Mon, 24 Jul 2023 11:53:18 +0100 Subject: [PATCH 07/13] Update feature --- changelog.d/15988.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/15988.feature b/changelog.d/15988.feature index 7b09a57ec314..dee8fa597f56 100644 --- a/changelog.d/15988.feature +++ b/changelog.d/15988.feature @@ -1 +1 @@ -Render plain, CSS, CSV, JSON and XML media content in the browser (inline) when requested through the /download endpoint. \ No newline at end of file +Render plain, CSS, CSV, JSON and common image formats media content in the browser (inline) when requested through the /download endpoint. \ No newline at end of file From 21b8ba0d1a5f719127837f5e6eefa39a1e448f90 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Mon, 24 Jul 2023 18:11:44 +0100 Subject: [PATCH 08/13] Should be image/png --- tests/media/test_base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/media/test_base.py b/tests/media/test_base.py index b141ae88fdba..057ed7dd869e 100644 --- a/tests/media/test_base.py +++ b/tests/media/test_base.py @@ -44,7 +44,7 @@ class AddFileHeadersTests(unittest.TestCase): TEST_CASES = { "text/plain": b"inline", "text/csv": b"inline", - "text/png": b"inline", + "image/png": b"inline", "text/html": b"attachment; filename=file.name", "any/thing": b"attachment; filename=file.name", } From 4e86ab830f60d78fe826906567ca4422c23459d6 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Tue, 8 Aug 2023 18:28:14 +0100 Subject: [PATCH 09/13] More inline --- synapse/media/_base.py | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-) diff --git a/synapse/media/_base.py b/synapse/media/_base.py index 489babc329ee..f714ea6aa6be 100644 --- a/synapse/media/_base.py +++ b/synapse/media/_base.py @@ -57,15 +57,32 @@ "text/csv", "application/json", "application/ld+json", - # We allow images listed under the common image list - # https://developer.mozilla.org/en-US/docs/Web/Media/Formats/Image_types#common_image_file_types - # except SVGs - "image/apng", - "image/avif", - "image/gif", + # We allow some media files deemed as safe, which comes from the matrix-react-sdk. + # https://github.com/matrix-org/matrix-react-sdk/blob/a70fcfd0bcf7f8c85986da18001ea11597989a7c/src/utils/blobs.ts#L51 + # SVGs are *intentionally* omitted. "image/jpeg", + "image/gif", "image/png", + "image/apng", "image/webp", + "image/avif", + + "video/mp4", + "video/webm", + "video/ogg", + "video/quicktime", + + "audio/mp4", + "audio/webm", + "audio/aac", + "audio/mpeg", + "audio/ogg", + "audio/wave", + "audio/wav", + "audio/x-wav", + "audio/x-pn-wav", + "audio/flac", + "audio/x-flac", ] From 32c3fd94395708c104b0de3e4d817aeabcde1267 Mon Sep 17 00:00:00 2001 From: Half-Shot Date: Tue, 8 Aug 2023 18:28:37 +0100 Subject: [PATCH 10/13] Lint --- synapse/media/_base.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/synapse/media/_base.py b/synapse/media/_base.py index f714ea6aa6be..4bc83de2944b 100644 --- a/synapse/media/_base.py +++ b/synapse/media/_base.py @@ -66,12 +66,10 @@ "image/apng", "image/webp", "image/avif", - "video/mp4", "video/webm", "video/ogg", "video/quicktime", - "audio/mp4", "audio/webm", "audio/aac", From dd3af555f7944cc00b604b6a6c157838960c949e Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 22 Sep 2023 13:18:19 -0400 Subject: [PATCH 11/13] Allow filenames for inline content. --- synapse/media/_base.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/synapse/media/_base.py b/synapse/media/_base.py index 4bc83de2944b..80c448de2be2 100644 --- a/synapse/media/_base.py +++ b/synapse/media/_base.py @@ -186,14 +186,14 @@ def _quote(x: str) -> str: request.setHeader(b"Content-Type", content_type.encode("UTF-8")) - # Use a Content-Disposition of attachment to force download of media. - disposition = "attachment" - - # We allow a strict subset of content types to be inlined - # so that they may be viewed directly in a browser. + # 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: disposition = "inline" - elif upload_name: + else: + disposition = "attachment" + + if upload_name: # RFC6266 section 4.1 [1] defines both `filename` and `filename*`. # # `filename` is defined to be a `value`, which is defined by RFC2616 From a7979602559137122fa6be2a627d9dcea369224d Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 22 Sep 2023 13:50:31 -0400 Subject: [PATCH 12/13] Update tests. --- tests/media/test_media_storage.py | 40 ++++++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/tests/media/test_media_storage.py b/tests/media/test_media_storage.py index ea0051dde42f..04fc7bdcef45 100644 --- a/tests/media/test_media_storage.py +++ b/tests/media/test_media_storage.py @@ -129,6 +129,8 @@ class _TestImage: a 404/400 is expected. unable_to_thumbnail: True if we expect the thumbnailing to fail (400), or False if the thumbnailing should succeed or a normal 404 is expected. + is_inline: True if we expect the file to be served using an inline + Content-Disposition or False if we expect an attachment. """ data: bytes @@ -138,6 +140,7 @@ class _TestImage: expected_scaled: Optional[bytes] = None expected_found: bool = True unable_to_thumbnail: bool = False + is_inline: bool = True @parameterized_class( @@ -198,6 +201,25 @@ class _TestImage: unable_to_thumbnail=True, ), ), + # An SVG. + ( + _TestImage( + b""" + + + + +""", + b"image/svg", + b".svg", + expected_found=False, + unable_to_thumbnail=True, + is_inline=False, + ), + ), ], ) class MediaRepoTests(unittest.HomeserverTestCase): @@ -339,7 +361,11 @@ def test_disposition_filename_ascii(self) -> None: ) self.assertEqual( headers.getRawHeaders(b"Content-Disposition"), - [b"attachment; filename=out" + self.test_image.extension], + [ + (b"inline" if self.test_image.is_inline else b"attachment") + + b"; filename=out" + + self.test_image.extension + ], ) def test_disposition_filenamestar_utf8escaped(self) -> None: @@ -359,7 +385,12 @@ def test_disposition_filenamestar_utf8escaped(self) -> None: ) self.assertEqual( headers.getRawHeaders(b"Content-Disposition"), - [b"attachment; filename*=utf-8''" + filename + self.test_image.extension], + [ + (b"inline" if self.test_image.is_inline else b"attachment") + + b"; filename*=utf-8''" + + filename + + self.test_image.extension + ], ) def test_disposition_none(self) -> None: @@ -373,7 +404,10 @@ def test_disposition_none(self) -> None: self.assertEqual( headers.getRawHeaders(b"Content-Type"), [self.test_image.content_type] ) - self.assertEqual(headers.getRawHeaders(b"Content-Disposition"), [b"attachment"]) + self.assertEqual( + headers.getRawHeaders(b"Content-Disposition"), + [b"inline" if self.test_image.is_inline else b"attachment"], + ) def test_thumbnail_crop(self) -> None: """Test that a cropped remote thumbnail is available.""" From 9029506086d49a768175a48f0c96f54150697feb Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 25 Sep 2023 09:58:38 -0400 Subject: [PATCH 13/13] Fix-up tests. --- tests/media/test_base.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/tests/media/test_base.py b/tests/media/test_base.py index 057ed7dd869e..119d7ba66fd6 100644 --- a/tests/media/test_base.py +++ b/tests/media/test_base.py @@ -42,15 +42,24 @@ def tests(self) -> None: class AddFileHeadersTests(unittest.TestCase): TEST_CASES = { - "text/plain": b"inline", - "text/csv": b"inline", - "image/png": b"inline", + "text/plain": b"inline; filename=file.name", + "text/csv": b"inline; filename=file.name", + "image/png": b"inline; filename=file.name", "text/html": b"attachment; filename=file.name", "any/thing": b"attachment; filename=file.name", } - def tests(self) -> None: + 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) + + def test_no_filename(self) -> None: + request = Mock() + add_file_headers(request, "text/plain", 0, None) + request.setHeader.assert_any_call(b"Content-Disposition", b"inline") + + request.reset_mock() + add_file_headers(request, "text/html", 0, None) + request.setHeader.assert_any_call(b"Content-Disposition", b"attachment")