[fc] Repository: plone.restapi

Branch: refs/heads/main
Date: 2025-01-20T21:30:22-08:00
Author: Dylan Jay (djay) <[email protected]>
Commit: plone/plone.restapi@18da4ec

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

* Another should be BadRequest found in the wild

* add changelog

* add parse_int util

* use parse_int for navigation also

* improve bad int message

* change log

* fix flake8/black

* use parse_int for comment_ids

* fix black

* Update news/1858.bugfix

---------

Co-authored-by: David Glick &lt;[email protected]&gt;

Files changed:
A news/1858.bugfix
M src/plone/restapi/batching.py
M src/plone/restapi/deserializer/__init__.py
M src/plone/restapi/services/discussion/conversation.py
M src/plone/restapi/services/navigation/get.py
M src/plone/restapi/services/querystringsearch/get.py
M src/plone/restapi/tests/test_batching.py
M src/plone/restapi/tests/test_services_navigation.py

last_commit.txt

@@ -1,40 +1,46 @@
-Repository: plone.app.users
-
-
-Branch: refs/heads/master
-Date: 2025-01-08T09:13:03+01:00
-Author: ale-rt (ale-rt) <[email protected]>
-Commit: https://github.com/plone/plone.app.users/commit/0f5477693b27d666bc457f7bc4512c6fd3e5b5fc
-
-The member-search logic has been moved from the page template to the Python code
-
-Refs. #125
+Repository: plone.restapi
+
+
+Branch: refs/heads/main
+Date: 2025-01-20T21:30:22-08:00
+Author: Dylan Jay (djay) <[email protected]>
+Commit: https://github.com/plone/plone.restapi/commit/18da4ec79546c4c44a60dd237aea700f0c2ba9d7
+
+Add parse_int to handle all cases of BadRequest from ints passed in (#1858)
+
+* Another should be BadRequest found in the wild
+
+* add changelog
+
+* add parse_int util
+
+* use parse_int for navigation also
+
+* improve bad int message
+
+* change log
+
+* fix flake8/black
+
+* use parse_int for comment_ids
+
+* fix black
+
+* Update news/1858.bugfix
+
+---------
+
+Co-authored-by: David Glick &lt;[email protected]&gt;
 
 Files changed:
-A news/125.bugfix.rst
-M plone/app/users/browser/membersearch.py
-M plone/app/users/browser/membersearch_form.pt
-M setup.py
-
-b'diff --git a/news/125.bugfix.rst b/news/125.bugfix.rst\nnew file mode 100644\nindex 00000000..cf699cfb\n--- /dev/null\n+++ b/news/125.bugfix.rst\n@@ -0,0 +1,2 @@\n+The member-search logic has been moved from the page template to the Python code\n+[ale-rt]\ndiff --git a/plone/app/users/browser/membersearch.py b/plone/app/users/browser/membersearch.py\nindex 08d1bc23..5cc031bf 100644\n--- a/plone/app/users/browser/membersearch.py\n+++ b/plone/app/users/browser/membersearch.py\n@@ -1,6 +1,9 @@\n from plone.autoform.form import AutoExtensibleForm\n from plone.base import PloneMessageFactory as _\n+from plone.memoize.view import memoize\n from plone.supermodel import model\n+from Products.CMFCore.permissions import ListPortalMembers\n+from Products.CMFCore.utils import getToolByName\n from Products.Five.browser.pagetemplatefile import ViewPageTemplateFile\n from z3c.form import button\n from z3c.form import form\n@@ -97,6 +100,32 @@ class MemberSearchForm(AutoExtensibleForm, form.Form):\n \n     submitted = False\n \n+    @property\n+    @memoize\n+    def listing_allowed(self):\n+        """\n+        Check if the user has the necessary "List Portal Members" permissions\n+        to view the list of search results.\n+        """\n+        pm = getToolByName(self.context, "portal_membership")\n+        return pm.checkPermission(ListPortalMembers, self.context)\n+\n+    @property\n+    def results(self):\n+        """\n+        Retrieve, merge, and sort the list of results based on search criteria.\n+\n+        This is based on the methods previously defined in the\n+        Products.PlonePAS.browser.search module.\n+        """\n+        # First of all check if we have the proper permissions\n+        if not self.listing_allowed:\n+            return []\n+\n+        view = self.context.restrictedTraverse("@@pas_search")\n+        criteria = extractCriteriaFromRequest(self.request.form.copy())\n+        return view.searchUsers(sort_by="fullname", **criteria)\n+\n     @button.buttonAndHandler(_("label_search", default="Search"), name="search")\n     def handleApply(self, action):\n         request = self.request\n@@ -108,7 +137,3 @@ def handleApply(self, action):\n \n         if request.get("form.buttons.search", None):\n             self.submitted = True\n-\n-            view = self.context.restrictedTraverse("@@pas_search")\n-            criteria = extractCriteriaFromRequest(self.request.form.copy())\n-            self.results = view.searchUsers(sort_by="fullname", **criteria)\ndiff --git a/plone/app/users/browser/membersearch_form.pt b/plone/app/users/browser/membersearch_form.pt\nindex 79ead56a..95c24814 100644\n--- a/plone/app/users/browser/membersearch_form.pt\n+++ b/plone/app/users/browser/membersearch_form.pt\n@@ -23,8 +23,8 @@\n       <tal:block condition="view/submitted">\n         <article id="content"\n                  tal:define="\n-                   listing_allowed python: checkPermission(\'List portal members\', here);\n-                   results python:listing_allowed and view.results;\n+                   listing_allowed python: view.listing_allowed;\n+                   results python: view.results;\n                    Batch python:modules[\'Products.CMFPlone\'].Batch;\n                    DateTime python:modules[\'DateTime\'].DateTime;\n                    b_size python:12;\ndiff --git a/setup.py b/setup.py\nindex 34844a08..12ad8f91 100644\n--- a/setup.py\n+++ b/setup.py\n@@ -71,6 +71,7 @@\n         "plone.base",\n         "plone.formwidget.namedfile >= 1.0.3",\n         "plone.i18n",\n+        "plone.memoize",\n         "plone.namedfile",\n         "plone.protect",\n         "plone.registry",\n'
-
-Repository: plone.app.users
-
-
-Branch: refs/heads/master
-Date: 2025-01-20T14:52:06+01:00
-Author: Alessandro Pisa (ale-rt) <[email protected]>
-Commit: https://github.com/plone/plone.app.users/commit/f8564fafbe47c2665a4218ac329b0c6ddc8d96f8
-
-Merge pull request #133 from plone/ale/bugfix/125
-
-The member-search form is now protected
-
-Files changed:
-A news/125.bugfix.rst
-M plone/app/users/browser/membersearch.py
-M plone/app/users/browser/membersearch_form.pt
-M setup.py
-
-b'diff --git a/news/125.bugfix.rst b/news/125.bugfix.rst\nnew file mode 100644\nindex 00000000..cf699cfb\n--- /dev/null\n+++ b/news/125.bugfix.rst\n@@ -0,0 +1,2 @@\n+The member-search logic has been moved from the page template to the Python code\n+[ale-rt]\ndiff --git a/plone/app/users/browser/membersearch.py b/plone/app/users/browser/membersearch.py\nindex 08d1bc23..5cc031bf 100644\n--- a/plone/app/users/browser/membersearch.py\n+++ b/plone/app/users/browser/membersearch.py\n@@ -1,6 +1,9 @@\n from plone.autoform.form import AutoExtensibleForm\n from plone.base import PloneMessageFactory as _\n+from plone.memoize.view import memoize\n from plone.supermodel import model\n+from Products.CMFCore.permissions import ListPortalMembers\n+from Products.CMFCore.utils import getToolByName\n from Products.Five.browser.pagetemplatefile import ViewPageTemplateFile\n from z3c.form import button\n from z3c.form import form\n@@ -97,6 +100,32 @@ class MemberSearchForm(AutoExtensibleForm, form.Form):\n \n     submitted = False\n \n+    @property\n+    @memoize\n+    def listing_allowed(self):\n+        """\n+        Check if the user has the necessary "List Portal Members" permissions\n+        to view the list of search results.\n+        """\n+        pm = getToolByName(self.context, "portal_membership")\n+        return pm.checkPermission(ListPortalMembers, self.context)\n+\n+    @property\n+    def results(self):\n+        """\n+        Retrieve, merge, and sort the list of results based on search criteria.\n+\n+        This is based on the methods previously defined in the\n+        Products.PlonePAS.browser.search module.\n+        """\n+        # First of all check if we have the proper permissions\n+        if not self.listing_allowed:\n+            return []\n+\n+        view = self.context.restrictedTraverse("@@pas_search")\n+        criteria = extractCriteriaFromRequest(self.request.form.copy())\n+        return view.searchUsers(sort_by="fullname", **criteria)\n+\n     @button.buttonAndHandler(_("label_search", default="Search"), name="search")\n     def handleApply(self, action):\n         request = self.request\n@@ -108,7 +137,3 @@ def handleApply(self, action):\n \n         if request.get("form.buttons.search", None):\n             self.submitted = True\n-\n-            view = self.context.restrictedTraverse("@@pas_search")\n-            criteria = extractCriteriaFromRequest(self.request.form.copy())\n-            self.results = view.searchUsers(sort_by="fullname", **criteria)\ndiff --git a/plone/app/users/browser/membersearch_form.pt b/plone/app/users/browser/membersearch_form.pt\nindex 79ead56a..95c24814 100644\n--- a/plone/app/users/browser/membersearch_form.pt\n+++ b/plone/app/users/browser/membersearch_form.pt\n@@ -23,8 +23,8 @@\n       <tal:block condition="view/submitted">\n         <article id="content"\n                  tal:define="\n-                   listing_allowed python: checkPermission(\'List portal members\', here);\n-                   results python:listing_allowed and view.results;\n+                   listing_allowed python: view.listing_allowed;\n+                   results python: view.results;\n                    Batch python:modules[\'Products.CMFPlone\'].Batch;\n                    DateTime python:modules[\'DateTime\'].DateTime;\n                    b_size python:12;\ndiff --git a/setup.py b/setup.py\nindex 34844a08..12ad8f91 100644\n--- a/setup.py\n+++ b/setup.py\n@@ -71,6 +71,7 @@\n         "plone.base",\n         "plone.formwidget.namedfile >= 1.0.3",\n         "plone.i18n",\n+        "plone.memoize",\n         "plone.namedfile",\n         "plone.protect",\n         "plone.registry",\n'
+A news/1858.bugfix
+M src/plone/restapi/batching.py
+M src/plone/restapi/deserializer/__init__.py
+M src/plone/restapi/services/discussion/conversation.py
+M src/plone/restapi/services/navigation/get.py
+M src/plone/restapi/services/querystringsearch/get.py
+M src/plone/restapi/tests/test_batching.py
+M src/plone/restapi/tests/test_services_navigation.py
+
+b'diff --git a/news/1858.bugfix b/news/1858.bugfix\nnew file mode 100644\nindex 0000000000..b275567243\n--- /dev/null\n+++ b/news/1858.bugfix\n@@ -0,0 +1 @@\n+Add parse_int to handle all cases of BadRequests from ints passed in. @djay\n\\ No newline at end of file\ndiff --git a/src/plone/restapi/batching.py b/src/plone/restapi/batching.py\nindex 30653c43c3..7d9bec01ba 100644\n--- a/src/plone/restapi/batching.py\n+++ b/src/plone/restapi/batching.py\n@@ -1,5 +1,6 @@\n from plone.batching.batch import Batch\n from plone.restapi.deserializer import json_body\n+from plone.restapi.deserializer import parse_int\n from plone.restapi.exceptions import DeserializationError\n from urllib.parse import parse_qsl\n from urllib.parse import urlencode\n@@ -14,14 +15,15 @@ def __init__(self, request, results):\n         self.request = request\n \n         try:\n-            self.b_start = int(json_body(self.request).get("b_start", False)) or int(\n-                self.request.form.get("b_start", 0)\n-            )\n-            self.b_size = int(json_body(self.request).get("b_size", False)) or int(\n-                self.request.form.get("b_size", DEFAULT_BATCH_SIZE)\n-            )\n-        except (ValueError, DeserializationError) as e:\n+            data = json_body(request)\n+        except DeserializationError as e:\n             raise BadRequest(e)\n+        self.b_start = parse_int(data, "b_start", False) or parse_int(\n+            self.request.form, "b_start", 0\n+        )\n+        self.b_size = parse_int(data, "b_size", False) or parse_int(\n+            self.request.form, "b_size", DEFAULT_BATCH_SIZE\n+        )\n         self.batch = Batch(results, self.b_size, self.b_start)\n \n     def __iter__(self):\ndiff --git a/src/plone/restapi/deserializer/__init__.py b/src/plone/restapi/deserializer/__init__.py\nindex a5112dae93..0096235c67 100644\n--- a/src/plone/restapi/deserializer/__init__.py\n+++ b/src/plone/restapi/deserializer/__init__.py\n@@ -1,4 +1,5 @@\n from plone.restapi.exceptions import DeserializationError\n+from zExceptions import BadRequest\n \n import json\n \n@@ -28,3 +29,19 @@ def boolean_value(value):\n \n     """\n     return value not in {False, "false", "False", "0", 0}\n+\n+\n+def parse_int(data, prop, default):\n+    """\n+    Args:\n+        data: dict from a request\n+        prop: name of a integer paramater in the dict\n+        default: default if not found\n+\n+    Returns: an integer\n+    Raises: BadRequest if not an int\n+    """\n+    try:\n+        return int(data.get(prop, default))\n+    except (ValueError, TypeError):\n+        raise BadRequest(f"Invalid {prop}: Not an integer")\ndiff --git a/src/plone/restapi/services/discussion/conversation.py b/src/plone/restapi/services/discussion/conversation.py\nindex 071d75b04d..36b0b5b860 100644\n--- a/src/plone/restapi/services/discussion/conversation.py\n+++ b/src/plone/restapi/services/discussion/conversation.py\n@@ -2,7 +2,7 @@\n from plone.app.discussion.browser.comment import EditCommentForm\n from plone.app.discussion.browser.comments import CommentForm\n from plone.app.discussion.interfaces import IConversation\n-from plone.restapi.deserializer import json_body\n+from plone.restapi.deserializer import json_body, parse_int\n from plone.restapi.interfaces import ISerializeToJson\n from plone.restapi.services import Service\n from plone.restapi.services.discussion.utils import can_delete\n@@ -38,7 +38,7 @@ class CommentsGet(Service):\n \n     def publishTraverse(self, request, name):\n         if name:\n-            self.comment_id = int(name)\n+            self.comment_id = parse_int(dict(comment_id=name), "comment_id", None)\n         return self\n \n     def reply(self):\n@@ -57,7 +57,7 @@ class CommentsAdd(Service):\n \n     def publishTraverse(self, request, name):\n         if name:\n-            self.comment_id = int(name)\n+            self.comment_id = parse_int(dict(comment_id=name), "comment_id", None)\n             request["form.widgets.in_reply_to"] = name\n         return self\n \n@@ -96,7 +96,7 @@ class CommentsUpdate(Service):\n \n     def publishTraverse(self, request, name):\n         if name:\n-            self.comment_id = int(name)\n+            self.comment_id = parse_int(dict(comment_id=name), "comment_id", None)\n             request["form.widgets.comment_id"] = name\n         return self\n \n@@ -140,7 +140,7 @@ class CommentsDelete(Service):\n     comment_id = None\n \n     def publishTraverse(self, request, name):\n-        self.comment_id = int(name)\n+        self.comment_id = parse_int(dict(comment_id=name), "comment_id", None)\n         return self\n \n     def reply(self):\ndiff --git a/src/plone/restapi/services/navigation/get.py b/src/plone/restapi/services/navigation/get.py\nindex de9f91ed6b..96748f336f 100644\n--- a/src/plone/restapi/services/navigation/get.py\n+++ b/src/plone/restapi/services/navigation/get.py\n@@ -6,6 +6,7 @@\n from plone.registry.interfaces import IRegistry\n from plone.restapi.bbb import INavigationSchema\n from plone.restapi.bbb import safe_text\n+from plone.restapi.deserializer import parse_int\n from plone.restapi.interfaces import IExpandableElement\n from plone.restapi.serializer.converters import json_compatible\n from plone.restapi.services import Service\n@@ -17,7 +18,6 @@\n from zope.i18n import translate\n from zope.interface import implementer\n from zope.interface import Interface\n-from zExceptions import BadRequest\n \n \n @implementer(IExpandableElement)\n@@ -29,13 +29,7 @@ def __init__(self, context, request):\n         self.portal = getSite()\n \n     def __call__(self, expand=False):\n-        if self.request.form.get("expand.navigation.depth", False):\n-            try:\n-                self.depth = int(self.request.form["expand.navigation.depth"])\n-            except (ValueError, TypeError) as e:\n-                raise BadRequest(e)\n-        else:\n-            self.depth = 1\n+        self.depth = parse_int(self.request.form, "expand.navigation.depth", 1)\n \n         result = {"navigation": {"@id": f"{self.context.absolute_url()}/@navigation"}}\n         if not expand:\ndiff --git a/src/plone/restapi/services/querystringsearch/get.py b/src/plone/restapi/services/querystringsearch/get.py\nindex 9b3c0e60c7..5e2bfee45d 100644\n--- a/src/plone/restapi/services/querystringsearch/get.py\n+++ b/src/plone/restapi/services/querystringsearch/get.py\n@@ -2,6 +2,7 @@\n from pkg_resources import parse_version\n from plone.restapi.bbb import IPloneSiteRoot\n from plone.restapi.deserializer import json_body\n+from plone.restapi.deserializer import parse_int\n from plone.restapi.exceptions import DeserializationError\n from plone.restapi.interfaces import ISerializeToJson\n from plone.restapi.services import Service\n@@ -31,20 +32,12 @@ def __call__(self):\n             raise BadRequest(str(err))\n \n         query = data.get("query", None)\n-        try:\n-            b_start = int(data.get("b_start", 0))\n-        except (ValueError, TypeError):\n-            raise BadRequest("Invalid b_start")\n-        try:\n-            b_size = int(data.get("b_size", 25))\n-        except (ValueError, TypeError):\n-            raise BadRequest("Invalid b_size")\n+\n+        b_start = parse_int(data, "b_start", 0)\n+        b_size = parse_int(data, "b_size", 25)\n         sort_on = data.get("sort_on", None)\n         sort_order = data.get("sort_order", None)\n-        try:\n-            limit = int(data.get("limit", 1000))\n-        except (ValueError, TypeError):\n-            raise BadRequest("Invalid limit")\n+        limit = parse_int(data, "limit", 1000)\n         fullobjects = bool(data.get("fullobjects", False))\n \n         if not query:\ndiff --git a/src/plone/restapi/tests/test_batching.py b/src/plone/restapi/tests/test_batching.py\nindex 4470b2c8f6..a1b4414e38 100644\n--- a/src/plone/restapi/tests/test_batching.py\n+++ b/src/plone/restapi/tests/test_batching.py\n@@ -188,7 +188,11 @@ def test_batching_links_omitted_if_resulset_fits_in_single_batch(self):\n     def test_batching_badrequests(self):\n         response = self.api_session.get("/collection?b_size=php")\n         self.assertEqual(response.status_code, 400)\n-        self.assertIn("invalid literal for int()", response.json()["message"])\n+        self.assertIn("Invalid b_size", response.json()["message"])\n+\n+        response = self.api_session.get("/collection?b_size:list=1")\n+        self.assertEqual(response.status_code, 400)\n+        self.assertIn("Invalid b_size", response.json()["message"])\n \n \n class TestBatchingDXFolders(TestBatchingDXBase):\ndiff --git a/src/plone/restapi/tests/test_services_navigation.py b/src/plone/restapi/tests/test_services_navigation.py\nindex a61009005c..c0bc6149f4 100644\n--- a/src/plone/restapi/tests/test_services_navigation.py\n+++ b/src/plone/restapi/tests/test_services_navigation.py\n@@ -246,4 +246,4 @@ def test_navigation_badrequests(self):\n         )\n \n         self.assertEqual(response.status_code, 400)\n-        self.assertIn("invalid literal for int()", response.json()["message"])\n+        self.assertIn("Invalid expand.navigation.depth", response.json()["message"])\n'