Handle invalid pagination parameters

The api checks sort_dir, sort_key and limit.
Incorrect sort_dir throws InvalidSortDir.
Incorrect sort key throws InvalidSortKey.
Incorrect limit throws InvalidLimit

Storage checks for marker.
Incorrect marker results in InvalidMarker.

Closes-Bug: 1282682
Change-Id: I993c82479cace5a285fdba480fbd8bccb53a0aeb

11 files changed, 137 insertions, 12 deletions

diff --git a/designate/api/v2/controllers/blacklists.py b/designate/api/v2/controllers/blacklists.py
index 4ea41189..8da31d2f 100644
--- a/designate/api/v2/controllers/blacklists.py
+++ b/designate/api/v2/controllers/blacklists.py
@@ -30,6 +30,7 @@ class BlacklistsController(rest.RestController):
     _view = blacklists_view.BlacklistsView()
     _resource_schema = schema.Schema('v2', 'blacklist')
     _collection_schema = schema.Schema('v2', 'blacklists')
+    SORT_KEYS = ['created_at', 'id', 'updated_at', 'pattern']
 
     @pecan.expose(template='json:', content_type='application/json')
     def get_one(self, blacklist_id):
diff --git a/designate/api/v2/controllers/records.py b/designate/api/v2/controllers/records.py
index de132dd8..a0042667 100644
--- a/designate/api/v2/controllers/records.py
+++ b/designate/api/v2/controllers/records.py
@@ -29,6 +29,8 @@ class RecordsController(rest.RestController):
     _view = records_view.RecordsView()
     _resource_schema = schema.Schema('v2', 'record')
     _collection_schema = schema.Schema('v2', 'records')
+    SORT_KEYS = ['created_at', 'id', 'updated_at', 'domain_id', 'tenant_id',
+                 'recordset_id', 'status']
 
     @pecan.expose(template='json:', content_type='application/json')
     def get_one(self, zone_id, recordset_id, record_id):
diff --git a/designate/api/v2/controllers/recordsets.py b/designate/api/v2/controllers/recordsets.py
index 635821b5..7d42fd9f 100644
--- a/designate/api/v2/controllers/recordsets.py
+++ b/designate/api/v2/controllers/recordsets.py
@@ -30,6 +30,8 @@ class RecordSetsController(rest.RestController):
     _view = recordsets_view.RecordSetsView()
     _resource_schema = schema.Schema('v2', 'recordset')
     _collection_schema = schema.Schema('v2', 'recordsets')
+    SORT_KEYS = ['created_at', 'id', 'updated_at', 'domain_id', 'tenant_id',
+                 'name', 'type', 'ttl']
 
     records = records.RecordsController()
 
diff --git a/designate/api/v2/controllers/rest.py b/designate/api/v2/controllers/rest.py
index d72d9464..84b1d936 100644
--- a/designate/api/v2/controllers/rest.py
+++ b/designate/api/v2/controllers/rest.py
@@ -24,11 +24,14 @@
 # ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
 # (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
 # SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+import six
 import inspect
 import pecan
 import pecan.rest
 import pecan.routing
+from designate import exceptions
 from designate.openstack.common import log as logging
+from designate.openstack.common.gettextutils import _
 
 LOG = logging.getLogger(__name__)
 
@@ -41,6 +44,9 @@ class RestController(pecan.rest.RestController):
     Ideally, we get these additions merged upstream.
     """
 
+    # default sort_keys.  The Controllers can override this.
+    SORT_KEYS = ['created_at', 'id']
+
     def _get_paging_params(self, params):
         """
         Extract any paging parameters
@@ -49,6 +55,34 @@ class RestController(pecan.rest.RestController):
         limit = params.pop('limit', None)
         sort_key = params.pop('sort_key', None)
         sort_dir = params.pop('sort_dir', None)
+
+        # Negative and zero limits are not caught in storage.
+        # With a number bigger than MAXSIZE, rpc throws an 'OverflowError long
+        #  too big to convert'.
+        # So the parameter 'limit' is checked here.
+        if limit:
+            try:
+                invalid_limit_message = _(str.format(
+                    'limit should be an integer between 1 and {0}',
+                    six.MAXSIZE))
+                int_limit = int(limit)
+                if int_limit <= 0 or int_limit > six.MAXSIZE:
+                    raise exceptions.InvalidLimit(invalid_limit_message)
+            # This exception is raised for non ints when int(limit) is called
+            except ValueError:
+                raise exceptions.InvalidLimit(invalid_limit_message)
+
+        # sort_dir is checked in paginate_query.
+        # We duplicate the sort_dir check here to throw a more specific
+        # exception than ValueError.
+        if sort_dir and sort_dir not in ['asc', 'desc']:
+            raise exceptions.InvalidSortDir(_("Unknown sort direction, "
+                                              "must be 'desc' or 'asc'"))
+
+        if sort_key and sort_key not in self.SORT_KEYS:
+            raise exceptions.InvalidSortKey(_(str.format(
+                'sort key must be one of {0}', str(self.SORT_KEYS))))
+
         return marker, limit, sort_key, sort_dir
 
     def _handle_post(self, method, remainder):
