From d7aede22d1b1e33ebc48e17bb133886606f33dfa Mon Sep 17 00:00:00 2001 From: Maurits van Rees Date: Fri, 27 Oct 2023 02:32:15 +0200 Subject: [PATCH 1/3] Be more strict when checking if mimetype is allowed to be displayed inline. This takes over some code from https://github.com/zopefoundation/Zope/pull/1167. See also https://github.com/plone/plone.namedfile/pull/154 --- news/1167.bugfix | 2 ++ src/plone/restapi/services/users/get.py | 26 ++++++++++++++++++++++--- 2 files changed, 25 insertions(+), 3 deletions(-) create mode 100644 news/1167.bugfix diff --git a/news/1167.bugfix b/news/1167.bugfix new file mode 100644 index 0000000000..3d8ef86055 --- /dev/null +++ b/news/1167.bugfix @@ -0,0 +1,2 @@ +Be more strict when checking if mimetype is allowed to be displayed inline. +[maurits] diff --git a/src/plone/restapi/services/users/get.py b/src/plone/restapi/services/users/get.py index bef0c3ef0e..61ae3718a4 100644 --- a/src/plone/restapi/services/users/get.py +++ b/src/plone/restapi/services/users/get.py @@ -27,6 +27,28 @@ DEFAULT_SEARCH_RESULTS_LIMIT = 25 +try: + from OFS.Image import extract_media_type +except ImportError: + try: + from plone.namedfile.utils import extract_media_type + except ImportError: + + def extract_media_type(content_type): + """extract the proper media type from *content_type*. + + Ignore parameters and whitespace and normalize to lower case. + See https://github.com/zopefoundation/Zope/pull/1167 + """ + if not content_type: + return content_type + # ignore parameters + content_type = content_type.split(";", 1)[0] + # ignore whitespace + content_type = "".join(content_type.split()) + # normalize to lowercase + return content_type.lower() + def getPortraitUrl(user): if not user: @@ -84,7 +106,6 @@ def _sort_users(users: Iterable[MemberData]) -> Sequence[MemberData]: def _principal_search_results( self, search_for_principal, get_principal_by_id, principal_type, id_key ): - hunter = getMultiAdapter((self.context, self.request), name="pas_search") principals = [] @@ -209,7 +230,6 @@ def reply(self): if self.has_permission_to_access_user_info() or ( current_user_id and current_user_id == self._get_user_id ): - # we retrieve the user on the user id not the username user = self._get_user(self._get_user_id) if not user: @@ -251,7 +271,7 @@ def _get_user_id(self): def _should_force_download(self, portrait): # If this returns True, the caller should set the Content-Disposition header. - mimetype = portrait.content_type + mimetype = extract_media_type(portrait.content_type) if not mimetype: return False if self.use_denylist: From bfc79a704c6cf2778e1dc114d3041f8c8da8c87d Mon Sep 17 00:00:00 2001 From: Maurits van Rees Date: Fri, 27 Oct 2023 12:40:17 +0200 Subject: [PATCH 2/3] Copy a test for extract_media_type from Zope. --- src/plone/restapi/tests/test_services_users.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/plone/restapi/tests/test_services_users.py b/src/plone/restapi/tests/test_services_users.py index 6de014a8cb..cebe00d696 100644 --- a/src/plone/restapi/tests/test_services_users.py +++ b/src/plone/restapi/tests/test_services_users.py @@ -23,6 +23,17 @@ import unittest +class TestUnit(unittest.TestCase): + def test_extract_media_type(self): + from plone.restapi.services.users.get import extract_media_type as extract + + self.assertIsNone(extract(None)) + self.assertEqual(extract("text/plain"), "text/plain") + self.assertEqual(extract("TEXT/PLAIN"), "text/plain") + self.assertEqual(extract("text / plain"), "text/plain") + self.assertEqual(extract(" text/plain ; charset=utf-8"), "text/plain") + + class TestUsersEndpoint(unittest.TestCase): layer = PLONE_RESTAPI_DX_FUNCTIONAL_TESTING From 43890f050d41c95e5aded918197c33e3242debe2 Mon Sep 17 00:00:00 2001 From: Maurits van Rees Date: Fri, 27 Oct 2023 12:43:03 +0200 Subject: [PATCH 3/3] Rename extract_media_type to _extract_media_type to signal it as private. --- src/plone/restapi/services/users/get.py | 11 +++++++---- src/plone/restapi/tests/test_services_users.py | 2 +- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/plone/restapi/services/users/get.py b/src/plone/restapi/services/users/get.py index 61ae3718a4..393237f740 100644 --- a/src/plone/restapi/services/users/get.py +++ b/src/plone/restapi/services/users/get.py @@ -28,13 +28,16 @@ DEFAULT_SEARCH_RESULTS_LIMIT = 25 try: - from OFS.Image import extract_media_type + # Zope 5.8.4+ + from OFS.Image import extract_media_type as _extract_media_type except ImportError: try: - from plone.namedfile.utils import extract_media_type + from plone.namedfile.utils import extract_media_type as _extract_media_type except ImportError: + # Note that we start the method with an underscore, to signal that this + # is a private implementation detail and no one should be importing this. - def extract_media_type(content_type): + def _extract_media_type(content_type): """extract the proper media type from *content_type*. Ignore parameters and whitespace and normalize to lower case. @@ -271,7 +274,7 @@ def _get_user_id(self): def _should_force_download(self, portrait): # If this returns True, the caller should set the Content-Disposition header. - mimetype = extract_media_type(portrait.content_type) + mimetype = _extract_media_type(portrait.content_type) if not mimetype: return False if self.use_denylist: diff --git a/src/plone/restapi/tests/test_services_users.py b/src/plone/restapi/tests/test_services_users.py index cebe00d696..59cdd97d4e 100644 --- a/src/plone/restapi/tests/test_services_users.py +++ b/src/plone/restapi/tests/test_services_users.py @@ -25,7 +25,7 @@ class TestUnit(unittest.TestCase): def test_extract_media_type(self): - from plone.restapi.services.users.get import extract_media_type as extract + from plone.restapi.services.users.get import _extract_media_type as extract self.assertIsNone(extract(None)) self.assertEqual(extract("text/plain"), "text/plain")