[allura] 02/41: misc: avoid filter=foo erroring

[br...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

brondsem pushed a commit to branch db/8349
in repository https://gitbox.apache.org/repos/asf/allura.git

commit f543c2bc49bdce24f51592d445d4b052ebb50e91
Author: Dave Brondsema <da...@brondsema.net>
AuthorDate: Mon Feb 10 11:43:01 2020 -0500

    misc: avoid filter=foo erroring
---
 Allura/allura/lib/validators.py                         | 7 ++++++-
 Allura/allura/tests/test_validators.py                  | 2 ++
 ForgeTracker/forgetracker/model/ticket.py               | 3 ++-
 ForgeTracker/forgetracker/tests/functional/test_root.py | 3 +++
 ForgeTracker/forgetracker/tracker_main.py               | 6 +-----
 5 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/Allura/allura/lib/validators.py b/Allura/allura/lib/validators.py
index 7bcbbb4..2041dcb 100644
--- a/Allura/allura/lib/validators.py
+++ b/Allura/allura/lib/validators.py
@@ -257,13 +257,18 @@ class JsonValidator(fev.FancyValidator):
 
 class JsonConverter(fev.FancyValidator):
 
-    """Deserializes a string to JSON and returns a Python object"""
+    """
+    Deserializes a string to JSON and returns a Python object
+    Must be an object, not a simple literal
+    """
 
     def _to_python(self, value, state):
         try:
             obj = json.loads(value)
         except ValueError, e:
             raise fe.Invalid('Invalid JSON: ' + str(e), value, state)
+        if not isinstance(obj, dict):
+            raise fe.Invalid('Not a dict (JSON object)', value, state)
         return obj
 
 
diff --git a/Allura/allura/tests/test_validators.py b/Allura/allura/tests/test_validators.py
index 3f9d27a..4d75135 100644
--- a/Allura/allura/tests/test_validators.py
+++ b/Allura/allura/tests/test_validators.py
@@ -45,6 +45,8 @@ class TestJsonConverter(unittest.TestCase):
     def test_invalid(self):
         with self.assertRaises(fe.Invalid):
             self.val.to_python('{')
+        with self.assertRaises(fe.Invalid):
+            self.val.to_python('3')
 
 
 class TestJsonFile(unittest.TestCase):
diff --git a/ForgeTracker/forgetracker/model/ticket.py b/ForgeTracker/forgetracker/model/ticket.py
index 6824e42..e33ec35 100644
--- a/ForgeTracker/forgetracker/model/ticket.py
+++ b/ForgeTracker/forgetracker/model/ticket.py
@@ -1261,7 +1261,8 @@ class Ticket(VersionedArtifact, ActivityObject, VotableArtifact):
         limit, page, start = g.handle_paging(limit, page, default=25)
         count = 0
         tickets = []
-        if filter is None: filter = {}
+        if filter is None:
+            filter = {}
         refined_sort = sort if sort else 'ticket_num_i desc'
         if 'ticket_num_i' not in refined_sort:
             refined_sort += ',ticket_num_i asc'
diff --git a/ForgeTracker/forgetracker/tests/functional/test_root.py b/ForgeTracker/forgetracker/tests/functional/test_root.py
index 8c50d0d..0428654 100644
--- a/ForgeTracker/forgetracker/tests/functional/test_root.py
+++ b/ForgeTracker/forgetracker/tests/functional/test_root.py
@@ -1381,6 +1381,9 @@ class TestFunctionalController(TrackerTestController):
         assert '3 results' in response, response.showbrowser()
         assert 'test third ticket' in response, response.showbrowser()
 
+        # 'filter' is special kwarg, don't let it cause problems
+        r = self.app.get('/p/test/bugs/search/?q=test&filter=blah')
+
     def test_search_with_strange_chars(self):
         r = self.app.get('/p/test/bugs/search/?' +
                          urlencode({'q': 'tést'}))
diff --git a/ForgeTracker/forgetracker/tracker_main.py b/ForgeTracker/forgetracker/tracker_main.py
index 50c5db8..ddd836b 100644
--- a/ForgeTracker/forgetracker/tracker_main.py
+++ b/ForgeTracker/forgetracker/tracker_main.py
@@ -92,7 +92,7 @@ search_validators = dict(
     limit=validators.Int(if_invalid=None),
     page=validators.Int(if_empty=0, if_invalid=0),
     sort=validators.UnicodeString(if_empty=None),
-    filter=V.JsonConverter(if_empty={}),
+    filter=V.JsonConverter(if_empty={}, if_invalid={}),
     deleted=validators.StringBool(if_empty=False))
 
 
@@ -714,10 +714,6 @@ class RootController(BaseController, FeedController):
         elif deleted and not has_access(c.app, 'delete'):
             deleted = False
 
-        if not isinstance(filter, dict):
-            # JsonConverter above can return an int, string, etc, if users give bad inputs, but it needs to be a dict
-            filter = {}
-
         # it's just our original query mangled and sent back to us
         kw.pop('q', None)
         result = TM.Ticket.paged_query_or_search(c.app.config, c.user,