diff --git a/designate/api/v2/controllers/tlds.py b/designate/api/v2/controllers/tlds.py
index f05cd3b4..ed66a9b5 100644
--- a/designate/api/v2/controllers/tlds.py
+++ b/designate/api/v2/controllers/tlds.py
@@ -28,6 +28,7 @@ class TldsController(rest.RestController):
     _view = tlds_view.TldsView()
     _resource_schema = schema.Schema('v2', 'tld')
     _collection_schema = schema.Schema('v2', 'tlds')
+    SORT_KEYS = ['created_at', 'id', 'updated_at', 'name']
 
     @pecan.expose(template='json:', content_type='application/json')
     def get_one(self, tld_id):
diff --git a/designate/api/v2/controllers/zones.py b/designate/api/v2/controllers/zones.py
index 8196c703..9bb2f135 100644
--- a/designate/api/v2/controllers/zones.py
+++ b/designate/api/v2/controllers/zones.py
@@ -34,6 +34,8 @@ class ZonesController(rest.RestController):
     _view = zones_view.ZonesView()
     _resource_schema = schema.Schema('v2', 'zone')
     _collection_schema = schema.Schema('v2', 'zones')
+    SORT_KEYS = ['created_at', 'id', 'updated_at', 'name', 'tenant_id',
+                 'serial', 'ttl']
 
     recordsets = recordsets.RecordSetsController()
 
diff --git a/designate/exceptions.py b/designate/exceptions.py
index 6e8169a9..6571714b 100644
--- a/designate/exceptions.py
+++ b/designate/exceptions.py
@@ -91,6 +91,26 @@ class MarkerNotFound(BadRequest):
     error_type = 'marker_not_found'
 
 
+class ValueError(BadRequest):
+    error_type = 'value_error'
+
+
+class InvalidMarker(BadRequest):
+    error_type = 'invalid_marker'
+
+
+class InvalidSortDir(BadRequest):
+    error_type = 'invalid_sort_dir'
+
+
+class InvalidLimit(BadRequest):
+    error_type = 'invalid_limit'
+
+
+class InvalidSortKey(BadRequest):
+    error_type = 'invalid_sort_key'
+
+
 class InvalidOperation(BadRequest):
     error_code = 400
     error_type = 'invalid_operation'
diff --git a/designate/storage/impl_sqlalchemy/__init__.py b/designate/storage/impl_sqlalchemy/__init__.py
index a8a86e3c..687aa9ad 100644
--- a/designate/storage/impl_sqlalchemy/__init__.py
+++ b/designate/storage/impl_sqlalchemy/__init__.py
@@ -15,10 +15,12 @@
 # under the License.
 import time
 from sqlalchemy.orm import exc
+from sqlalchemy import exc as sqlalchemy_exc
 from sqlalchemy import distinct, func
 from oslo.config import cfg
 from designate.openstack.common import log as logging
 from designate.openstack.common.db.sqlalchemy.utils import paginate_query
+from designate.openstack.common.db.sqlalchemy.utils import InvalidSortKey
 from designate import exceptions
 from designate.storage import base
 from designate.storage.impl_sqlalchemy import models
@@ -116,7 +118,7 @@ class SQLAlchemyStorage(base.Storage):
                 raise exceptions.NotFound()
         else:
             # If marker is not none and basestring we query it.
