openstack
/
nova
Code Issues Proposed changes

Generate better validation error message when using name regexes 

There are some regexes used in the json-schema for complex string validation.
When the validation failed, json-schema lib didn't generate a useful error
info for the user, it shows the regex to the user. But regex is really
unreadable for normal user. This patch override the default FormatChecker to
support passed in custom error message. This required using custom format
checker instead of using 'pattern'.

For aggregates API, it enabled 'null' in the name input. As the 'format'
keyword works for all allowed types and name format check will only validate
string type, so this patch change the schema to use 'oneOf' keyword, then
the 'format' will only against on string type.

Change-Id: Ic0e608b8a18b635bfcd936f57f14c9f54e1ef8b4
Partial-Bug: #1541691
This commit is contained in:
He Jie Xu 2016-02-17 16:26:19 +08:00
parent 4b47b5b6c6
commit 923cf20eca
6 changed files with 339 additions and 92 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
nova/api/openstack/compute/schemas/aggregates.py
View File

@ -16,11 +16,11 @@ import copy
from nova.api.validation import parameter_types
availability_zone = copy.deepcopy(parameter_types.name)
availability_zone['type'] = ['string', 'null']
availability_zone_with_leading_trailing_spaces = copy.deepcopy(parameter_types.
name_with_leading_trailing_spaces)
availability_zone_with_leading_trailing_spaces['type'] = ['string', 'null']
availability_zone = {'oneOf': [parameter_types.name, {'type': 'null'}]}
availability_zone_with_leading_trailing_spaces = {
'oneOf': [parameter_types.name_with_leading_trailing_spaces,
{'type': 'null'}]
}
create = {

50
nova/api/validation/parameter_types.py
View File

@ -21,6 +21,14 @@ import unicodedata
import six
from nova.i18n import _
class ValidationRegex(object):
def __init__(self, regex, reason):
self.regex = regex
self.reason = reason
def _is_printable(char):
"""determine if a unicode code point is printable.
@ -110,10 +118,12 @@ def _build_regex_range(ws=True, invert=False, exclude=None):
valid_name_regex_base = '^(?![%s])[%s]*(?<![%s])$'
valid_name_regex = valid_name_regex_base % (
_build_regex_range(ws=False, invert=True),
_build_regex_range(),
_build_regex_range(ws=False, invert=True))
valid_name_regex = ValidationRegex(
valid_name_regex_base % (
_build_regex_range(ws=False, invert=True),
_build_regex_range(),
_build_regex_range(ws=False, invert=True)),
_("printable characters. Can not start or end with whitespace."))
# This regex allows leading/trailing whitespace
@ -122,26 +132,32 @@ valid_name_leading_trailing_spaces_regex_base = (
"^[%(ws)s]*[%(no_ws)s][%(no_ws)s%(ws)s]+[%(no_ws)s][%(ws)s]*$")
valid_cell_name_regex = valid_name_regex_base % (
_build_regex_range(ws=False, invert=True),
_build_regex_range(exclude=['!', '.', '@']),
_build_regex_range(ws=False, invert=True))
valid_cell_name_regex = ValidationRegex(
valid_name_regex_base % (
_build_regex_range(ws=False, invert=True),
_build_regex_range(exclude=['!', '.', '@']),
_build_regex_range(ws=False, invert=True)),
_("printable characters except !, ., @. "
"Can not start or end with whitespace."))
# cell's name disallow '!', '.' and '@'.
valid_cell_name_leading_trailing_spaces_regex = (
valid_cell_name_leading_trailing_spaces_regex = ValidationRegex(
valid_name_leading_trailing_spaces_regex_base % {
'ws': _build_regex_range(exclude=['!', '.', '@']),
'no_ws': _build_regex_range(ws=False, exclude=['!', '.', '@'])})
'no_ws': _build_regex_range(ws=False, exclude=['!', '.', '@'])},
_("printable characters except !, ., @, "
"with at least one non space character"))
valid_name_leading_trailing_spaces_regex = (
valid_name_leading_trailing_spaces_regex = ValidationRegex(
valid_name_leading_trailing_spaces_regex_base % {
'ws': _build_regex_range(),
'no_ws': _build_regex_range(ws=False)})
'no_ws': _build_regex_range(ws=False)},
_("printable characters with at least one non space character"))
valid_name_regex_obj = re.compile(valid_name_regex, re.UNICODE)
valid_name_regex_obj = re.compile(valid_name_regex.regex, re.UNICODE)
valid_description_regex_base = '^[%s]*$'
@ -201,25 +217,25 @@ name = {
# stored in the DB and Nova specific parameters.
# This definition is used for all their parameters.
'type': 'string', 'minLength': 1, 'maxLength': 255,
'pattern': valid_name_regex,
'format': 'name'
}
cell_name = {
'type': 'string', 'minLength': 1, 'maxLength': 255,
'pattern': valid_cell_name_regex,
'format': 'cell_name'
}
cell_name_leading_trailing_spaces = {
'type': 'string', 'minLength': 1, 'maxLength': 255,
'pattern': valid_cell_name_leading_trailing_spaces_regex,
'format': 'cell_name_with_leading_trailing_spaces'
}
name_with_leading_trailing_spaces = {
'type': 'string', 'minLength': 1, 'maxLength': 255,
'pattern': valid_name_leading_trailing_spaces_regex,
'format': 'name_with_leading_trailing_spaces'
}

100
nova/api/validation/validators.py
View File

@ -27,6 +27,7 @@ from oslo_utils import uuidutils
import rfc3986
import six
from nova.api.validation import parameter_types
from nova import exception
from nova.i18n import _
@ -75,6 +76,60 @@ def _validate_uri(instance):
require_authority=True)
@jsonschema.FormatChecker.cls_checks('name_with_leading_trailing_spaces',
exception.InvalidName)
def _validate_name_with_leading_trailing_spaces(instance):
regex = parameter_types.valid_name_leading_trailing_spaces_regex
try:
if re.search(regex.regex, instance):
return True
except TypeError:
# The name must be string type. If instance isn't string type, the
# TypeError will be raised at here.
pass
raise exception.InvalidName(reason=regex.reason)
@jsonschema.FormatChecker.cls_checks('name', exception.InvalidName)
def _validate_name(instance):
regex = parameter_types.valid_name_regex
try:
if re.search(regex.regex, instance):
return True
except TypeError:
# The name must be string type. If instance isn't string type, the
# TypeError will be raised at here.
pass
raise exception.InvalidName(reason=regex.reason)
@jsonschema.FormatChecker.cls_checks('cell_name_with_leading_trailing_spaces',
exception.InvalidName)
def _validate_cell_name_with_leading_trailing_spaces(instance):
regex = parameter_types.valid_cell_name_leading_trailing_spaces_regex
try:
if re.search(regex.regex, instance):
return True
except TypeError:
# The name must be string type. If instance isn't string type, the
# TypeError will be raised at here.
pass
raise exception.InvalidName(reason=regex.reason)
@jsonschema.FormatChecker.cls_checks('cell_name', exception.InvalidName)
def _validate_cell_name(instance):
regex = parameter_types.valid_cell_name_regex
try:
if re.search(regex.regex, instance):
return True
except TypeError:
# The name must be string type. If instance isn't string type, the
# TypeError will be raised at here.
pass
raise exception.InvalidName(reason=regex.reason)
def _soft_validate_additional_properties(validator,
additional_properties_value,
instance,
@ -132,6 +187,41 @@ def _soft_validate_additional_properties(validator,
del instance[prop]
class FormatChecker(jsonschema.FormatChecker):
"""A FormatChecker can output the message from cause exception
We need understandable validation errors messages for users. When a
custom checker has an exception, the FormatChecker will output a
readable message provided by the checker.
"""
def check(self, instance, format):
"""Check whether the instance conforms to the given format.
:argument instance: the instance to check
:type: any primitive type (str, number, bool)
:argument str format: the format that instance should conform to
:raises: :exc:`FormatError` if instance does not conform to format
"""
if format not in self.checkers:
return
# For safety reasons custom checkers can be registered with
# allowed exception types. Anything else will fall into the
# default formatter.
func, raises = self.checkers[format]
result, cause = None, None
try:
result = func(instance)
except raises as e:
cause = e
if not result:
msg = "%r is not a %r" % (instance, format)
raise jsonschema_exc.FormatError(msg, cause=cause)
class _SchemaValidator(object):
"""A validator class
@ -156,16 +246,18 @@ class _SchemaValidator(object):
validator_cls = jsonschema.validators.extend(self.validator_org,
validators)
format_checker = jsonschema.FormatChecker()
format_checker = FormatChecker()
self.validator = validator_cls(schema, format_checker=format_checker)
def validate(self, *args, **kwargs):
try:
self.validator.validate(*args, **kwargs)
except jsonschema.ValidationError as ex:
# NOTE: For whole OpenStack message consistency, this error
# message has been written as the similar format of WSME.
if len(ex.path) > 0:
if isinstance(ex.cause, exception.InvalidName):
detail = ex.cause.format_message()
elif len(ex.path) > 0:
# NOTE: For whole OpenStack message consistency, this error
# message has been written as the similar format of WSME.
detail = _("Invalid input for field/attribute %(path)s."
" Value: %(value)s. %(message)s") % {
'path': ex.path.pop(), 'value': ex.instance,

5
nova/exception.py
View File

@ -420,6 +420,11 @@ class InvalidStrTime(Invalid):
msg_fmt = _("Invalid datetime string: %(reason)s")
class InvalidName(Invalid):
msg_fmt = _("An invalid 'name' value was provided. "
"The name must be: %(reason)s")
class InstanceInvalidState(Invalid):
msg_fmt = _("Instance %(instance_uuid)s in %(attr)s %(state)s. Cannot "
"%(method)s while the instance is in this state.")

29
nova/tests/unit/api/openstack/compute/test_aggregates.py
View File

@ -16,6 +16,7 @@
"""Tests for the aggregates admin api."""
import mock
import uuid
from webob import exc
from nova.api.openstack.compute import aggregates as aggregates_v21
@ -263,6 +264,19 @@ class AggregateTestCaseV21(test.NoDBTestCase):
{"name": "test",
"availability_zone": ""}})
@mock.patch('nova.compute.api.AggregateAPI.create_aggregate')
def test_create_with_none_availability_zone(self, mock_create_agg):
mock_create_agg.return_value = objects.Aggregate(self.context,
name='test',
uuid=uuid.uuid4(),
hosts=[],
metadata={})
body = {"aggregate": {"name": "test",
"availability_zone": None}}
result = self.controller.create(self.req, body=body)
mock_create_agg.assert_called_once_with(self.context, 'test', None)
self.assertEqual(result['aggregate']['name'], 'test')
def test_create_with_extra_invalid_arg(self):
self.assertRaises(self.bad_request, self.controller.create,
self.req, body={"name": "test",
@ -380,6 +394,21 @@ class AggregateTestCaseV21(test.NoDBTestCase):
self.assertRaises(self.bad_request, self.controller.update,
self.req, "2", body=test_metadata)
@mock.patch('nova.compute.api.AggregateAPI.update_aggregate')
def test_update_with_none_availability_zone(self, mock_update_agg):
agg_id = uuid.uuid4()
mock_update_agg.return_value = objects.Aggregate(self.context,
name='test',
uuid=agg_id,
hosts=[],
metadata={})
body = {"aggregate": {"name": "test",
"availability_zone": None}}
result = self.controller.update(self.req, agg_id, body=body)
mock_update_agg.assert_called_once_with(self.context, agg_id,
body['aggregate'])
self.assertEqual(result['aggregate']['name'], 'test')
def test_update_with_bad_aggregate(self):
test_metadata = {"aggregate": {"name": "test_name"}}

237
nova/tests/unit/test_api_validation.py
View File

@ -16,11 +16,13 @@ import copy
import re
import fixtures
from jsonschema import exceptions as jsonschema_exc
import six
from nova.api.openstack import api_version_request as api_version
from nova.api import validation
from nova.api.validation import parameter_types
from nova.api.validation import validators
from nova import exception
from nova import test
@ -36,7 +38,7 @@ class FakeRequest(object):
class ValidationRegex(test.NoDBTestCase):
def test_cell_names(self):
cellre = re.compile(parameter_types.valid_cell_name_regex)
cellre = re.compile(parameter_types.valid_cell_name_regex.regex)
self.assertTrue(cellre.search('foo'))
self.assertFalse(cellre.search('foo.bar'))
self.assertFalse(cellre.search('foo@bar'))
@ -108,6 +110,33 @@ class APIValidationTestCase(test.NoDBTestCase):
self.fail('Any exception does not happen.')
class FormatCheckerTestCase(test.NoDBTestCase):
def test_format_checker_failed(self):
format_checker = validators.FormatChecker()
exc = self.assertRaises(jsonschema_exc.FormatError,
format_checker.check, " ", "name")
self.assertIsInstance(exc.cause, exception.InvalidName)
self.assertEqual("An invalid 'name' value was provided. The name must "
"be: printable characters. "
"Can not start or end with whitespace.",
exc.cause.format_message())
def test_format_checker_failed_with_non_string(self):
checks = ["name", "name_with_leading_trailing_spaces",
"cell_name", "cell_name_with_leading_trailing_spaces"]
format_checker = validators.FormatChecker()
for check in checks:
exc = self.assertRaises(jsonschema_exc.FormatError,
format_checker.check, None, "name")
self.assertIsInstance(exc.cause, exception.InvalidName)
self.assertEqual("An invalid 'name' value was provided. The name "
"must be: printable characters. "
"Can not start or end with whitespace.",
exc.cause.format_message())
class MicroversionsSchemaTestCase(APIValidationTestCase):
def setUp(self):
@ -668,6 +697,122 @@ class HostnameIPaddressTestCase(APIValidationTestCase):
expected_detail=detail)
class CellNameTestCase(APIValidationTestCase):
def setUp(self):
super(CellNameTestCase, self).setUp()
schema = {
'type': 'object',
'properties': {
'foo': parameter_types.cell_name,
},
}
@validation.schema(request_body_schema=schema)
def post(req, body):
return 'Validation succeeded.'
self.post = post
def test_validate_name(self):
self.assertEqual('Validation succeeded.',
self.post(body={'foo': 'abc'},
req=FakeRequest()))
self.assertEqual('Validation succeeded.',
self.post(body={'foo': 'my server'},
req=FakeRequest()))
self.assertEqual('Validation succeeded.',
self.post(body={'foo': u'\u0434'}, req=FakeRequest()))
self.assertEqual('Validation succeeded.',
self.post(body={'foo': u'\u0434\u2006\ufffd'},
req=FakeRequest()))
def test_validate_name_fails(self):
error = ("An invalid 'name' value was provided. The name must be: "
"printable characters except !, ., @. "
"Can not start or end with whitespace.")
should_fail = (' ',
' server',
'server ',
u'a\xa0', # trailing unicode space
u'\uffff', # non-printable unicode
'abc!def',
'abc.def',
'abc@def')
for item in should_fail:
self.check_validation_error(self.post, body={'foo': item},
expected_detail=error)
# four-byte unicode, if supported by this python build
try:
self.check_validation_error(self.post, body={'foo': u'\U00010000'},
expected_detail=error)
except ValueError:
pass
class CellNameLeadingTrailingSpacesTestCase(APIValidationTestCase):
def setUp(self):
super(CellNameLeadingTrailingSpacesTestCase, self).setUp()
schema = {
'type': 'object',
'properties': {
'foo': parameter_types.cell_name_leading_trailing_spaces,
},
}
@validation.schema(request_body_schema=schema)
def post(req, body):
return 'Validation succeeded.'
self.post = post
def test_validate_name(self):
self.assertEqual('Validation succeeded.',
self.post(body={'foo': 'abc'},
req=FakeRequest()))
self.assertEqual('Validation succeeded.',
self.post(body={'foo': 'my server'},
req=FakeRequest()))
self.assertEqual('Validation succeeded.',
self.post(body={'foo': u'\u0434'}, req=FakeRequest()))
self.assertEqual('Validation succeeded.',
self.post(body={'foo': u'\u0434\u2006\ufffd'},
req=FakeRequest()))
self.assertEqual('Validation succeeded.',
self.post(body={'foo': ' my server'},
req=FakeRequest()))
self.assertEqual('Validation succeeded.',
self.post(body={'foo': 'my server '},
req=FakeRequest()))
def test_validate_name_fails(self):
error = ("An invalid 'name' value was provided. The name must be: "
"printable characters except !, ., @, "
"with at least one non space character")
should_fail = (
' ',
u'\uffff', # non-printable unicode
'abc!def',
'abc.def',
'abc@def')
for item in should_fail:
self.check_validation_error(self.post, body={'foo': item},
expected_detail=error)
# four-byte unicode, if supported by this python build
try:
self.check_validation_error(self.post, body={'foo': u'\U00010000'},
expected_detail=error)
except ValueError:
pass
class NameTestCase(APIValidationTestCase):
def setUp(self):
@ -701,54 +846,25 @@ class NameTestCase(APIValidationTestCase):
req=FakeRequest()))
def test_validate_name_fails(self):
detail = (u"Invalid input for field/attribute foo. Value: ."
" ' ' does not match .*")
self.check_validation_error(self.post, body={'foo': ' '},
expected_detail=detail)
error = ("An invalid 'name' value was provided. The name must be: "
"printable characters. "
"Can not start or end with whitespace.")
detail = ("Invalid input for field/attribute foo. Value: server."
" ' server' does not match .*")
self.check_validation_error(self.post, body={'foo': ' server'},
expected_detail=detail)
should_fail = (' ',
' server',
'server ',
u'a\xa0', # trailing unicode space
u'\uffff', # non-printable unicode
)
detail = ("Invalid input for field/attribute foo. Value: server ."
" 'server ' does not match .*")
self.check_validation_error(self.post, body={'foo': 'server '},
expected_detail=detail)
detail = ("Invalid input for field/attribute foo. Value: a."
" ' a' does not match .*")
self.check_validation_error(self.post, body={'foo': ' a'},
expected_detail=detail)
detail = ("Invalid input for field/attribute foo. Value: a ."
" 'a ' does not match .*")
self.check_validation_error(self.post, body={'foo': 'a '},
expected_detail=detail)
# NOTE(stpierre): Quoting for the unicode values in the error
# messages below gets *really* messy, so we just wildcard it
# out. (e.g., '.* does not match'). In practice, we don't
# particularly care about that part of the error message.
# trailing unicode space
detail = (u"Invalid input for field/attribute foo. Value: a\xa0."
u' .* does not match .*')
self.check_validation_error(self.post, body={'foo': u'a\xa0'},
expected_detail=detail)
# non-printable unicode
detail = (u"Invalid input for field/attribute foo. Value: \uffff."
u" .* does not match .*")
self.check_validation_error(self.post, body={'foo': u'\uffff'},
expected_detail=detail)
for item in should_fail:
self.check_validation_error(self.post, body={'foo': item},
expected_detail=error)
# four-byte unicode, if supported by this python build
try:
detail = (u"Invalid input for field/attribute foo. Value: "
u"\U00010000. .* does not match .*")
self.check_validation_error(self.post, body={'foo': u'\U00010000'},
expected_detail=detail)
expected_detail=error)
except ValueError:
pass
@ -799,34 +915,23 @@ class NameWithLeadingTrailingSpacesTestCase(APIValidationTestCase):
req=FakeRequest()))
def test_validate_name_fails(self):
detail = (u"Invalid input for field/attribute foo. Value: ."
u" ' ' does not match .*")
self.check_validation_error(self.post, body={'foo': ' '},
expected_detail=detail)
error = ("An invalid 'name' value was provided. The name must be: "
"printable characters with at least one non space character")
# NOTE(stpierre): Quoting for the unicode values in the error
# messages below gets *really* messy, so we just wildcard it
# out. (e.g., '.* does not match'). In practice, we don't
# particularly care about that part of the error message.
should_fail = (
' ',
u'\xa0', # unicode space
u'\uffff', # non-printable unicode
)
# unicode space
detail = (u"Invalid input for field/attribute foo. Value: \xa0."
u' .* does not match .*')
self.check_validation_error(self.post, body={'foo': u'\xa0'},
expected_detail=detail)
# non-printable unicode
detail = (u"Invalid input for field/attribute foo. Value: \uffff."
u" .* does not match .*")
self.check_validation_error(self.post, body={'foo': u'\uffff'},
expected_detail=detail)
for item in should_fail:
self.check_validation_error(self.post, body={'foo': item},
expected_detail=error)
# four-byte unicode, if supported by this python build
try:
detail = (u"Invalid input for field/attribute foo. Value: "
u"\U00010000. .* does not match .*")
self.check_validation_error(self.post, body={'foo': u'\U00010000'},
expected_detail=detail)
expected_detail=error)
except ValueError:
pass
@ -1193,7 +1298,7 @@ class Ipv6TestCase(APIValidationTestCase):
detail = ("Invalid input for field/attribute foo. Value: localhost."
" 'localhost' is not a 'ipv6'")
self.check_validation_error(self.post, body={'foo': 'localhost'},
expected_detail=detail)
expected_detail=detail)
detail = ("Invalid input for field/attribute foo."
" Value: 192.168.0.100. '192.168.0.100' is not a 'ipv6'")