From 649b198bca1c5991439e04d480b0810d8905f40f Mon Sep 17 00:00:00 2001 From: Maurits van Rees Date: Thu, 2 Nov 2023 17:40:29 +0100 Subject: [PATCH] Read json from request BODYFILE instead of BODY. This fixes a regression due to a low Zope form memory limit of 1MB used since Plone 6.0.7. See `CMFPlone issue 3848 `_ and `Zope PR 1142 `_. --- news/3848.bugfix | 4 +++ src/plone/restapi/deserializer/__init__.py | 3 +- .../restapi/services/querystringsearch/get.py | 8 +++-- src/plone/restapi/services/registry/update.py | 3 +- src/plone/restapi/services/users/update.py | 3 +- src/plone/restapi/testing.py | 8 +++++ src/plone/restapi/tests/test_auth.py | 29 ++++++++++++------- .../restapi/tests/test_blocks_deserializer.py | 3 +- .../tests/test_dxcontent_deserializer.py | 5 ++-- .../restapi/tests/test_site_deserializer.py | 5 ++-- src/plone/restapi/tests/test_workflow.py | 24 ++++++++++----- 11 files changed, 68 insertions(+), 27 deletions(-) create mode 100644 news/3848.bugfix diff --git a/news/3848.bugfix b/news/3848.bugfix new file mode 100644 index 0000000000..b4f969185a --- /dev/null +++ b/news/3848.bugfix @@ -0,0 +1,4 @@ +Read json from request ``BODYFILE`` instead of ``BODY``. +This fixes a regression due to a low Zope form memory limit of 1MB used since Plone 6.0.7. +See `CMFPlone issue 3848 `_ and `Zope PR 1142 `_. +@maurits and @davisagli diff --git a/src/plone/restapi/deserializer/__init__.py b/src/plone/restapi/deserializer/__init__.py index cb790cb704..33c037aa99 100644 --- a/src/plone/restapi/deserializer/__init__.py +++ b/src/plone/restapi/deserializer/__init__.py @@ -5,7 +5,8 @@ def json_body(request): try: - data = json.loads(request.get("BODY") or "{}") + bodyfile = request.get("BODYFILE") + data = {} if bodyfile is None else json.load(bodyfile) except ValueError: raise DeserializationError("No JSON object could be decoded") if not isinstance(data, dict): diff --git a/src/plone/restapi/services/querystringsearch/get.py b/src/plone/restapi/services/querystringsearch/get.py index acd9f3647b..dc0712283d 100644 --- a/src/plone/restapi/services/querystringsearch/get.py +++ b/src/plone/restapi/services/querystringsearch/get.py @@ -1,3 +1,4 @@ +from io import BytesIO from pkg_resources import get_distribution from pkg_resources import parse_version from plone.restapi.bbb import IPloneSiteRoot @@ -101,10 +102,13 @@ class QuerystringSearchGet(Service): def reply(self): # We need to copy the JSON query parameters from the querystring - # into the request body, because that's where other code expects to find them - self.request["BODY"] = parse.unquote( + # into the request BODY and BODYFILE, because that's where other code + # expects to find them. + body = parse.unquote( self.request.form.get("query", "{}") ).encode(self.request.charset) + self.request["BODY"] = body + self.request["BODYFILE"] = BytesIO(body) # unset the get parameters self.request.form = {} diff --git a/src/plone/restapi/services/registry/update.py b/src/plone/restapi/services/registry/update.py index 8693e9681e..b4a350e0b9 100644 --- a/src/plone/restapi/services/registry/update.py +++ b/src/plone/restapi/services/registry/update.py @@ -1,5 +1,6 @@ from plone.registry import field from plone.registry.interfaces import IRegistry +from plone.restapi.deserializer import json_body from plone.restapi.services import Service from zope.component import getUtility from zope.interface import alsoProvides @@ -11,7 +12,7 @@ class RegistryUpdate(Service): def reply(self): - records_to_update = json.loads(self.request.get("BODY", "{}")) + records_to_update = json_body(self.request) registry = getUtility(IRegistry) # Disable CSRF protection diff --git a/src/plone/restapi/services/users/update.py b/src/plone/restapi/services/users/update.py index 0e07d69d03..e43e7c0ec3 100644 --- a/src/plone/restapi/services/users/update.py +++ b/src/plone/restapi/services/users/update.py @@ -4,6 +4,7 @@ from OFS.Image import Image from plone.restapi import _ from plone.restapi.bbb import ISecuritySchema +from plone.restapi.deserializer import json_body from plone.restapi.services import Service from Products.CMFCore.permissions import SetOwnPassword from Products.CMFCore.utils import getToolByName @@ -57,7 +58,7 @@ def translate(self, msgid): ) def reply(self): - user_settings_to_update = json.loads(self.request.get("BODY", "{}")) + user_settings_to_update = json_body(self.request) user = self._get_user(self._get_user_id) # Disable CSRF protection diff --git a/src/plone/restapi/testing.py b/src/plone/restapi/testing.py index 7ed7c0358d..b677298659 100644 --- a/src/plone/restapi/testing.py +++ b/src/plone/restapi/testing.py @@ -1,5 +1,6 @@ # pylint: disable=E1002 # E1002: Use of super on an old style class +from io import BytesIO from plone.app.contenttypes.testing import PLONE_APP_CONTENTTYPES_FIXTURE from plone.app.i18n.locales.interfaces import IContentLanguages from plone.app.i18n.locales.interfaces import IMetadataLanguages @@ -355,3 +356,10 @@ def normalize_html(value): line = " " + line lines.append(line) return "".join(lines).strip() + + +def set_request_body(request, body): + if isinstance(body, str): + body = body.encode("utf-8") + request["BODY"] = body + request["BODYFILE"] = BytesIO(body) diff --git a/src/plone/restapi/tests/test_auth.py b/src/plone/restapi/tests/test_auth.py index ece162b21e..2b48dbf989 100644 --- a/src/plone/restapi/tests/test_auth.py +++ b/src/plone/restapi/tests/test_auth.py @@ -2,6 +2,7 @@ from plone.app.testing import SITE_OWNER_PASSWORD from plone.app.testing import TEST_USER_PASSWORD from plone.restapi.permissions import UseRESTAPI +from plone.restapi.testing import set_request_body from plone.restapi.testing import PLONE_RESTAPI_DX_INTEGRATION_TESTING from unittest import TestCase from zExceptions import Unauthorized @@ -40,16 +41,19 @@ def test_login_without_credentials_fails(self): self.assertNotIn("token", res) def test_login_with_invalid_credentials_fails(self): - self.request["BODY"] = '{"login": "admin", "password": "admin"}' + set_request_body(self.request, '{"login": "admin", "password": "admin"}') service = self.traverse() res = service.reply() self.assertIn("error", res) self.assertNotIn("token", res) def test_successful_login_returns_token(self): - self.request["BODY"] = '{{"login": "{}", "password": "{}"}}'.format( - SITE_OWNER_NAME, - SITE_OWNER_PASSWORD, + set_request_body( + self.request, + '{{"login": "{}", "password": "{}"}}'.format( + SITE_OWNER_NAME, + SITE_OWNER_PASSWORD, + ), ) service = self.traverse() res = service.reply() @@ -69,9 +73,12 @@ def test_expired_token_returns_400(self): def test_login_without_api_permission(self): self.portal.manage_permission(UseRESTAPI, roles=[]) - self.request["BODY"] = '{{"login": "{}", "password": "{}"}}'.format( - SITE_OWNER_NAME, - SITE_OWNER_PASSWORD, + set_request_body( + self.request, + '{{"login": "{}", "password": "{}"}}'.format( + SITE_OWNER_NAME, + SITE_OWNER_PASSWORD, + ), ) service = self.traverse() res = service.render() @@ -82,8 +89,9 @@ def test_login_with_zope_user_fails_without_pas_plugin(self): uf.plugins.users.addUser("zopeuser", "zopeuser", TEST_USER_PASSWORD) if "jwt_auth" in uf: uf["jwt_auth"].manage_activateInterfaces([]) - self.request["BODY"] = ( - '{"login": "zopeuser", "password": "' + TEST_USER_PASSWORD + '"}' + set_request_body( + self.request, + '{"login": "zopeuser", "password": "' + TEST_USER_PASSWORD + '"}', ) service = self.traverse() res = service.reply() @@ -97,7 +105,8 @@ def test_login_with_zope_user(self): self.layer["app"].acl_users.plugins.users.addUser( "zopeuser", "zopeuser", TEST_USER_PASSWORD ) - self.request["BODY"] = ( + set_request_body( + self.request, '{"login": "zopeuser", "password": "' + TEST_USER_PASSWORD + '"}' ) service = self.traverse() diff --git a/src/plone/restapi/tests/test_blocks_deserializer.py b/src/plone/restapi/tests/test_blocks_deserializer.py index 37a4e6ecd5..07fd019433 100644 --- a/src/plone/restapi/tests/test_blocks_deserializer.py +++ b/src/plone/restapi/tests/test_blocks_deserializer.py @@ -3,6 +3,7 @@ from plone.restapi.behaviors import IBlocks from plone.restapi.interfaces import IBlockFieldDeserializationTransformer from plone.restapi.interfaces import IDeserializeFromJson +from plone.restapi.testing import set_request_body from plone.restapi.testing import PLONE_RESTAPI_DX_INTEGRATION_TESTING from plone.uuid.interfaces import IUUID from zope.component import adapter @@ -42,7 +43,7 @@ def setUp(self): def deserialize(self, blocks=None, validate_all=False, context=None): blocks = blocks or "" context = context or self.portal.doc1 - self.request["BODY"] = json.dumps({"blocks": blocks}) + set_request_body(self.request, json.dumps({"blocks": blocks})) deserializer = getMultiAdapter((context, self.request), IDeserializeFromJson) return deserializer(validate_all=validate_all) diff --git a/src/plone/restapi/tests/test_dxcontent_deserializer.py b/src/plone/restapi/tests/test_dxcontent_deserializer.py index e9cc6fdf14..3db53fe80f 100644 --- a/src/plone/restapi/tests/test_dxcontent_deserializer.py +++ b/src/plone/restapi/tests/test_dxcontent_deserializer.py @@ -4,6 +4,7 @@ from plone.restapi.exceptions import DeserializationError from plone.restapi.interfaces import IDeserializeFromJson from plone.restapi.interfaces import ISerializeToJson +from plone.restapi.testing import set_request_body from plone.restapi.testing import PLONE_RESTAPI_DX_INTEGRATION_TESTING from plone.restapi.tests.dxtypes import ITestAnnotationsBehavior from plone.restapi.tests.mixin_ordering import OrderingMixin @@ -43,7 +44,7 @@ def setUp(self): def deserialize(self, body="{}", validate_all=False, context=None, create=False): context = context or self.portal.doc1 - self.request["BODY"] = body + set_request_body(self.request, body) deserializer = getMultiAdapter((context, self.request), IDeserializeFromJson) return deserializer(validate_all=validate_all, create=create) @@ -257,7 +258,7 @@ def deserialize(self, field, value, validate_all=False, context=None): body = {} body[field] = value body = json.dumps(body) - self.request["BODY"] = body + set_request_body(self.request, body) deserializer = getMultiAdapter((context, self.request), IDeserializeFromJson) return deserializer(validate_all=validate_all) diff --git a/src/plone/restapi/tests/test_site_deserializer.py b/src/plone/restapi/tests/test_site_deserializer.py index 80b8b85234..6d66a074bc 100644 --- a/src/plone/restapi/tests/test_site_deserializer.py +++ b/src/plone/restapi/tests/test_site_deserializer.py @@ -2,6 +2,7 @@ from plone.dexterity.interfaces import IDexterityFTI from plone.dexterity.schema import SCHEMA_CACHE from plone.restapi.interfaces import IDeserializeFromJson +from plone.restapi.testing import set_request_body from plone.restapi.testing import PLONE_RESTAPI_DX_INTEGRATION_TESTING from plone.restapi.tests.mixin_ordering import OrderingMixin from zope.component import getMultiAdapter @@ -34,7 +35,7 @@ def setUp(self): def deserialize(self, body="{}", validate_all=False, context=None): context = context or self.portal - self.request["BODY"] = body + set_request_body(self.request, body) deserializer = getMultiAdapter((context, self.request), IDeserializeFromJson) return deserializer(validate_all=validate_all) @@ -70,7 +71,7 @@ def setUp(self): def deserialize(self, body="{}", validate_all=False, context=None): context = context or self.portal - self.request["BODY"] = body + set_request_body(self.request, body) deserializer = getMultiAdapter((context, self.request), IDeserializeFromJson) return deserializer(validate_all=validate_all) diff --git a/src/plone/restapi/tests/test_workflow.py b/src/plone/restapi/tests/test_workflow.py index c31539ae68..5094b226cb 100644 --- a/src/plone/restapi/tests/test_workflow.py +++ b/src/plone/restapi/tests/test_workflow.py @@ -8,6 +8,7 @@ from plone.app.testing import TEST_USER_NAME from plone.app.testing import TEST_USER_PASSWORD from plone.restapi.interfaces import ISerializeToJson +from plone.restapi.testing import set_request_body from plone.restapi.testing import PLONE_RESTAPI_WORKFLOWS_INTEGRATION_TESTING from Products.CMFCore.utils import getToolByName from unittest import TestCase @@ -175,7 +176,7 @@ def test_calling_workflow_with_additional_path_segments_results_in_404(self): self.traverse("/plone/doc1/@workflow/publish/test") def test_transition_with_comment(self): - self.request["BODY"] = '{"comment": "A comment"}' + set_request_body(self.request, '{"comment": "A comment"}') service = self.traverse("/plone/doc1/@workflow/publish") res = service.reply() self.assertEqual("A comment", res["comments"]) @@ -183,7 +184,10 @@ def test_transition_with_comment(self): def test_transition_including_children(self): folder = self.portal[self.portal.invokeFactory("Folder", id="folder")] subfolder = folder[folder.invokeFactory("Folder", id="subfolder")] - self.request["BODY"] = '{"comment": "A comment", "include_children": true}' + set_request_body( + self.request, + '{"comment": "A comment", "include_children": true}', + ) service = self.traverse("/plone/folder/@workflow/publish") service.reply() self.assertEqual(200, self.request.response.getStatus()) @@ -191,7 +195,7 @@ def test_transition_including_children(self): self.assertEqual("published", self.wftool.getInfoFor(subfolder, "review_state")) def test_transition_with_effective_date(self): - self.request["BODY"] = '{"effective": "2018-06-24T09:17:02"}' + set_request_body(self.request, '{"effective": "2018-06-24T09:17:02"}') service = self.traverse("/plone/doc1/@workflow/publish") service.reply() self.assertEqual( @@ -199,7 +203,7 @@ def test_transition_with_effective_date(self): ) def test_transition_with_expiration_date(self): - self.request["BODY"] = '{"expires": "2019-06-20T18:00:00"}' + set_request_body(self.request, '{"expires": "2019-06-20T18:00:00"}') service = self.traverse("/plone/doc1/@workflow/publish") service.reply() self.assertEqual( @@ -213,7 +217,7 @@ def test_invalid_transition_results_in_400(self): self.assertEqual("WorkflowException", res["error"]["type"]) def test_invalid_effective_date_results_in_400(self): - self.request["BODY"] = '{"effective": "now"}' + set_request_body(self.request, '{"effective": "now"}') service = self.traverse("/plone/doc1/@workflow/publish") res = service.reply() self.assertEqual(400, self.request.response.getStatus()) @@ -241,7 +245,10 @@ def test_transition_including_children_without_wf(self): folder.invokeFactory("Document", id="document", title="Document") ] - self.request["BODY"] = '{"comment": "A comment", "include_children": true}' + set_request_body( + self.request, + '{"comment": "A comment", "include_children": true}', + ) service = self.traverse("/plone/folder/@workflow/publish") service.reply() self.assertEqual(200, self.request.response.getStatus()) @@ -266,7 +273,10 @@ def test_transition_recursive_do_not_break_if_children_does_not_have_the_action( self.assertEqual("private", self.wftool.getInfoFor(document, "review_state")) # now try to publish folder and all children - self.request["BODY"] = '{"comment": "A comment", "include_children": true}' + set_request_body( + self.request, + '{"comment": "A comment", "include_children": true}', + ) service = self.traverse("/plone/folder/@workflow/publish") service.reply() self.assertEqual(200, self.request.response.getStatus())