-            # Othwewise, return all matching records
+            # Otherwise, return all matching records
             if marker is not None:
                 try:
                     marker = self._find(model, context, {'id': marker},
@@ -124,15 +126,27 @@ class SQLAlchemyStorage(base.Storage):
                 except exceptions.NotFound:
                     raise exceptions.MarkerNotFound(
                         'Marker %s could not be found' % marker)
+                # Malformed UUIDs return StatementError
+                except sqlalchemy_exc.StatementError as statement_error:
+                    raise exceptions.InvalidMarker(statement_error.message)
             sort_key = sort_key or 'created_at'
             sort_dir = sort_dir or 'asc'
 
-            query = paginate_query(
-                query, model, limit,
-                [sort_key, 'id', 'created_at'], marker=marker,
-                sort_dir=sort_dir)
-
-            return query.all()
+            try:
+                query = paginate_query(
+                    query, model, limit,
+                    [sort_key, 'id', 'created_at'], marker=marker,
+                    sort_dir=sort_dir)
+
+                return query.all()
+            except InvalidSortKey as sort_key_error:
+                raise exceptions.InvalidSortKey(sort_key_error.message)
+            # Any ValueErrors are propagated back to the user as is.
+            # Limits, sort_dir and sort_key are checked at the API layer.
+            # If however central or storage is called directly, invalid values
+            # show up as ValueError
+            except ValueError as value_error:
+                raise exceptions.ValueError(value_error.message)
 
     ## CRUD for our resources (quota, server, tsigkey, tenant, domain & record)
     ## R - get_*, find_*s
diff --git a/designate/tests/test_api/test_v2/__init__.py b/designate/tests/test_api/test_v2/__init__.py
index 1e549f26..fe46c291 100644
--- a/designate/tests/test_api/test_v2/__init__.py
+++ b/designate/tests/test_api/test_v2/__init__.py
@@ -53,17 +53,25 @@ class ApiV2TestCase(ApiTestCase):
 
         super(ApiV2TestCase, self).tearDown()
 
-    def _assert_paging(self, data, url, key=None, limit=5):
+    def _assert_paging(self, data, url, key=None, limit=5, sort_dir='asc',
+                       sort_key='created_at', marker=None, status=200):
         def _page(marker=None):
-            params = {'limit': limit}
+            params = {'limit': limit,
+                      'sort_dir': sort_dir,
+                      'sort_key': sort_key}
 
             if marker is not None:
                 params['marker'] = marker
 
-            r = self.client.get(url, params)
-            return r.json[key] if key in r.json else r.json
+            r = self.client.get(url, params, status=status)
+            if status != 200:
+                return r
+            else:
+                return r.json[key] if key in r.json else r.json
 
-        page_items = _page()
+        page_items = _page(marker=marker)
+        if status != 200:
+            return page_items
 
         x = 0
         length = len(data)
diff --git a/designate/tests/test_api/test_v2/test_zones.py b/designate/tests/test_api/test_v2/test_zones.py
index 94653f56..0d611681 100644
--- a/designate/tests/test_api/test_v2/test_zones.py
+++ b/designate/tests/test_api/test_v2/test_zones.py
@@ -136,6 +136,27 @@ class ApiV2ZonesTest(ApiV2TestCase):
                 for i in 'abcdefghij']
         self._assert_paging(data, '/zones', key='zones')
 
+        # Check for invalid limits, sort_dir, sort_key, marker
+        response = self._assert_paging(data, '/zones', key='zones',
+                                       limit='invalid_limit', status=400)
+        self.assertEqual(400, response.status_int)
+        self.assertEqual('invalid_limit', response.json['type'])
+
+        response = self._assert_paging(data, '/zones', key='zones',
+                                       sort_dir='invalid_sort_dir', status=400)
+        self.assertEqual(400, response.status_int)
+        self.assertEqual('invalid_sort_dir', response.json['type'])
+
+        response = self._assert_paging(data, '/zones', key='zones',
+                                       sort_key='invalid_sort_key', status=400)
+        self.assertEqual(400, response.status_int)
+        self.assertEqual('invalid_sort_key', response.json['type'])
+
+        response = self._assert_paging(data, '/zones', key='zones',
+                                       marker='invalid_marker', status=400)
+        self.assertEqual(400, response.status_int)
+        self.assertEqual('invalid_marker', response.json['type'])
+
     @patch.object(central_service.Service, 'find_domains',
                   side_effect=rpc_common.Timeout())
     def test_get_zones_timeout(self, _):
diff --git a/designate/tests/test_storage/__init__.py b/designate/tests/test_storage/__init__.py
index 076e49fc..1073b1d2 100644
--- a/designate/tests/test_storage/__init__.py
+++ b/designate/tests/test_storage/__init__.py
@@ -98,6 +98,26 @@ class StorageTestCase(object):
             self.storage.find_servers(
                 self.admin_context, marker=str(uuid.uuid4()), limit=5)
 
+    def test_paging_marker_invalid(self):
+        with testtools.ExpectedException(exceptions.InvalidMarker):
+            self.storage.find_servers(
+                self.admin_context, marker='4')
+
+    def test_paging_limit_invalid(self):
+        with testtools.ExpectedException(exceptions.ValueError):
+            self.storage.find_servers(
+                self.admin_context, limit='z')
+
+    def test_paging_sort_dir_invalid(self):
+        with testtools.ExpectedException(exceptions.ValueError):
+            self.storage.find_servers(
+                self.admin_context, sort_dir='invalid_sort_dir')
+
+    def test_paging_sort_key_invalid(self):
+        with testtools.ExpectedException(exceptions.InvalidSortKey):
+            self.storage.find_servers(
+                self.admin_context, sort_key='invalid_sort_key')
+
     # Quota Tests
     def test_create_quota(self):
         values = self.get_quota_fixture()