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 <https://github.com/plone/Products.CMFPlone/issues/3848>`_ and `Zope PR 1142 <https://github.com/zopefoundation/Zope/pull/1142>`_.
plone · Nov 2, 2023 · 5bc5145 · 5bc5145
1 parent 3b4c810
commit 5bc5145
Show file tree

Hide file tree

Showing 11 changed files with 71 additions and 32 deletions.
diff --git a/news/3848.bugfix 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 <https://github.com/plone/Products.CMFPlone/issues/3848>`_ and `Zope PR 1142 <https://github.com/zopefoundation/Zope/pull/1142>`_.
+@maurits and @davisagli
diff --git 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
@@ -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(
-            self.request.form.get("query", "{}")
-        ).encode(self.request.charset)
+        # 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
@@ -1,17 +1,17 @@
 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
 from zope.schema.interfaces import WrongType
 
-import json
 import plone.protect.interfaces
 
 
 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
@@ -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
@@ -18,7 +19,6 @@
 from zope.publisher.interfaces import IPublishTraverse
 
 import codecs
-import json
 import plone
 
 
@@ -57,7 +57,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
@@ -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
@@ -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,8 +105,9 @@ def test_login_with_zope_user(self):
         self.layer["app"].acl_users.plugins.users.addUser(
             "zopeuser", "zopeuser", TEST_USER_PASSWORD
         )
-        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()

diff --git 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
@@ -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
@@ -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
@@ -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,31 +176,34 @@ 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"])
 
     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())
         self.assertEqual("published", self.wftool.getInfoFor(folder, "review_state"))
         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(
             "2018-06-24T09:17:00+00:00", self.portal.doc1.effective().ISO8601()
         )
 
     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())