Skip to content

Commit

Permalink
Read json from request BODYFILE instead of BODY.
Browse files Browse the repository at this point in the history
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>`_.
  • Loading branch information
mauritsvanrees committed Nov 2, 2023
1 parent 3b4c810 commit 649b198
Show file tree
Hide file tree
Showing 11 changed files with 68 additions and 27 deletions.
4 changes: 4 additions & 0 deletions news/3848.bugfix
Original file line number Diff line number Diff line change
@@ -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
3 changes: 2 additions & 1 deletion src/plone/restapi/deserializer/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
8 changes: 6 additions & 2 deletions src/plone/restapi/services/querystringsearch/get.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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 = {}
Expand Down
3 changes: 2 additions & 1 deletion src/plone/restapi/services/registry/update.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand Down
3 changes: 2 additions & 1 deletion src/plone/restapi/services/users/update.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions src/plone/restapi/testing.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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)
29 changes: 19 additions & 10 deletions src/plone/restapi/tests/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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()
Expand Down
3 changes: 2 additions & 1 deletion src/plone/restapi/tests/test_blocks_deserializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down
5 changes: 3 additions & 2 deletions src/plone/restapi/tests/test_dxcontent_deserializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down
5 changes: 3 additions & 2 deletions src/plone/restapi/tests/test_site_deserializer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down
24 changes: 17 additions & 7 deletions src/plone/restapi/tests/test_workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -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())
Expand Down Expand Up @@ -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())
Expand All @@ -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())
Expand Down

0 comments on commit 649b198

Please sign in to comment.