From 7939264c4609a9dd3c3d12151c91e74a1bc46f0a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20P=C3=A9ters?= <fpeters@entrouvert.com>
Date: Sat, 8 Dec 2018 08:25:24 +0100
Subject: [PATCH] api: check limit/offset parameters are valid (#28773)

---
 tests/test_api.py            | 16 ++++++++++++++++
 wcs/api.py                   | 14 ++++++++++----
 wcs/backoffice/management.py | 10 ++++++++--
 3 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/tests/test_api.py b/tests/test_api.py
index f6e0d73d5..4d4a08e1e 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -1550,6 +1550,10 @@ def test_api_list_formdata(pub, local_user):
         resp_partial_ids.extend([x.get('id') for x in resp.json])
     assert resp_all_ids == resp_partial_ids
 
+    # check error handling
+    get_app(pub).get(sign_uri('/api/forms/test/list?filter=all&offset=plop', user=local_user), status=400)
+    get_app(pub).get(sign_uri('/api/forms/test/list?filter=all&limit=plop', user=local_user), status=400)
+
 def test_api_anonymized_formdata(pub, local_user, admin_user):
     Role.wipe()
     role = Role(name='test')
@@ -1845,6 +1849,18 @@ def test_api_global_listing(pub, local_user):
     resp = get_app(pub).get(sign_uri('/api/forms/?status=done', user=local_user))
     assert len(resp.json['data']) == 20
 
+    # check limit/offset
+    resp = get_app(pub).get(sign_uri('/api/forms/?status=done&limit=5', user=local_user))
+    assert len(resp.json['data']) == 5
+    resp = get_app(pub).get(sign_uri('/api/forms/?status=done&offset=5&limit=5', user=local_user))
+    assert len(resp.json['data']) == 5
+    resp = get_app(pub).get(sign_uri('/api/forms/?status=done&offset=18&limit=5', user=local_user))
+    assert len(resp.json['data']) == 2
+
+    # check error handling
+    get_app(pub).get(sign_uri('/api/forms/?status=done&limit=plop', user=local_user), status=400)
+    get_app(pub).get(sign_uri('/api/forms/?status=done&offset=plop', user=local_user), status=400)
+
 def test_api_global_listing_ignored_roles(pub, local_user):
     test_api_global_listing(pub, local_user)
 
diff --git a/wcs/api.py b/wcs/api.py
index b5a766b86..eccb67f3b 100644
--- a/wcs/api.py
+++ b/wcs/api.py
@@ -28,7 +28,7 @@ from qommon import _
 from qommon import misc
 from qommon.evalutils import make_datetime
 from qommon.errors import (AccessForbiddenError, QueryError, TraversalError,
-    UnknownNameIdAccessForbiddenError)
+    UnknownNameIdAccessForbiddenError, RequestError)
 from qommon.form import ComputedExpressionWidget, ConditionWidget
 
 from wcs.categories import Category
@@ -213,9 +213,15 @@ class ApiFormsDirectory(Directory):
             roles_criterias = criterias
             criterias = management_directory.get_global_listing_criterias(ignore_user_roles=True)
 
-        limit = int(get_request().form.get('limit',
-            get_publisher().get_site_option('default-page-size') or 20))
-        offset = int(get_request().form.get('offset', 0))
+        try:
+            limit = int(get_request().form.get('limit',
+                get_publisher().get_site_option('default-page-size') or 20))
+        except ValueError:
+            raise RequestError('invalid limit parameter')
+        try:
+            offset = int(get_request().form.get('offset', 0))
+        except ValueError:
+            raise RequestError('invalid offset parameter')
         order_by = get_request().form.get('order_by',
             get_publisher().get_site_option('default-sort-order') or '-receipt_time')
 
diff --git a/wcs/backoffice/management.py b/wcs/backoffice/management.py
index 09734b200..1a7cefe36 100644
--- a/wcs/backoffice/management.py
+++ b/wcs/backoffice/management.py
@@ -1610,10 +1610,16 @@ class FormPage(Directory):
         query = get_request().form.get('q') if not anonymise else None
         offset = None
         if 'offset' in get_request().form:
-            offset = int(get_request().form['offset'])
+            try:
+                offset = int(get_request().form['offset'])
+            except ValueError:
+                raise errors.RequestError('invalid offset parameter')
         limit = None
         if 'limit' in get_request().form:
-            limit = int(get_request().form['limit'])
+            try:
+                limit = int(get_request().form['limit'])
+            except ValueError:
+                raise errors.RequestError('invalid limit parameter')
         items, total_count = FormDefUI(self.formdef).get_listing_items(
             selected_filter, user=user, query=query, criterias=criterias,
             order_by=order_by, anonymise=anonymise, offset=offset, limit=limit)
-- 
2.20.0.rc2