Skip to content

Commit

Permalink
Handle bad input as BadRequest instead of ValueError (#1855)
Browse files Browse the repository at this point in the history
* Handle bad input as BadRequest instead of ValueError

useful so it can be more easily ignored

* added basic test

* Update src/plone/restapi/services/navigation/get.py

Co-authored-by: David Glick <[email protected]>

* batching int valueerror to badrequests

* fix flake8

* add changelog

* Update news/1855.bugfix

---------

Co-authored-by: David Glick <[email protected]>
  • Loading branch information
djay and davisagli authored Jan 14, 2025
1 parent 7214df3 commit aeb3ad0
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 8 deletions.
1 change: 1 addition & 0 deletions news/1855.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Changed bad int inputs into 500 Exceptions to 400 BadRequest so they can be filtered out of logs more easily. @djay
18 changes: 11 additions & 7 deletions src/plone/restapi/batching.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
from plone.batching.batch import Batch
from plone.restapi.deserializer import json_body
from plone.restapi.exceptions import DeserializationError
from urllib.parse import parse_qsl
from urllib.parse import urlencode
from zExceptions import BadRequest


DEFAULT_BATCH_SIZE = 25
Expand All @@ -11,13 +13,15 @@ class HypermediaBatch:
def __init__(self, request, results):
self.request = request

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)
)

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:
raise BadRequest(e)
self.batch = Batch(results, self.b_size, self.b_start)

def __iter__(self):
Expand Down
6 changes: 5 additions & 1 deletion src/plone/restapi/services/navigation/get.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from zope.i18n import translate
from zope.interface import implementer
from zope.interface import Interface
from zExceptions import BadRequest


@implementer(IExpandableElement)
Expand All @@ -29,7 +30,10 @@ def __init__(self, context, request):

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

Expand Down
5 changes: 5 additions & 0 deletions src/plone/restapi/tests/test_batching.py
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,11 @@ def test_batching_links_omitted_if_resulset_fits_in_single_batch(self):
response = self.api_session.get("/collection?b_size=100")
self.assertNotIn("batching", list(response.json()))

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"])


class TestBatchingDXFolders(TestBatchingDXBase):

Expand Down
8 changes: 8 additions & 0 deletions src/plone/restapi/tests/test_services_navigation.py
Original file line number Diff line number Diff line change
Expand Up @@ -239,3 +239,11 @@ def test_use_nav_title_when_available_and_set(self):
)

self.assertEqual(response.json()["items"][1]["items"][-1]["title"], nav_title)

def test_navigation_badrequests(self):
response = self.api_session.get(
"/folder/@navigation", params={"expand.navigation.depth": "php"}
)

self.assertEqual(response.status_code, 400)
self.assertIn("invalid literal for int()", response.json()["message"])

0 comments on commit aeb3ad0

Please sign in to comment.