Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Read json from request BODYFILE instead of BODY. #1731

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
25 changes: 21 additions & 4 deletions src/plone/restapi/deserializer/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,27 @@


def json_body(request):
try:
data = json.loads(request.get("BODY") or "{}")
except ValueError:
raise DeserializationError("No JSON object could be decoded")
bodyfile = request.get("BODYFILE")
if bodyfile is None:
data = {}
else:
try:
fpos = bodyfile.tell()
except Exception:
fpos = None
if fpos:
# Something has already begun reading the bodyfile.
# Go back to the beginning.
bodyfile.seek(0)
try:
body = bodyfile.read()
data = {} if not body else json.loads(body)
except ValueError:
raise DeserializationError("No JSON object could be decoded")
finally:
if fpos is not None:
# Go back to the previous position.
bodyfile.seek(fpos)
if not isinstance(data, dict):
raise DeserializationError("Malformed body")
return data
Expand Down
12 changes: 8 additions & 4 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(
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 = {}
Expand Down
4 changes: 2 additions & 2 deletions src/plone/restapi/services/registry/update.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down
4 changes: 2 additions & 2 deletions 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 All @@ -18,7 +19,6 @@
from zope.publisher.interfaces import IPublishTraverse

import codecs
import json
import plone


Expand Down Expand Up @@ -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
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)
31 changes: 20 additions & 11 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,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()
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