From ffc42c4a32015525ae58f5e3bf557c18ef1ec88b Mon Sep 17 00:00:00 2001 From: Dylan Jay Date: Mon, 13 Jan 2025 11:57:10 +0700 Subject: [PATCH 1/7] Handle bad input as BadRequest instead of ValueError useful so it can be more easily ignored --- src/plone/restapi/services/navigation/get.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/plone/restapi/services/navigation/get.py b/src/plone/restapi/services/navigation/get.py index a21a52abe4..6420417d84 100644 --- a/src/plone/restapi/services/navigation/get.py +++ b/src/plone/restapi/services/navigation/get.py @@ -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) @@ -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 as e: + raise BadRequest(e) else: self.depth = 1 From 26a8b7a053efc446beb4392df6f252e928886e86 Mon Sep 17 00:00:00 2001 From: Dylan Jay Date: Tue, 14 Jan 2025 15:55:34 +0700 Subject: [PATCH 2/7] added basic test --- src/plone/restapi/tests/test_services_navigation.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/plone/restapi/tests/test_services_navigation.py b/src/plone/restapi/tests/test_services_navigation.py index 890c5c1220..a61009005c 100644 --- a/src/plone/restapi/tests/test_services_navigation.py +++ b/src/plone/restapi/tests/test_services_navigation.py @@ -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"]) From bc924b3793a8297be78adf9c98fcc947a8ee02e1 Mon Sep 17 00:00:00 2001 From: Dylan Jay Date: Tue, 14 Jan 2025 15:57:09 +0700 Subject: [PATCH 3/7] Update src/plone/restapi/services/navigation/get.py Co-authored-by: David Glick --- src/plone/restapi/services/navigation/get.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/plone/restapi/services/navigation/get.py b/src/plone/restapi/services/navigation/get.py index 6420417d84..de9f91ed6b 100644 --- a/src/plone/restapi/services/navigation/get.py +++ b/src/plone/restapi/services/navigation/get.py @@ -32,7 +32,7 @@ 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 as e: + except (ValueError, TypeError) as e: raise BadRequest(e) else: self.depth = 1 From 4fb0e7ba58074a3ecd2df9ea667fbd7edc1b8b84 Mon Sep 17 00:00:00 2001 From: Dylan Jay Date: Tue, 14 Jan 2025 16:08:51 +0700 Subject: [PATCH 4/7] batching int valueerror to badrequests --- src/plone/restapi/batching.py | 18 +++++++++++------- src/plone/restapi/tests/test_batching.py | 4 ++++ 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/src/plone/restapi/batching.py b/src/plone/restapi/batching.py index 7b0b814eea..30653c43c3 100644 --- a/src/plone/restapi/batching.py +++ b/src/plone/restapi/batching.py @@ -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 @@ -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): diff --git a/src/plone/restapi/tests/test_batching.py b/src/plone/restapi/tests/test_batching.py index 3e809ebabc..83c803157b 100644 --- a/src/plone/restapi/tests/test_batching.py +++ b/src/plone/restapi/tests/test_batching.py @@ -185,6 +185,10 @@ 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): From 30b8039642d23d116aca89b13c6099d2aeba6d9c Mon Sep 17 00:00:00 2001 From: Dylan Jay Date: Tue, 14 Jan 2025 16:11:40 +0700 Subject: [PATCH 5/7] fix flake8 --- src/plone/restapi/tests/test_batching.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/plone/restapi/tests/test_batching.py b/src/plone/restapi/tests/test_batching.py index 83c803157b..4470b2c8f6 100644 --- a/src/plone/restapi/tests/test_batching.py +++ b/src/plone/restapi/tests/test_batching.py @@ -190,6 +190,7 @@ def test_batching_badrequests(self): self.assertEqual(response.status_code, 400) self.assertIn("invalid literal for int()", response.json()["message"]) + class TestBatchingDXFolders(TestBatchingDXBase): layer = PLONE_RESTAPI_DX_FUNCTIONAL_TESTING From 3a4fbd171897213df113d1475ea3e539a42d8f8e Mon Sep 17 00:00:00 2001 From: Dylan Jay Date: Tue, 14 Jan 2025 16:24:52 +0700 Subject: [PATCH 6/7] add changelog --- news/1855.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 news/1855.bugfix diff --git a/news/1855.bugfix b/news/1855.bugfix new file mode 100644 index 0000000000..9c3136ad4d --- /dev/null +++ b/news/1855.bugfix @@ -0,0 +1 @@ +Changed bad int inputs into 500 Exceptions to 400 BadRequest so they can be filtered out of logs more easily. - djay \ No newline at end of file From 03d8487d795be794475a461f6d5524c3902c7aef Mon Sep 17 00:00:00 2001 From: David Glick Date: Tue, 14 Jan 2025 14:48:53 -0800 Subject: [PATCH 7/7] Update news/1855.bugfix --- news/1855.bugfix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/news/1855.bugfix b/news/1855.bugfix index 9c3136ad4d..b164e9f037 100644 --- a/news/1855.bugfix +++ b/news/1855.bugfix @@ -1 +1 @@ -Changed bad int inputs into 500 Exceptions to 400 BadRequest so they can be filtered out of logs more easily. - djay \ No newline at end of file +Changed bad int inputs into 500 Exceptions to 400 BadRequest so they can be filtered out of logs more easily. @djay \ No newline at end of file