Ensure page parameters are valid.

Fixes #2042 by ensuring that:

- Invalid numbers aren't passed through to logic layer or paginator
- There is a default should an invalid number is passed.

Adds a _get_page_number() to the BaseController for re-use in all the
subclasses.

ckan/controllers/feed.py

@@ -306,12 +306,7 @@ def custom(self):
                 search_params[param] = value
                 fq += ' %s:"%s"' % (param, value)
 
-        try:
-            page = int(request.params.get('page', 1))
-        except ValueError:
-            base.abort(400, _('"page" parameter must be a positive integer'))
-        if page < 0:
-            base.abort(400, _('"page" parameter must be a positive integer'))
+        page = self._get_page_number(request.params)
 
         limit = ITEMS_LIMIT
         data_dict = {
@@ -456,12 +451,7 @@ def _parse_url_params(self):
         Returns the constructed search-query dict, and the valid URL
         query parameters.
         """
-        try:
-            page = int(request.params.get('page', 1)) or 1
-        except ValueError:
-            base.abort(400, _('"page" parameter must be a positive integer'))
-        if page < 0:
-            base.abort(400, _('"page" parameter must be a positive integer'))
+        page = self._get_page_number(request.params)
 
         limit = ITEMS_LIMIT
         data_dict = {

ckan/controllers/group.py

@@ -160,7 +160,7 @@ def index(self):
 
         c.page = h.Page(
             collection=results,
-            page=request.params.get('page', 1),
+            page = self._get_page_number(request.params),
             url=h.pager_url,
             items_per_page=21
         )
@@ -217,10 +217,7 @@ def _read(self, id, limit):
         # if we drop support for those then we can delete this line.
         c.group_admins = new_authz.get_group_or_org_admin_ids(c.group.id)
 
-        try:
-            page = int(request.params.get('page', 1))
-        except ValueError, e:
-            abort(400, ('"page" parameter must be an integer'))
+        page = self._get_page_number(request.params)
 
         # most search operations should reset the page counter:
         params_nopage = [(k, v) for k, v in request.params.items()

ckan/controllers/package.py

@@ -149,10 +149,8 @@ def search(self):
         # unicode format (decoded from utf8)
         q = c.q = request.params.get('q', u'')
         c.query_error = False
-        try:
-            page = int(request.params.get('page', 1))
-        except ValueError, e:
-            abort(400, ('"page" parameter must be an integer'))
+        page = self._get_page_number(request.params)
+
         limit = g.datasets_per_page
 
         # most search operations should reset the page counter:

ckan/controllers/related.py

@@ -34,10 +34,8 @@ def dashboard(self):
 
         params_nopage = [(k, v) for k, v in base.request.params.items()
                          if k != 'page']
-        try:
-            page = int(base.request.params.get('page', 1))
-        except ValueError:
-            base.abort(400, ('"page" parameter must be an integer'))
+
+        page = self._get_page_number(request.params)
 
         # Update ordering in the context
         related_list = logic.get_action('related_list')(context, data_dict)

ckan/controllers/revision.py

@@ -122,7 +122,7 @@ def list(self):
             query = model.Session.query(model.Revision)
             c.page = h.Page(
                 collection=query,
-                page=request.params.get('page', 1),
+                page=self._get_page_number(request.params),
                 url=h.pager_url,
                 items_per_page=20
             )

ckan/controllers/tag.py

@@ -32,7 +32,7 @@ def index(self):
         data_dict = {'all_fields': True}
 
         if c.q:
-            page = int(request.params.get('page', 1))
+            page = self._get_page_number(request.params)
             data_dict['q'] = c.q
             data_dict['limit'] = LIMIT
             data_dict['offset'] = (page - 1) * LIMIT

ckan/controllers/user.py

@@ -85,7 +85,7 @@ def _get_repoze_handler(self, handler_name):
     def index(self):
         LIMIT = 20
 
-        page = int(request.params.get('page', 1))
+        page = self._get_page_number(request.params)
         c.q = request.params.get('q', '')
         c.order_by = request.params.get('order_by', 'name')
 

ckan/lib/alphabet_paginate.py

@@ -47,7 +47,7 @@ def __init__(self, collection, alpha_attribute, page, other_text, paging_thresho
         self.controller_name = controller_name
 
         self.letters = [char for char in 'ABCDEFGHIJKLMNOPQRSTUVWXYZ'] + [self.other_text]
-        
+
         # Work out which alphabet letters are 'available' i.e. have some results
         # because we grey-out those which aren't.
         self.available = dict( (c,0,) for c in self.letters )

ckan/lib/base.py

@@ -421,6 +421,24 @@ def _get_user_for_apikey(self):
         user = query.filter_by(apikey=apikey).first()
         return user
 
+    def _get_page_number(self, params, key='page', default=1):
+        """
+        Returns the page number from the provided params after
+        verifies that it is an integer.
+
+        If it fails it will return the default value.
+        """
+        p = params.get(key, default)
+
+        try:
+            p = int(p)
+            if p < 1:
+                p = default
+        except:
+            p = default
+
+        return p
+
 
 # Include the '_' function in the public names
 __all__ = [__name for __name in locals().keys() if not __name.startswith('_')

ckan/tests/functional/test_group.py

@@ -54,8 +54,10 @@ def test_atom_feed_page_negative(self):
         offset = url_for(controller='feed', action='group',
                          id=group_name)
         offset = offset + '?page=-2'
-        res = self.app.get(offset, expect_errors=True)
-        assert '"page" parameter must be a positive integer' in res, res
+        res = self.app.get(offset, expect_errors=False)
+        assert '<feed' in res, res
+        assert 'xmlns="http://www.w3.org/2005/Atom"' in res, res
+        assert '</feed>' in res, res
 
     def test_sorting(self):
         model.repo.rebuild_db()