diff --git a/karbor/api/common.py b/karbor/api/common.py index 9afe5465..4c1aa2b4 100644 --- a/karbor/api/common.py +++ b/karbor/api/common.py @@ -19,7 +19,9 @@ from oslo_log import log as logging from six.moves import urllib import webob +from karbor import db from karbor.i18n import _ +from karbor import utils api_common_opts = [ @@ -87,17 +89,8 @@ def _get_marker_param(params): def _get_offset_param(params): """Extract offset id from request's dictionary (defaults to 0) or fail.""" - try: - offset = int(params.pop('offset', 0)) - except ValueError: - msg = _('offset param must be an integer') - raise webob.exc.HTTPBadRequest(explanation=msg) - - if offset < 0: - msg = _('offset param must be positive') - raise webob.exc.HTTPBadRequest(explanation=msg) - - return offset + offset = params.pop('offset', 0) + return utils.validate_integer(offset, 'offset', 0, db.MAX_INT) def limited(items, request, max_limit=None): diff --git a/karbor/api/openstack/wsgi.py b/karbor/api/openstack/wsgi.py index 00a79089..efc3cfee 100644 --- a/karbor/api/openstack/wsgi.py +++ b/karbor/api/openstack/wsgi.py @@ -875,33 +875,6 @@ class Controller(object): except exception.InvalidInput as error: raise webob.exc.HTTPBadRequest(explanation=error.msg) - @staticmethod - def validate_integer(value, name, min_value=None, max_value=None): - """Make sure that value is a valid integer, potentially within range. - - :param value: the value of the integer - :param name: the name of the integer - :param min_length: the min_length of the integer - :param max_length: the max_length of the integer - :returns: integer - """ - try: - value = int(value) - except (TypeError, ValueError, UnicodeEncodeError): - raise webob.exc.HTTPBadRequest(explanation=( - _('%s must be an integer.') % name)) - - if min_value is not None and value < min_value: - raise webob.exc.HTTPBadRequest( - explanation=(_('%(value_name)s must be >= %(min_value)d') % - {'value_name': name, 'min_value': min_value})) - if max_value is not None and value > max_value: - raise webob.exc.HTTPBadRequest( - explanation=(_('%(value_name)s must be <= %(max_value)d') % - {'value_name': name, 'max_value': max_value})) - - return value - class Fault(webob.exc.HTTPException): """Wrap webob.exc.HTTPException to provide API friendly response.""" diff --git a/karbor/tests/unit/api/v1/test_plans.py b/karbor/tests/unit/api/v1/test_plans.py index e2070016..e565ebcc 100644 --- a/karbor/tests/unit/api/v1/test_plans.py +++ b/karbor/tests/unit/api/v1/test_plans.py @@ -131,6 +131,29 @@ class PlanApiTest(base.TestCase): self.controller.index(req) self.assertTrue(moak_get_all.called) + @mock.patch( + 'karbor.api.v1.plans.PlansController._get_all') + def test_plan_index_limit_offset(self, moak_get_all): + req = fakes.HTTPRequest.blank('/v1/plans?limit=2&offset=1') + self.controller.index(req) + self.assertTrue(moak_get_all.called) + + req = fakes.HTTPRequest.blank('/v1/plans?limit=-1&offset=1') + self.assertRaises(exc.HTTPBadRequest, + self.controller.index, + req) + + req = fakes.HTTPRequest.blank('/v1/plans?limit=a&offset=1') + self.assertRaises(exc.HTTPBadRequest, + self.controller.index, + req) + + url = '/v1/plans?limit=2&offset=43543564546567575' + req = fakes.HTTPRequest.blank(url) + self.assertRaises(exc.HTTPBadRequest, + self.controller.index, + req) + def test_plan_create_empty_dict(self): plan = self._plan_in_request_body(parameters={}) body = {"plan": plan} diff --git a/karbor/utils.py b/karbor/utils.py index 1d624891..81c71816 100644 --- a/karbor/utils.py +++ b/karbor/utils.py @@ -13,6 +13,7 @@ """Utilities and helper functions.""" import ast import os +import webob.exc from keystoneclient import discover as ks_discover from oslo_config import cfg @@ -134,3 +135,30 @@ def get_auth_uri(v3=True): importutils.import_module('keystonemiddleware.auth_token') auth_uri = cfg.CONF.keystone_authtoken.auth_uri return auth_uri.replace('v2.0', 'v3') if auth_uri and v3 else auth_uri + + +def validate_integer(value, name, min_value=None, max_value=None): + """Make sure that value is a valid integer, potentially within range. + + :param value: the value of the integer + :param name: the name of the integer + :param min_length: the min_length of the integer + :param max_length: the max_length of the integer + :returns: integer + """ + try: + value = int(value) + except (TypeError, ValueError, UnicodeEncodeError): + raise webob.exc.HTTPBadRequest(explanation=( + _('%s must be an integer.') % name)) + + if min_value is not None and value < min_value: + raise webob.exc.HTTPBadRequest( + explanation=(_('%(value_name)s must be >= %(min_value)d') % + {'value_name': name, 'min_value': min_value})) + if max_value is not None and value > max_value: + raise webob.exc.HTTPBadRequest( + explanation=(_('%(value_name)s must be <= %(max_value)d') % + {'value_name': name, 'max_value': max_value})) + + return value