From d4558fdaaaa68d7372a88ba145e2afe25ae701fa Mon Sep 17 00:00:00 2001 From: mauritsvanrees Date: Thu, 21 Sep 2023 13:33:35 +0200 Subject: [PATCH] [fc] Repository: plone.restapi Branch: refs/heads/main Date: 2023-09-21T13:33:35+02:00 Author: Maurits van Rees (mauritsvanrees) Commit: https://github.com/plone/plone.restapi/commit/5f44c23ac69db7d6d933d77f177e07603cf05f8b Merge pull request from GHSA-hc5c-r8m5-2gfh Done by forcing a download instead of displaying inline. Normal accessing via an image tag is not affected and is safe. Files changed: A news/1.bugfix A src/plone/restapi/tests/image.svg M src/plone/restapi/services/users/get.py M src/plone/restapi/tests/test_services_users.py --- last_commit.txt | 41 +++++++++++------------------------------ 1 file changed, 11 insertions(+), 30 deletions(-) diff --git a/last_commit.txt b/last_commit.txt index 1715aa18bf..3b43fbca73 100644 --- a/last_commit.txt +++ b/last_commit.txt @@ -1,40 +1,21 @@ -Repository: plone.namedfile +Repository: plone.restapi -Branch: refs/heads/master -Date: 2023-09-18T14:04:36+02:00 +Branch: refs/heads/main +Date: 2023-09-21T13:33:35+02:00 Author: Maurits van Rees (mauritsvanrees) -Commit: https://github.com/plone/plone.namedfile/commit/07a890a857ff14c6077fa5eabab42bdf99061162 +Commit: https://github.com/plone/plone.restapi/commit/5f44c23ac69db7d6d933d77f177e07603cf05f8b -Force download of image scales when inline is not allowed for this mimetype. +Merge pull request from GHSA-hc5c-r8m5-2gfh -We already did this for the `@@display-file` view in 2021, but now it is done for image scales as well. - -See [security advisory](https://github.com/plone/plone.namedfile/security/advisories/GHSA-jj7c-jrv4-c65x). - -Files changed: -A news/1.bugfix -M plone/namedfile/scaling.py -M plone/namedfile/tests/test_display_file.py - -b'diff --git a/news/1.bugfix b/news/1.bugfix\nnew file mode 100644\nindex 0000000..c6cf8bb\n--- /dev/null\n+++ b/news/1.bugfix\n@@ -0,0 +1,4 @@\n+Fix stored XSS (Cross Site Scripting) for SVG images.\n+Done by forcing a download instead of displaying inline.\n+See `security advisory `_.\n+[maurits]\ndiff --git a/plone/namedfile/scaling.py b/plone/namedfile/scaling.py\nindex ffa064d..47503dd 100644\n--- a/plone/namedfile/scaling.py\n+++ b/plone/namedfile/scaling.py\n@@ -4,6 +4,9 @@\n from io import BytesIO\n from plone.memoize import ram\n from plone.namedfile.file import FILECHUNK_CLASSES\n+from plone.namedfile.browser import ALLOWED_INLINE_MIMETYPES\n+from plone.namedfile.browser import DISALLOWED_INLINE_MIMETYPES\n+from plone.namedfile.browser import USE_DENYLIST\n from plone.namedfile.interfaces import IAvailableSizes\n from plone.namedfile.interfaces import IStableImageScale\n from plone.namedfile.picture import get_picture_variants\n@@ -75,6 +78,14 @@ class ImageScale(BrowserView):\n __allow_access_to_unprotected_subobjects__ = 1\n data = None\n \n+ # You can control which mimetypes may be shown inline\n+ # and which must always be downloaded, for security reasons.\n+ # Make the configuration available on the class.\n+ # Then subclasses can override this.\n+ allowed_inline_mimetypes = ALLOWED_INLINE_MIMETYPES\n+ disallowed_inline_mimetypes = DISALLOWED_INLINE_MIMETYPES\n+ use_denylist = USE_DENYLIST\n+\n def __init__(self, context, request, **info):\n self.context = context\n self.request = request\n@@ -167,10 +178,37 @@ def validate_access(self):\n fieldname = getattr(self.data, "fieldname", getattr(self, "fieldname", None))\n guarded_getattr(self.context, fieldname)\n \n+ def _should_force_download(self):\n+ # If this returns True, the caller should call set_headers with a filename.\n+ if not hasattr(self.data, "contentType"):\n+ return\n+ mimetype = self.data.contentType\n+ if self.use_denylist:\n+ # We explicitly deny a few mimetypes, and allow the rest.\n+ return mimetype in self.disallowed_inline_mimetypes\n+ # Use the allowlist.\n+ # We only explicitly allow a few mimetypes, and deny the rest.\n+ return mimetype not in self.allowed_inline_mimetypes\n+\n+ def set_headers(self, response=None):\n+ # set headers for the image\n+ image = self.data\n+ if response is None:\n+ response = self.request.response\n+ filename = None\n+ if self._should_force_download():\n+ # We MUST pass a filename, even a dummy one if needed.\n+ filename = getattr(image, "filename", getattr(self, "filename", None))\n+ if filename is None:\n+ filename = getattr(image, "fieldname", getattr(self, "fieldname", None))\n+ if filename is None:\n+ filename = "image.ext"\n+ set_headers(image, response, filename=filename)\n+\n def index_html(self):\n """download the image"""\n self.validate_access()\n- set_headers(self.data, self.request.response)\n+ self.set_headers()\n return stream_data(self.data)\n \n def manage_DAVget(self):\n@@ -190,7 +228,7 @@ def HEAD(self, REQUEST, RESPONSE=None):\n without transfer of the image itself\n """\n self.validate_access()\n- set_headers(self.data, REQUEST.response)\n+ self.set_headers(response=REQUEST.response)\n return ""\n \n HEAD.__roles__ = ("Anonymous",)\ndiff --git a/plone/namedfile/tests/test_display_file.py b/plone/namedfile/tests/test_display_file.py\nindex 86251a8..4ab467d 100644\n--- a/plone/namedfile/tests/test_display_file.py\n+++ b/plone/namedfile/tests/test_display_file.py\n@@ -3,12 +3,15 @@\n from plone.app.testing import SITE_OWNER_PASSWORD\n from plone.namedfile import field\n from plone.namedfile import file\n+from plone.namedfile.interfaces import IAvailableSizes\n from plone.namedfile.interfaces import IImageScaleTraversable\n from plone.namedfile.testing import PLONE_NAMEDFILE_FUNCTIONAL_TESTING\n from plone.namedfile.tests import getFile\n from plone.testing.zope import Browser\n from Products.CMFPlone.utils import safe_unicode\n+from zope.annotation import IAnnotations\n from zope.annotation import IAttributeAnnotatable\n+from zope.component import getSiteManager\n from zope.interface import implementer\n \n import transaction\n@@ -46,6 +49,11 @@ def get_disposition_header(browser):\n return browser.headers.get(name, None)\n \n \n+def custom_available_sizes():\n+ # Define available image scales.\n+ return {"custom": (10, 10)}\n+\n+\n class TestAttackVectorNamedImage(unittest.TestCase):\n layer = PLONE_NAMEDFILE_FUNCTIONAL_TESTING\n field_class = file.NamedImage\n@@ -56,6 +64,12 @@ def setUp(self):\n item = DummyContent()\n self.layer["app"]._setOb("item", item)\n self.item = self.layer["app"].item\n+ sm = getSiteManager()\n+ sm.registerUtility(component=custom_available_sizes, provided=IAvailableSizes)\n+\n+ def tearDown(self):\n+ sm = getSiteManager()\n+ sm.unregisterUtility(provided=IAvailableSizes)\n \n def get_admin_browser(self):\n browser = Browser(self.layer["app"])\n@@ -98,12 +112,50 @@ def assert_display_inline_is_download(self, base_url):\n self.assertIn("attachment", header)\n self.assertIn("filename", header)\n \n+ def assert_scale_view_works(self, base_url):\n+ # Test that accessing a scale view shows the image inline.\n+ browser = self.get_anon_browser()\n+ browser.open(base_url + "/@@images/{}".format(self.field_name))\n+ self.assertIsNone(get_disposition_header(browser))\n+\n+ # Note: the \'custom\' scale is defined in an adapter above.\n+ browser.open(base_url + "/@@images/{}/custom".format(self.field_name))\n+ self.assertIsNone(get_disposition_header(browser))\n+\n+ unique_scale_id = list(IAnnotations(self.item)["plone.scale"].keys())[0]\n+ browser.open(base_url + "/@@images/{}".format(unique_scale_id))\n+ self.assertIsNone(get_disposition_header(browser))\n+\n+ def assert_scale_view_is_download(self, base_url):\n+ # Test that accessing a scale view turns into a download.\n+ browser = self.get_anon_browser()\n+ browser.open(base_url + "/@@images/{}".format(self.field_name))\n+ header = get_disposition_header(browser)\n+ self.assertIsNotNone(header)\n+ self.assertIn("attachment", header)\n+ self.assertIn("filename", header)\n+\n+ browser.open(base_url + "/@@images/{}/custom".format(self.field_name))\n+ header = get_disposition_header(browser)\n+ self.assertIsNotNone(header)\n+ self.assertIn("attachment", header)\n+ self.assertIn("filename", header)\n+\n+ unique_scale_id = list(IAnnotations(self.item)["plone.scale"].keys())[0]\n+ browser.open(base_url + "/@@images/{}".format(unique_scale_id))\n+ header = get_disposition_header(browser)\n+ self.assertIsNotNone(header)\n+ self.assertIn("attachment", header)\n+ self.assertIn("filename", header)\n+\n def test_png_image(self):\n setattr(self.item, self.field_name, self._named_file("image.png"))\n transaction.commit()\n base_url = self.item.absolute_url()\n self.assert_download_works(base_url)\n self.assert_display_inline_works(base_url)\n+ if self.field_name == "image":\n+ self.assert_scale_view_works(base_url)\n \n def test_svg_image(self):\n setattr(self.item, self.field_name, self._named_file("image.svg"))\n@@ -111,10 +163,13 @@ def test_svg_image(self):\n base_url = self.item.absolute_url()\n self.assert_download_works(base_url)\n self.assert_display_inline_is_download(base_url)\n+ if self.field_name == "image":\n+ self.assert_scale_view_is_download(base_url)\n \n def test_filename_none(self):\n- # A \'None\' filename None probably does not happen during normal upload,\n- # but if an attacker manages this, even @@download will show inline.\n+ # A \'None\' filename probably does not happen during normal upload,\n+ # but if an attacker manages this, even @@download would show inline.\n+ # We prevent this.\n data = self._named_file("image.svg")\n data.filename = None\n setattr(self.item, self.field_name, data)\n@@ -122,6 +177,8 @@ def test_filename_none(self):\n base_url = self.item.absolute_url()\n self.assert_download_works(base_url)\n self.assert_display_inline_is_download(base_url)\n+ if self.field_name == "image":\n+ self.assert_scale_view_is_download(base_url)\n \n def test_filename_empty(self):\n # An empty filename is probably no problem, but let\'s check.\n@@ -132,6 +189,8 @@ def test_filename_empty(self):\n base_url = self.item.absolute_url()\n self.assert_download_works(base_url)\n self.assert_display_inline_is_download(base_url)\n+ if self.field_name == "image":\n+ self.assert_scale_view_is_download(base_url)\n \n \n class TestAttackVectorNamedBlobImage(TestAttackVectorNamedImage):\n' - -Repository: plone.namedfile - - -Branch: refs/heads/master -Date: 2023-09-21T13:22:31+02:00 -Author: Maurits van Rees (mauritsvanrees) -Commit: https://github.com/plone/plone.namedfile/commit/f0f911f2a72b2e5c923dc2ab9179319cc47788f9 - -Merge pull request from GHSA-jj7c-jrv4-c65x - -Force download of image scales when inline is not allowed for mime type [master] +Done by forcing a download instead of displaying inline. +Normal accessing via an image tag is not affected and is safe. Files changed: A news/1.bugfix -M plone/namedfile/scaling.py -M plone/namedfile/tests/test_display_file.py +A src/plone/restapi/tests/image.svg +M src/plone/restapi/services/users/get.py +M src/plone/restapi/tests/test_services_users.py -b'diff --git a/news/1.bugfix b/news/1.bugfix\nnew file mode 100644\nindex 0000000..c6cf8bb\n--- /dev/null\n+++ b/news/1.bugfix\n@@ -0,0 +1,4 @@\n+Fix stored XSS (Cross Site Scripting) for SVG images.\n+Done by forcing a download instead of displaying inline.\n+See `security advisory `_.\n+[maurits]\ndiff --git a/plone/namedfile/scaling.py b/plone/namedfile/scaling.py\nindex ffa064d..47503dd 100644\n--- a/plone/namedfile/scaling.py\n+++ b/plone/namedfile/scaling.py\n@@ -4,6 +4,9 @@\n from io import BytesIO\n from plone.memoize import ram\n from plone.namedfile.file import FILECHUNK_CLASSES\n+from plone.namedfile.browser import ALLOWED_INLINE_MIMETYPES\n+from plone.namedfile.browser import DISALLOWED_INLINE_MIMETYPES\n+from plone.namedfile.browser import USE_DENYLIST\n from plone.namedfile.interfaces import IAvailableSizes\n from plone.namedfile.interfaces import IStableImageScale\n from plone.namedfile.picture import get_picture_variants\n@@ -75,6 +78,14 @@ class ImageScale(BrowserView):\n __allow_access_to_unprotected_subobjects__ = 1\n data = None\n \n+ # You can control which mimetypes may be shown inline\n+ # and which must always be downloaded, for security reasons.\n+ # Make the configuration available on the class.\n+ # Then subclasses can override this.\n+ allowed_inline_mimetypes = ALLOWED_INLINE_MIMETYPES\n+ disallowed_inline_mimetypes = DISALLOWED_INLINE_MIMETYPES\n+ use_denylist = USE_DENYLIST\n+\n def __init__(self, context, request, **info):\n self.context = context\n self.request = request\n@@ -167,10 +178,37 @@ def validate_access(self):\n fieldname = getattr(self.data, "fieldname", getattr(self, "fieldname", None))\n guarded_getattr(self.context, fieldname)\n \n+ def _should_force_download(self):\n+ # If this returns True, the caller should call set_headers with a filename.\n+ if not hasattr(self.data, "contentType"):\n+ return\n+ mimetype = self.data.contentType\n+ if self.use_denylist:\n+ # We explicitly deny a few mimetypes, and allow the rest.\n+ return mimetype in self.disallowed_inline_mimetypes\n+ # Use the allowlist.\n+ # We only explicitly allow a few mimetypes, and deny the rest.\n+ return mimetype not in self.allowed_inline_mimetypes\n+\n+ def set_headers(self, response=None):\n+ # set headers for the image\n+ image = self.data\n+ if response is None:\n+ response = self.request.response\n+ filename = None\n+ if self._should_force_download():\n+ # We MUST pass a filename, even a dummy one if needed.\n+ filename = getattr(image, "filename", getattr(self, "filename", None))\n+ if filename is None:\n+ filename = getattr(image, "fieldname", getattr(self, "fieldname", None))\n+ if filename is None:\n+ filename = "image.ext"\n+ set_headers(image, response, filename=filename)\n+\n def index_html(self):\n """download the image"""\n self.validate_access()\n- set_headers(self.data, self.request.response)\n+ self.set_headers()\n return stream_data(self.data)\n \n def manage_DAVget(self):\n@@ -190,7 +228,7 @@ def HEAD(self, REQUEST, RESPONSE=None):\n without transfer of the image itself\n """\n self.validate_access()\n- set_headers(self.data, REQUEST.response)\n+ self.set_headers(response=REQUEST.response)\n return ""\n \n HEAD.__roles__ = ("Anonymous",)\ndiff --git a/plone/namedfile/tests/test_display_file.py b/plone/namedfile/tests/test_display_file.py\nindex 86251a8..4ab467d 100644\n--- a/plone/namedfile/tests/test_display_file.py\n+++ b/plone/namedfile/tests/test_display_file.py\n@@ -3,12 +3,15 @@\n from plone.app.testing import SITE_OWNER_PASSWORD\n from plone.namedfile import field\n from plone.namedfile import file\n+from plone.namedfile.interfaces import IAvailableSizes\n from plone.namedfile.interfaces import IImageScaleTraversable\n from plone.namedfile.testing import PLONE_NAMEDFILE_FUNCTIONAL_TESTING\n from plone.namedfile.tests import getFile\n from plone.testing.zope import Browser\n from Products.CMFPlone.utils import safe_unicode\n+from zope.annotation import IAnnotations\n from zope.annotation import IAttributeAnnotatable\n+from zope.component import getSiteManager\n from zope.interface import implementer\n \n import transaction\n@@ -46,6 +49,11 @@ def get_disposition_header(browser):\n return browser.headers.get(name, None)\n \n \n+def custom_available_sizes():\n+ # Define available image scales.\n+ return {"custom": (10, 10)}\n+\n+\n class TestAttackVectorNamedImage(unittest.TestCase):\n layer = PLONE_NAMEDFILE_FUNCTIONAL_TESTING\n field_class = file.NamedImage\n@@ -56,6 +64,12 @@ def setUp(self):\n item = DummyContent()\n self.layer["app"]._setOb("item", item)\n self.item = self.layer["app"].item\n+ sm = getSiteManager()\n+ sm.registerUtility(component=custom_available_sizes, provided=IAvailableSizes)\n+\n+ def tearDown(self):\n+ sm = getSiteManager()\n+ sm.unregisterUtility(provided=IAvailableSizes)\n \n def get_admin_browser(self):\n browser = Browser(self.layer["app"])\n@@ -98,12 +112,50 @@ def assert_display_inline_is_download(self, base_url):\n self.assertIn("attachment", header)\n self.assertIn("filename", header)\n \n+ def assert_scale_view_works(self, base_url):\n+ # Test that accessing a scale view shows the image inline.\n+ browser = self.get_anon_browser()\n+ browser.open(base_url + "/@@images/{}".format(self.field_name))\n+ self.assertIsNone(get_disposition_header(browser))\n+\n+ # Note: the \'custom\' scale is defined in an adapter above.\n+ browser.open(base_url + "/@@images/{}/custom".format(self.field_name))\n+ self.assertIsNone(get_disposition_header(browser))\n+\n+ unique_scale_id = list(IAnnotations(self.item)["plone.scale"].keys())[0]\n+ browser.open(base_url + "/@@images/{}".format(unique_scale_id))\n+ self.assertIsNone(get_disposition_header(browser))\n+\n+ def assert_scale_view_is_download(self, base_url):\n+ # Test that accessing a scale view turns into a download.\n+ browser = self.get_anon_browser()\n+ browser.open(base_url + "/@@images/{}".format(self.field_name))\n+ header = get_disposition_header(browser)\n+ self.assertIsNotNone(header)\n+ self.assertIn("attachment", header)\n+ self.assertIn("filename", header)\n+\n+ browser.open(base_url + "/@@images/{}/custom".format(self.field_name))\n+ header = get_disposition_header(browser)\n+ self.assertIsNotNone(header)\n+ self.assertIn("attachment", header)\n+ self.assertIn("filename", header)\n+\n+ unique_scale_id = list(IAnnotations(self.item)["plone.scale"].keys())[0]\n+ browser.open(base_url + "/@@images/{}".format(unique_scale_id))\n+ header = get_disposition_header(browser)\n+ self.assertIsNotNone(header)\n+ self.assertIn("attachment", header)\n+ self.assertIn("filename", header)\n+\n def test_png_image(self):\n setattr(self.item, self.field_name, self._named_file("image.png"))\n transaction.commit()\n base_url = self.item.absolute_url()\n self.assert_download_works(base_url)\n self.assert_display_inline_works(base_url)\n+ if self.field_name == "image":\n+ self.assert_scale_view_works(base_url)\n \n def test_svg_image(self):\n setattr(self.item, self.field_name, self._named_file("image.svg"))\n@@ -111,10 +163,13 @@ def test_svg_image(self):\n base_url = self.item.absolute_url()\n self.assert_download_works(base_url)\n self.assert_display_inline_is_download(base_url)\n+ if self.field_name == "image":\n+ self.assert_scale_view_is_download(base_url)\n \n def test_filename_none(self):\n- # A \'None\' filename None probably does not happen during normal upload,\n- # but if an attacker manages this, even @@download will show inline.\n+ # A \'None\' filename probably does not happen during normal upload,\n+ # but if an attacker manages this, even @@download would show inline.\n+ # We prevent this.\n data = self._named_file("image.svg")\n data.filename = None\n setattr(self.item, self.field_name, data)\n@@ -122,6 +177,8 @@ def test_filename_none(self):\n base_url = self.item.absolute_url()\n self.assert_download_works(base_url)\n self.assert_display_inline_is_download(base_url)\n+ if self.field_name == "image":\n+ self.assert_scale_view_is_download(base_url)\n \n def test_filename_empty(self):\n # An empty filename is probably no problem, but let\'s check.\n@@ -132,6 +189,8 @@ def test_filename_empty(self):\n base_url = self.item.absolute_url()\n self.assert_download_works(base_url)\n self.assert_display_inline_is_download(base_url)\n+ if self.field_name == "image":\n+ self.assert_scale_view_is_download(base_url)\n \n \n class TestAttackVectorNamedBlobImage(TestAttackVectorNamedImage):\n' +b'diff --git a/news/1.bugfix b/news/1.bugfix\nnew file mode 100644\nindex 0000000000..37a92b93ea\n--- /dev/null\n+++ b/news/1.bugfix\n@@ -0,0 +1,5 @@\n+Fix stored XSS (Cross Site Scripting) for SVG image in user portrait.\n+Done by forcing a download instead of displaying inline.\n+Normal accessing via an image tag is not affected and is safe.\n+See `security advisory `_.\n+[maurits]\ndiff --git a/src/plone/restapi/services/users/get.py b/src/plone/restapi/services/users/get.py\nindex 10ed48af4a..bef0c3ef0e 100644\n--- a/src/plone/restapi/services/users/get.py\n+++ b/src/plone/restapi/services/users/get.py\n@@ -2,6 +2,9 @@\n from Acquisition import aq_inner\n from itertools import chain\n from plone.app.workflow.browser.sharing import merge_search_results\n+from plone.namedfile.browser import ALLOWED_INLINE_MIMETYPES\n+from plone.namedfile.browser import DISALLOWED_INLINE_MIMETYPES\n+from plone.namedfile.browser import USE_DENYLIST\n from plone.namedfile.utils import stream_data\n from plone.restapi.interfaces import ISerializeToJson\n from plone.restapi.services import Service\n@@ -13,6 +16,7 @@\n from typing import Iterable\n from typing import Sequence\n from urllib.parse import parse_qs\n+from urllib.parse import quote\n from zExceptions import BadRequest\n from zope.component import getMultiAdapter\n from zope.component import queryMultiAdapter\n@@ -220,6 +224,14 @@ def reply(self):\n \n @implementer(IPublishTraverse)\n class PortraitGet(Service):\n+ # You can control which mimetypes may be shown inline\n+ # and which must always be downloaded, for security reasons.\n+ # Make the configuration available on the class.\n+ # Then subclasses can override this.\n+ allowed_inline_mimetypes = ALLOWED_INLINE_MIMETYPES\n+ disallowed_inline_mimetypes = DISALLOWED_INLINE_MIMETYPES\n+ use_denylist = USE_DENYLIST\n+\n def __init__(self, context, request):\n super().__init__(context, request)\n self.params = []\n@@ -237,6 +249,18 @@ def _get_user_id(self):\n raise Exception("Must supply exactly one parameter (user id)")\n return self.params[0]\n \n+ def _should_force_download(self, portrait):\n+ # If this returns True, the caller should set the Content-Disposition header.\n+ mimetype = portrait.content_type\n+ if not mimetype:\n+ return False\n+ if self.use_denylist:\n+ # We explicitly deny a few mimetypes, and allow the rest.\n+ return mimetype in self.disallowed_inline_mimetypes\n+ # Use the allowlist.\n+ # We only explicitly allow a few mimetypes, and deny the rest.\n+ return mimetype not in self.allowed_inline_mimetypes\n+\n def render(self):\n if len(self.params) == 1:\n # Retrieve the user portrait\n@@ -254,6 +278,15 @@ def render(self):\n self.request.response.setStatus(404)\n return None\n \n+ if self._should_force_download(portrait):\n+ # We need a filename, even a dummy one if needed.\n+ ext = portrait.content_type.split("/")[-1].split("+")[0]\n+ filename = f"{portrait.getId()}.{ext}"\n+ filename = quote(filename.encode("utf8"))\n+ self.request.response.setHeader(\n+ "Content-Disposition", f"attachment; filename*=UTF-8\'\'{filename}"\n+ )\n+\n self.request.response.setStatus(200)\n self.request.response.setHeader("Content-Type", portrait.content_type)\n \ndiff --git a/src/plone/restapi/tests/image.svg b/src/plone/restapi/tests/image.svg\nnew file mode 100644\nindex 0000000000..18d6411909\n--- /dev/null\n+++ b/src/plone/restapi/tests/image.svg\n@@ -0,0 +1,3 @@\n+\n+ \n+\n\\ No newline at end of file\ndiff --git a/src/plone/restapi/tests/test_services_users.py b/src/plone/restapi/tests/test_services_users.py\nindex b08170eafc..6de014a8cb 100644\n--- a/src/plone/restapi/tests/test_services_users.py\n+++ b/src/plone/restapi/tests/test_services_users.py\n@@ -16,6 +16,7 @@\n from zope.component import getAdapter\n from zope.component import getUtility\n \n+import base64\n import os\n import re\n import transaction\n@@ -530,6 +531,30 @@ def test_update_portrait(self):\n self.assertEqual(response.status_code, 204)\n transaction.commit()\n \n+ def _update_portrait_with_svg(self):\n+ here = os.path.dirname(__file__)\n+ # icon from https://icons.getbootstrap.com/icons/person/\n+ path = os.path.join(here, "image.svg")\n+ with open(path, "rb") as image:\n+ data = base64.encodebytes(image.read())\n+\n+ payload = {\n+ "portrait": {\n+ "filename": "image.svg",\n+ "encoding": "base64",\n+ "data": data,\n+ "content-type": "image/svg+xml",\n+ }\n+ }\n+ self.api_session.auth = ("noam", "password")\n+ response = self.api_session.patch("/@users/noam", json=payload)\n+\n+ self.assertEqual(response.status_code, 204)\n+ transaction.commit()\n+\n+ def test_update_portrait_with_svg(self):\n+ self._update_portrait_with_svg()\n+\n user = self.api_session.get("/@users/noam").json()\n self.assertTrue(user.get("portrait").endswith("/@portrait/noam"))\n \n@@ -1072,6 +1097,23 @@ def test_get_own_user_portrait(self):\n self.assertEqual(response.headers["Content-Type"], "image/gif")\n noam_api_session.close()\n \n+ def test_get_own_user_portrait_with_svg(self):\n+ self._update_portrait_with_svg()\n+\n+ noam_api_session = RelativeSession(self.portal_url, test=self)\n+ noam_api_session.headers.update({"Accept": "application/json"})\n+ noam_api_session.auth = ("noam", "password")\n+\n+ response = noam_api_session.get("/@portrait")\n+\n+ self.assertEqual(200, response.status_code)\n+ self.assertEqual(response.headers["Content-Type"], "image/svg+xml")\n+ self.assertEqual(\n+ response.headers["Content-Disposition"],\n+ "attachment; filename*=UTF-8\'\'noam.svg",\n+ )\n+ noam_api_session.close()\n+\n def test_get_own_user_portrait_logged_out(self):\n response = self.anon_api_session.get(\n "/@portrait",\n@@ -1089,6 +1131,9 @@ def test_get_user_portrait_not_set(self):\n def test_get_user_portrait(self):\n with self.makeRealImage() as image:\n pm = api.portal.get_tool("portal_membership")\n+ # Note: if you would set an SVG in this way, this would give a\n+ # PIL.UnidentifiedImageError, which is what happens in ClassicUI\n+ # as well.\n pm.changeMemberPortrait(image, "noam")\n transaction.commit()\n \n@@ -1098,6 +1143,26 @@ def test_get_user_portrait(self):\n \n self.assertEqual(200, response.status_code)\n self.assertEqual(response.headers["Content-Type"], "image/gif")\n+ self.assertIsNone(response.headers.get("Content-Disposition"))\n+\n+ def test_get_user_portrait_with_svg(self):\n+ # If we would upload an SVG in the same way as in\n+ # test_get_user_portrait, with pm.changeMemberPortrait,\n+ # this would actually give PIL.UnidentifiedImageError,\n+ # which is what happens in ClassicUI as well.\n+ # So update it with a restapi call instead.\n+ self._update_portrait_with_svg()\n+\n+ response = self.api_session.get(\n+ "/@portrait/noam",\n+ )\n+\n+ self.assertEqual(200, response.status_code)\n+ self.assertEqual(response.headers["Content-Type"], "image/svg+xml")\n+ self.assertEqual(\n+ response.headers["Content-Disposition"],\n+ "attachment; filename*=UTF-8\'\'noam.svg",\n+ )\n \n def test_get_user_portrait_anonymous(self):\n with self.makeRealImage() as image:\n@@ -1111,6 +1176,7 @@ def test_get_user_portrait_anonymous(self):\n \n self.assertEqual(200, response.status_code)\n self.assertEqual(response.headers["Content-Type"], "image/gif")\n+ self.assertIsNone(response.headers.get("Content-Disposition"))\n \n def test_get_user_portrait_if_email_login_enabled(self):\n # enable use_email_as_login\n'