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

Add parse_int to handle all cases of BadRequest from ints passed in #1858

Merged
merged 12 commits into from
Jan 21, 2025
1 change: 1 addition & 0 deletions news/1858.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add parse_int to handle all cases of BadRequests from ints passed in. @djay
16 changes: 9 additions & 7 deletions src/plone/restapi/batching.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from plone.batching.batch import Batch
from plone.restapi.deserializer import json_body
from plone.restapi.deserializer import parse_int
from plone.restapi.exceptions import DeserializationError
from urllib.parse import parse_qsl
from urllib.parse import urlencode
Expand All @@ -14,14 +15,15 @@ def __init__(self, request, results):
self.request = request

try:
self.b_start = int(json_body(self.request).get("b_start", False)) or int(
self.request.form.get("b_start", 0)
)
self.b_size = int(json_body(self.request).get("b_size", False)) or int(
self.request.form.get("b_size", DEFAULT_BATCH_SIZE)
)
except (ValueError, DeserializationError) as e:
data = json_body(request)
except DeserializationError as e:
raise BadRequest(e)
self.b_start = parse_int(data, "b_start", False) or parse_int(
self.request.form, "b_start", 0
)
self.b_size = parse_int(data, "b_size", False) or parse_int(
self.request.form, "b_size", DEFAULT_BATCH_SIZE
)
self.batch = Batch(results, self.b_size, self.b_start)

def __iter__(self):
Expand Down
17 changes: 17 additions & 0 deletions src/plone/restapi/deserializer/__init__.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from plone.restapi.exceptions import DeserializationError
from zExceptions import BadRequest

import json

Expand Down Expand Up @@ -28,3 +29,19 @@ def boolean_value(value):

"""
return value not in {False, "false", "False", "0", 0}


def parse_int(data, prop, default):
"""
Args:
data: dict from a request
prop: name of a integer paramater in the dict
default: default if not found

Returns: an integer
Raises: BadRequest if not an int
"""
try:
return int(data.get(prop, default))
except (ValueError, TypeError):
raise BadRequest(f"Invalid {prop}: Not an integer")
10 changes: 5 additions & 5 deletions src/plone/restapi/services/discussion/conversation.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
from plone.app.discussion.browser.comment import EditCommentForm
from plone.app.discussion.browser.comments import CommentForm
from plone.app.discussion.interfaces import IConversation
from plone.restapi.deserializer import json_body
from plone.restapi.deserializer import json_body, parse_int
from plone.restapi.interfaces import ISerializeToJson
from plone.restapi.services import Service
from plone.restapi.services.discussion.utils import can_delete
Expand Down Expand Up @@ -38,7 +38,7 @@ class CommentsGet(Service):

def publishTraverse(self, request, name):
if name:
self.comment_id = int(name)
self.comment_id = parse_int(dict(comment_id=name), "comment_id", None)
davisagli marked this conversation as resolved.
Show resolved Hide resolved
return self

def reply(self):
Expand All @@ -57,7 +57,7 @@ class CommentsAdd(Service):

def publishTraverse(self, request, name):
if name:
self.comment_id = int(name)
self.comment_id = parse_int(dict(comment_id=name), "comment_id", None)
request["form.widgets.in_reply_to"] = name
return self

Expand Down Expand Up @@ -96,7 +96,7 @@ class CommentsUpdate(Service):

def publishTraverse(self, request, name):
if name:
self.comment_id = int(name)
self.comment_id = parse_int(dict(comment_id=name), "comment_id", None)
request["form.widgets.comment_id"] = name
return self

Expand Down Expand Up @@ -140,7 +140,7 @@ class CommentsDelete(Service):
comment_id = None

def publishTraverse(self, request, name):
self.comment_id = int(name)
self.comment_id = parse_int(dict(comment_id=name), "comment_id", None)
return self

def reply(self):
Expand Down
10 changes: 2 additions & 8 deletions src/plone/restapi/services/navigation/get.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from plone.registry.interfaces import IRegistry
from plone.restapi.bbb import INavigationSchema
from plone.restapi.bbb import safe_text
from plone.restapi.deserializer import parse_int
from plone.restapi.interfaces import IExpandableElement
from plone.restapi.serializer.converters import json_compatible
from plone.restapi.services import Service
Expand All @@ -17,7 +18,6 @@
from zope.i18n import translate
from zope.interface import implementer
from zope.interface import Interface
from zExceptions import BadRequest


@implementer(IExpandableElement)
Expand All @@ -29,13 +29,7 @@ def __init__(self, context, request):
self.portal = getSite()

def __call__(self, expand=False):
if self.request.form.get("expand.navigation.depth", False):
try:
self.depth = int(self.request.form["expand.navigation.depth"])
except (ValueError, TypeError) as e:
raise BadRequest(e)
else:
self.depth = 1
self.depth = parse_int(self.request.form, "expand.navigation.depth", 1)

result = {"navigation": {"@id": f"{self.context.absolute_url()}/@navigation"}}
if not expand:
Expand Down
17 changes: 5 additions & 12 deletions src/plone/restapi/services/querystringsearch/get.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from pkg_resources import parse_version
from plone.restapi.bbb import IPloneSiteRoot
from plone.restapi.deserializer import json_body
from plone.restapi.deserializer import parse_int
from plone.restapi.exceptions import DeserializationError
from plone.restapi.interfaces import ISerializeToJson
from plone.restapi.services import Service
Expand Down Expand Up @@ -31,20 +32,12 @@ def __call__(self):
raise BadRequest(str(err))

query = data.get("query", None)
try:
b_start = int(data.get("b_start", 0))
except (ValueError, TypeError):
raise BadRequest("Invalid b_start")
try:
b_size = int(data.get("b_size", 25))
except (ValueError, TypeError):
raise BadRequest("Invalid b_size")

b_start = parse_int(data, "b_start", 0)
b_size = parse_int(data, "b_size", 25)
sort_on = data.get("sort_on", None)
sort_order = data.get("sort_order", None)
try:
limit = int(data.get("limit", 1000))
except (ValueError, TypeError):
raise BadRequest("Invalid limit")
limit = parse_int(data, "limit", 1000)
fullobjects = bool(data.get("fullobjects", False))

if not query:
Expand Down
6 changes: 5 additions & 1 deletion src/plone/restapi/tests/test_batching.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,11 @@ def test_batching_links_omitted_if_resulset_fits_in_single_batch(self):
def test_batching_badrequests(self):
response = self.api_session.get("/collection?b_size=php")
self.assertEqual(response.status_code, 400)
self.assertIn("invalid literal for int()", response.json()["message"])
self.assertIn("Invalid b_size", response.json()["message"])

response = self.api_session.get("/collection?b_size:list=1")
self.assertEqual(response.status_code, 400)
self.assertIn("Invalid b_size", response.json()["message"])


class TestBatchingDXFolders(TestBatchingDXBase):
Expand Down
2 changes: 1 addition & 1 deletion src/plone/restapi/tests/test_services_navigation.py
Original file line number Diff line number Diff line change
Expand Up @@ -246,4 +246,4 @@ def test_navigation_badrequests(self):
)

self.assertEqual(response.status_code, 400)
self.assertIn("invalid literal for int()", response.json()["message"])
self.assertIn("Invalid expand.navigation.depth", response.json()["message"])
Loading