Raise HTTPBadRequest instead of ValueError for API validation
Raise HTTPBadRequest exception instead of ValueError for parameters validation. Change-Id: I468181930299353908a53a3a940ff843d8547bde Closes-Bug: #1449349
This commit is contained in:
parent
316e798523
commit
767696fc47
@ -13,6 +13,7 @@
|
||||
|
||||
import itertools
|
||||
|
||||
import six
|
||||
from webob import exc
|
||||
|
||||
from heat.api.openstack.v1 import util
|
||||
@ -120,7 +121,11 @@ class EventController(object):
|
||||
|
||||
key = rpc_api.PARAM_LIMIT
|
||||
if key in params:
|
||||
limit = param_utils.extract_int(key, params[key], allow_zero=True)
|
||||
try:
|
||||
limit = param_utils.extract_int(key, params[key],
|
||||
allow_zero=True)
|
||||
except ValueError as e:
|
||||
raise exc.HTTPBadRequest(six.text_type(e))
|
||||
params[key] = limit
|
||||
|
||||
if resource_name is None:
|
||||
|
@ -13,6 +13,9 @@
|
||||
|
||||
import itertools
|
||||
|
||||
import six
|
||||
from webob import exc
|
||||
|
||||
from heat.api.openstack.v1 import util
|
||||
from heat.common import identifier
|
||||
from heat.common import param_utils
|
||||
@ -85,7 +88,10 @@ class ResourceController(object):
|
||||
nested_depth = 0
|
||||
key = rpc_api.PARAM_NESTED_DEPTH
|
||||
if key in req.params:
|
||||
nested_depth = param_utils.extract_int(key, req.params[key])
|
||||
try:
|
||||
nested_depth = param_utils.extract_int(key, req.params[key])
|
||||
except ValueError as e:
|
||||
raise exc.HTTPBadRequest(six.text_type(e))
|
||||
|
||||
res_list = self.rpc_client.list_stack_resources(req.context,
|
||||
identity,
|
||||
|
@ -169,6 +169,26 @@ class StackController(object):
|
||||
def default(self, req, **args):
|
||||
raise exc.HTTPNotFound()
|
||||
|
||||
def _extract_bool_param(self, name, value):
|
||||
try:
|
||||
return param_utils.extract_bool(name, value)
|
||||
except ValueError as e:
|
||||
raise exc.HTTPBadRequest(six.text_type(e))
|
||||
|
||||
def _extract_int_param(self, name, value,
|
||||
allow_zero=True, allow_negative=False):
|
||||
try:
|
||||
return param_utils.extract_int(name, value,
|
||||
allow_zero, allow_negative)
|
||||
except ValueError as e:
|
||||
raise exc.HTTPBadRequest(six.text_type(e))
|
||||
|
||||
def _extract_tags_param(self, tags):
|
||||
try:
|
||||
return param_utils.extract_tags(tags)
|
||||
except ValueError as e:
|
||||
raise exc.HTTPBadRequest(six.text_type(e))
|
||||
|
||||
def _index(self, req, tenant_safe=True):
|
||||
filter_whitelist = {
|
||||
'id': 'mixed',
|
||||
@ -196,55 +216,56 @@ class StackController(object):
|
||||
filter_params = util.get_allowed_params(req.params, filter_whitelist)
|
||||
|
||||
show_deleted = False
|
||||
if rpc_api.PARAM_SHOW_DELETED in params:
|
||||
params[rpc_api.PARAM_SHOW_DELETED] = param_utils.extract_bool(
|
||||
params[rpc_api.PARAM_SHOW_DELETED])
|
||||
show_deleted = params[rpc_api.PARAM_SHOW_DELETED]
|
||||
p_name = rpc_api.PARAM_SHOW_DELETED
|
||||
if p_name in params:
|
||||
params[p_name] = self._extract_bool_param(p_name, params[p_name])
|
||||
show_deleted = params[p_name]
|
||||
|
||||
show_nested = False
|
||||
if rpc_api.PARAM_SHOW_NESTED in params:
|
||||
params[rpc_api.PARAM_SHOW_NESTED] = param_utils.extract_bool(
|
||||
params[rpc_api.PARAM_SHOW_NESTED])
|
||||
show_nested = params[rpc_api.PARAM_SHOW_NESTED]
|
||||
p_name = rpc_api.PARAM_SHOW_NESTED
|
||||
if p_name in params:
|
||||
params[p_name] = self._extract_bool_param(p_name, params[p_name])
|
||||
show_nested = params[p_name]
|
||||
|
||||
key = rpc_api.PARAM_LIMIT
|
||||
if key in params:
|
||||
params[key] = param_utils.extract_int(key, params[key])
|
||||
params[key] = self._extract_int_param(key, params[key])
|
||||
|
||||
show_hidden = False
|
||||
if rpc_api.PARAM_SHOW_HIDDEN in params:
|
||||
params[rpc_api.PARAM_SHOW_HIDDEN] = param_utils.extract_bool(
|
||||
params[rpc_api.PARAM_SHOW_HIDDEN])
|
||||
show_hidden = params[rpc_api.PARAM_SHOW_HIDDEN]
|
||||
p_name = rpc_api.PARAM_SHOW_HIDDEN
|
||||
if p_name in params:
|
||||
params[p_name] = self._extract_bool_param(p_name, params[p_name])
|
||||
show_hidden = params[p_name]
|
||||
|
||||
tags = None
|
||||
if rpc_api.PARAM_TAGS in params:
|
||||
params[rpc_api.PARAM_TAGS] = param_utils.extract_tags(
|
||||
params[rpc_api.PARAM_TAGS] = self._extract_tags_param(
|
||||
params[rpc_api.PARAM_TAGS])
|
||||
tags = params[rpc_api.PARAM_TAGS]
|
||||
|
||||
tags_any = None
|
||||
if rpc_api.PARAM_TAGS_ANY in params:
|
||||
params[rpc_api.PARAM_TAGS_ANY] = param_utils.extract_tags(
|
||||
params[rpc_api.PARAM_TAGS_ANY] = self._extract_tags_param(
|
||||
params[rpc_api.PARAM_TAGS_ANY])
|
||||
tags_any = params[rpc_api.PARAM_TAGS_ANY]
|
||||
|
||||
not_tags = None
|
||||
if rpc_api.PARAM_NOT_TAGS in params:
|
||||
params[rpc_api.PARAM_NOT_TAGS] = param_utils.extract_tags(
|
||||
params[rpc_api.PARAM_NOT_TAGS] = self._extract_tags_param(
|
||||
params[rpc_api.PARAM_NOT_TAGS])
|
||||
not_tags = params[rpc_api.PARAM_NOT_TAGS]
|
||||
|
||||
not_tags_any = None
|
||||
if rpc_api.PARAM_NOT_TAGS_ANY in params:
|
||||
params[rpc_api.PARAM_NOT_TAGS_ANY] = param_utils.extract_tags(
|
||||
params[rpc_api.PARAM_NOT_TAGS_ANY] = self._extract_tags_param(
|
||||
params[rpc_api.PARAM_NOT_TAGS_ANY])
|
||||
not_tags_any = params[rpc_api.PARAM_NOT_TAGS_ANY]
|
||||
|
||||
# get the with_count value, if invalid, raise ValueError
|
||||
with_count = False
|
||||
if req.params.get('with_count'):
|
||||
with_count = param_utils.extract_bool(
|
||||
with_count = self._extract_bool_param(
|
||||
'with_count',
|
||||
req.params.get('with_count'))
|
||||
|
||||
if not filter_params:
|
||||
@ -270,8 +291,8 @@ class StackController(object):
|
||||
tags_any=tags_any,
|
||||
not_tags=not_tags,
|
||||
not_tags_any=not_tags_any)
|
||||
except AttributeError as exc:
|
||||
LOG.warn(_LW("Old Engine Version: %s") % exc)
|
||||
except AttributeError as ex:
|
||||
LOG.warn(_LW("Old Engine Version: %s") % ex)
|
||||
|
||||
return stacks_view.collection(req, stacks=stacks, count=count,
|
||||
tenant_safe=tenant_safe)
|
||||
@ -286,9 +307,11 @@ class StackController(object):
|
||||
Lists summary information for all stacks
|
||||
"""
|
||||
global_tenant = False
|
||||
if rpc_api.PARAM_GLOBAL_TENANT in req.params:
|
||||
global_tenant = param_utils.extract_bool(
|
||||
req.params.get(rpc_api.PARAM_GLOBAL_TENANT))
|
||||
name = rpc_api.PARAM_GLOBAL_TENANT
|
||||
if name in req.params:
|
||||
global_tenant = self._extract_bool_param(
|
||||
name,
|
||||
req.params.get(name))
|
||||
|
||||
if global_tenant:
|
||||
return self.global_index(req, req.context.tenant_id)
|
||||
@ -332,7 +355,7 @@ class StackController(object):
|
||||
args = data.args()
|
||||
key = rpc_api.PARAM_TIMEOUT
|
||||
if key in args:
|
||||
args[key] = param_utils.extract_int(key, args[key])
|
||||
args[key] = self._extract_int_param(key, args[key])
|
||||
|
||||
result = self.rpc_client.create_stack(req.context,
|
||||
data.stack_name(),
|
||||
@ -409,7 +432,7 @@ class StackController(object):
|
||||
args = data.args()
|
||||
key = rpc_api.PARAM_TIMEOUT
|
||||
if key in args:
|
||||
args[key] = param_utils.extract_int(key, args[key])
|
||||
args[key] = self._extract_int_param(key, args[key])
|
||||
|
||||
self.rpc_client.update_stack(req.context,
|
||||
identity,
|
||||
@ -431,7 +454,7 @@ class StackController(object):
|
||||
args = data.args()
|
||||
key = rpc_api.PARAM_TIMEOUT
|
||||
if key in args:
|
||||
args[key] = param_utils.extract_int(key, args[key])
|
||||
args[key] = self._extract_int_param(key, args[key])
|
||||
|
||||
self.rpc_client.update_stack(req.context,
|
||||
identity,
|
||||
|
@ -16,15 +16,16 @@ from oslo_utils import strutils
|
||||
from heat.common.i18n import _
|
||||
|
||||
|
||||
def extract_bool(subject):
|
||||
def extract_bool(name, value):
|
||||
'''
|
||||
Convert any true/false string to its corresponding boolean value,
|
||||
regardless of case.
|
||||
'''
|
||||
if str(subject).lower() not in ('true', 'false'):
|
||||
raise ValueError(_('Unrecognized value "%(value)s", acceptable '
|
||||
'values are: true, false.') % {'value': subject})
|
||||
return strutils.bool_from_string(subject, strict=True)
|
||||
if str(value).lower() not in ('true', 'false'):
|
||||
raise ValueError(_('Unrecognized value "%(value)s" for "%(name)s", '
|
||||
'acceptable values are: true, false.')
|
||||
% {'value': value, 'name': name})
|
||||
return strutils.bool_from_string(value, strict=True)
|
||||
|
||||
|
||||
def extract_int(name, value, allow_zero=True, allow_negative=False):
|
||||
|
@ -46,14 +46,14 @@ def extract_args(params):
|
||||
else:
|
||||
raise ValueError(_('Invalid timeout value %s') % timeout)
|
||||
|
||||
if rpc_api.PARAM_DISABLE_ROLLBACK in params:
|
||||
disable_rollback = param_utils.extract_bool(
|
||||
params[rpc_api.PARAM_DISABLE_ROLLBACK])
|
||||
kwargs[rpc_api.PARAM_DISABLE_ROLLBACK] = disable_rollback
|
||||
name = rpc_api.PARAM_DISABLE_ROLLBACK
|
||||
if name in params:
|
||||
disable_rollback = param_utils.extract_bool(name, params[name])
|
||||
kwargs[name] = disable_rollback
|
||||
|
||||
if rpc_api.PARAM_SHOW_DELETED in params:
|
||||
params[rpc_api.PARAM_SHOW_DELETED] = param_utils.extract_bool(
|
||||
params[rpc_api.PARAM_SHOW_DELETED])
|
||||
name = rpc_api.PARAM_SHOW_DELETED
|
||||
if name in params:
|
||||
params[name] = param_utils.extract_bool(name, params[name])
|
||||
|
||||
adopt_data = params.get(rpc_api.PARAM_ADOPT_STACK_DATA)
|
||||
if adopt_data:
|
||||
|
@ -427,7 +427,7 @@ class StackControllerTest(ControllerTest, common.HeatTestCase):
|
||||
params = {'limit': 'not-an-int'}
|
||||
req = self._get('/stacks', params=params)
|
||||
|
||||
ex = self.assertRaises(ValueError,
|
||||
ex = self.assertRaises(webob.exc.HTTPBadRequest,
|
||||
self.controller.index, req,
|
||||
tenant_id=self.tenant)
|
||||
self.assertEqual("Only integer is acceptable by 'limit'.",
|
||||
@ -499,9 +499,10 @@ class StackControllerTest(ControllerTest, common.HeatTestCase):
|
||||
params = {'with_count': 'invalid_value'}
|
||||
req = self._get('/stacks', params=params)
|
||||
|
||||
exc = self.assertRaises(ValueError, self.controller.index,
|
||||
exc = self.assertRaises(webob.exc.HTTPBadRequest,
|
||||
self.controller.index,
|
||||
req, tenant_id=self.tenant)
|
||||
excepted = ('Unrecognized value "invalid_value", '
|
||||
excepted = ('Unrecognized value "invalid_value" for "with_count", '
|
||||
'acceptable values are: true, false')
|
||||
self.assertIn(excepted, six.text_type(exc))
|
||||
|
||||
@ -848,7 +849,7 @@ class StackControllerTest(ControllerTest, common.HeatTestCase):
|
||||
req = self._post('/stacks', json.dumps(body))
|
||||
|
||||
mock_call = self.patchobject(rpc_client.EngineClient, 'call')
|
||||
ex = self.assertRaises(ValueError,
|
||||
ex = self.assertRaises(webob.exc.HTTPBadRequest,
|
||||
self.controller.create, req,
|
||||
tenant_id=self.tenant, body=body)
|
||||
|
||||
@ -1070,7 +1071,7 @@ class StackControllerTest(ControllerTest, common.HeatTestCase):
|
||||
req = self._post('/stacks', json.dumps(body))
|
||||
|
||||
mock_call = self.patchobject(rpc_client.EngineClient, 'call')
|
||||
ex = self.assertRaises(ValueError,
|
||||
ex = self.assertRaises(webob.exc.HTTPBadRequest,
|
||||
self.controller.create, req,
|
||||
tenant_id=self.tenant, body=body)
|
||||
|
||||
@ -1588,7 +1589,7 @@ class StackControllerTest(ControllerTest, common.HeatTestCase):
|
||||
json.dumps(body))
|
||||
|
||||
mock_call = self.patchobject(rpc_client.EngineClient, 'call')
|
||||
ex = self.assertRaises(ValueError,
|
||||
ex = self.assertRaises(webob.exc.HTTPBadRequest,
|
||||
self.controller.update, req,
|
||||
tenant_id=identity.tenant,
|
||||
stack_name=identity.stack_name,
|
||||
@ -1708,7 +1709,7 @@ class StackControllerTest(ControllerTest, common.HeatTestCase):
|
||||
json.dumps(body))
|
||||
|
||||
mock_call = self.patchobject(rpc_client.EngineClient, 'call')
|
||||
ex = self.assertRaises(ValueError,
|
||||
ex = self.assertRaises(webob.exc.HTTPBadRequest,
|
||||
self.controller.update_patch, req,
|
||||
tenant_id=identity.tenant,
|
||||
stack_name=identity.stack_name,
|
||||
@ -2295,7 +2296,7 @@ class ResourceControllerTest(ControllerTest, common.HeatTestCase):
|
||||
{'nested_depth': 'non-int'})
|
||||
|
||||
mock_call = self.patchobject(rpc_client.EngineClient, 'call')
|
||||
ex = self.assertRaises(ValueError,
|
||||
ex = self.assertRaises(webob.exc.HTTPBadRequest,
|
||||
self.controller.index, req,
|
||||
tenant_id=self.tenant,
|
||||
stack_name=stack_identity.stack_name,
|
||||
@ -3059,7 +3060,7 @@ class EventControllerTest(ControllerTest, common.HeatTestCase):
|
||||
req = self._get(sid._tenant_path() + '/events',
|
||||
params={'limit': 'not-an-int'})
|
||||
|
||||
ex = self.assertRaises(ValueError,
|
||||
ex = self.assertRaises(webob.exc.HTTPBadRequest,
|
||||
self.controller.index, req,
|
||||
tenant_id=self.tenant,
|
||||
stack_name=sid.stack_name,
|
||||
|
@ -18,11 +18,12 @@ from heat.tests import common
|
||||
class TestExtractBool(common.HeatTestCase):
|
||||
def test_extract_bool(self):
|
||||
for value in ('True', 'true', 'TRUE', True):
|
||||
self.assertTrue(param_utils.extract_bool(value))
|
||||
self.assertTrue(param_utils.extract_bool('bool', value))
|
||||
for value in ('False', 'false', 'FALSE', False):
|
||||
self.assertFalse(param_utils.extract_bool(value))
|
||||
self.assertFalse(param_utils.extract_bool('bool', value))
|
||||
for value in ('foo', 't', 'f', 'yes', 'no', 'y', 'n', '1', '0', None):
|
||||
self.assertRaises(ValueError, param_utils.extract_bool, value)
|
||||
self.assertRaises(ValueError, param_utils.extract_bool,
|
||||
'bool', value)
|
||||
|
||||
|
||||
class TestExtractInt(common.HeatTestCase):
|
||||
|
Loading…
x
Reference in New Issue
Block a user