openstack/neutron
Code Issues Proposed changes

Validate string length at API level

Browse Source 
Add validation of string field.
The the length of API validation matches max length of DB entries,
which is 255.

[Before fix]
DB returns 500 internal DB Error.
[After fix]
Neutron returns 400 Bad Request Error
(e.g. "XXX" exceeds maximum length of 255).

APIImpact

Change-Id: Ide98f347da563c5df10daca00491027a1b78523b
Closes-Bug: 1408230
This commit is contained in:
watanabe.isao 2015-01-08 11:15:44 +09:00
parent 86dbb886be
commit 3d1d08e008
11 changed files with 67 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

25
neutron/api/v2/attributes.py
View File

@ -31,6 +31,15 @@ SHARED = 'shared'
# Used by range check to indicate no limit for a bound.
UNLIMITED = None
# TODO(watanabe.isao): A fix like in neutron/db/models_v2.py needs to be
# done in other db modules, to reuse the following constants.
# Common definitions for maximum string field length
NAME_MAX_LEN = 255
TENANT_ID_MAX_LEN = 255
DESCRIPTION_MAX_LEN = 255
DEVICE_ID_MAX_LEN = 255
DEVICE_OWNER_MAX_LEN = 255
def _verify_dict_keys(expected_keys, target_dict, strict=True):
"""Allows to verify keys in a dictionary.
@ -680,7 +689,7 @@ RESOURCE_ATTRIBUTE_MAP = {
'is_visible': True,
'primary_key': True},
'name': {'allow_post': True, 'allow_put': True,
'validate': {'type:string': None},
'validate': {'type:string': NAME_MAX_LEN},
'default': '', 'is_visible': True},
'subnets': {'allow_post': False, 'allow_put': False,
'default': [],
@ -692,7 +701,7 @@ RESOURCE_ATTRIBUTE_MAP = {
'status': {'allow_post': False, 'allow_put': False,
'is_visible': True},
'tenant_id': {'allow_post': True, 'allow_put': False,
'validate': {'type:string': None},
'validate': {'type:string': TENANT_ID_MAX_LEN},
'required_by_policy': True,
'is_visible': True},
SHARED: {'allow_post': True,
@ -709,7 +718,7 @@ RESOURCE_ATTRIBUTE_MAP = {
'is_visible': True,
'primary_key': True},
'name': {'allow_post': True, 'allow_put': True, 'default': '',
'validate': {'type:string': None},
'validate': {'type:string': NAME_MAX_LEN},
'is_visible': True},
'network_id': {'allow_post': True, 'allow_put': False,
'required_by_policy': True,
@ -731,15 +740,15 @@ RESOURCE_ATTRIBUTE_MAP = {
'enforce_policy': True,
'is_visible': True},
'device_id': {'allow_post': True, 'allow_put': True,
'validate': {'type:string': None},
'validate': {'type:string': DEVICE_ID_MAX_LEN},
'default': '',
'is_visible': True},
'device_owner': {'allow_post': True, 'allow_put': True,
'validate': {'type:string': None},
'validate': {'type:string': DEVICE_OWNER_MAX_LEN},
'default': '',
'is_visible': True},
'tenant_id': {'allow_post': True, 'allow_put': False,
'validate': {'type:string': None},
'validate': {'type:string': TENANT_ID_MAX_LEN},
'required_by_policy': True,
'is_visible': True},
'status': {'allow_post': False, 'allow_put': False,
@ -751,7 +760,7 @@ RESOURCE_ATTRIBUTE_MAP = {
'is_visible': True,
'primary_key': True},
'name': {'allow_post': True, 'allow_put': True, 'default': '',
'validate': {'type:string': None},
'validate': {'type:string': NAME_MAX_LEN},
'is_visible': True},
'ip_version': {'allow_post': True, 'allow_put': False,
'convert_to': convert_to_int,
@ -783,7 +792,7 @@ RESOURCE_ATTRIBUTE_MAP = {
'validate': {'type:hostroutes': None},
'is_visible': True},
'tenant_id': {'allow_post': True, 'allow_put': False,
'validate': {'type:string': None},
'validate': {'type:string': TENANT_ID_MAX_LEN},
'required_by_policy': True,
'is_visible': True},
'enable_dhcp': {'allow_post': True, 'allow_put': True,

16
neutron/db/models_v2.py
View File

@ -16,6 +16,7 @@
import sqlalchemy as sa
from sqlalchemy import orm
from neutron.api.v2 import attributes as attr
from neutron.common import constants
from neutron.db import model_base
from neutron.openstack.common import uuidutils
@ -25,7 +26,7 @@ class HasTenant(object):
"""Tenant mixin, add to subclasses that have a tenant."""
# NOTE(jkoelker) tenant_id is just a free form string ;(
tenant_id = sa.Column(sa.String(255), index=True)
tenant_id = sa.Column(sa.String(attr.TENANT_ID_MAX_LEN), index=True)
class HasId(object):
@ -40,7 +41,7 @@ class HasStatusDescription(object):
"""Status with description mixin."""
status = sa.Column(sa.String(16), nullable=False)
status_description = sa.Column(sa.String(255))
status_description = sa.Column(sa.String(attr.DESCRIPTION_MAX_LEN))
class IPAvailabilityRange(model_base.BASEV2):
@ -128,15 +129,16 @@ class SubnetRoute(model_base.BASEV2, Route):
class Port(model_base.BASEV2, HasId, HasTenant):
"""Represents a port on a Neutron v2 network."""
name = sa.Column(sa.String(255))
name = sa.Column(sa.String(attr.NAME_MAX_LEN))
network_id = sa.Column(sa.String(36), sa.ForeignKey("networks.id"),
nullable=False)
fixed_ips = orm.relationship(IPAllocation, backref='ports', lazy='joined')
mac_address = sa.Column(sa.String(32), nullable=False)
admin_state_up = sa.Column(sa.Boolean(), nullable=False)
status = sa.Column(sa.String(16), nullable=False)
device_id = sa.Column(sa.String(255), nullable=False)
device_owner = sa.Column(sa.String(255), nullable=False)
device_id = sa.Column(sa.String(attr.DEVICE_ID_MAX_LEN), nullable=False)
device_owner = sa.Column(sa.String(attr.DEVICE_OWNER_MAX_LEN),
nullable=False)
__table_args__ = (
sa.UniqueConstraint(
network_id, mac_address,
@ -180,7 +182,7 @@ class Subnet(model_base.BASEV2, HasId, HasTenant):
are used for the IP allocation.
"""
name = sa.Column(sa.String(255))
name = sa.Column(sa.String(attr.NAME_MAX_LEN))
network_id = sa.Column(sa.String(36), sa.ForeignKey('networks.id'))
ip_version = sa.Column(sa.Integer, nullable=False)
cidr = sa.Column(sa.String(64), nullable=False)
@ -212,7 +214,7 @@ class Subnet(model_base.BASEV2, HasId, HasTenant):
class Network(model_base.BASEV2, HasId, HasTenant):
"""Represents a v2 neutron network."""
name = sa.Column(sa.String(255))
name = sa.Column(sa.String(attr.NAME_MAX_LEN))
ports = orm.relationship(Port, backref='networks')
subnets = orm.relationship(Subnet, backref='networks',
lazy="joined")

2
neutron/extensions/agent.py
View File

@ -52,7 +52,7 @@ RESOURCE_ATTRIBUTE_MAP = {
'is_visible': True},
'description': {'allow_post': False, 'allow_put': True,
'is_visible': True,
'validate': {'type:string': None}},
'validate': {'type:string': attr.DESCRIPTION_MAX_LEN}},
},
}

9
neutron/extensions/extra_dhcp_opt.py
View File

@ -42,6 +42,10 @@ attr.validators['type:list_of_dict_or_none'] = _validate_list_of_dict_or_none
# Attribute Map
EXTRADHCPOPTS = 'extra_dhcp_opts'
# Common definitions for maximum string field length
DHCP_OPT_NAME_MAX_LEN = 64
DHCP_OPT_VALUE_MAX_LEN = 255
EXTENDED_ATTRIBUTES_2_0 = {
'ports': {
EXTRADHCPOPTS:
@ -52,9 +56,10 @@ EXTENDED_ATTRIBUTES_2_0 = {
'validate': {
'type:list_of_dict_or_none': {
'id': {'type:uuid': None, 'required': False},
'opt_name': {'type:not_empty_string': None,
'opt_name': {'type:not_empty_string': DHCP_OPT_NAME_MAX_LEN,
'required': True},
'opt_value': {'type:not_empty_string_or_none': None,
'opt_value': {'type:not_empty_string_or_none':
DHCP_OPT_VALUE_MAX_LEN,
'required': True},
'ip_version': {'convert_to': attr.convert_to_int,
'type:values': [4, 6],

6
neutron/extensions/l3.py
View File

@ -91,7 +91,7 @@ RESOURCE_ATTRIBUTE_MAP = {
'is_visible': True,
'primary_key': True},
'name': {'allow_post': True, 'allow_put': True,
'validate': {'type:string': None},
'validate': {'type:string': attr.NAME_MAX_LEN},
'is_visible': True, 'default': ''},
'admin_state_up': {'allow_post': True, 'allow_put': True,
'default': True,
@ -101,7 +101,7 @@ RESOURCE_ATTRIBUTE_MAP = {
'is_visible': True},
'tenant_id': {'allow_post': True, 'allow_put': False,
'required_by_policy': True,
'validate': {'type:string': None},
'validate': {'type:string': attr.TENANT_ID_MAX_LEN},
'is_visible': True},
EXTERNAL_GW_INFO: {'allow_post': True, 'allow_put': True,
'is_visible': True, 'default': None,
@ -144,7 +144,7 @@ RESOURCE_ATTRIBUTE_MAP = {
'is_visible': True, 'default': None},
'tenant_id': {'allow_post': True, 'allow_put': False,
'required_by_policy': True,
'validate': {'type:string': None},
'validate': {'type:string': attr.TENANT_ID_MAX_LEN},
'is_visible': True},
'status': {'allow_post': False, 'allow_put': False,
'is_visible': True},

10
neutron/extensions/providernet.py
View File

@ -22,15 +22,21 @@ NETWORK_TYPE = 'provider:network_type'
PHYSICAL_NETWORK = 'provider:physical_network'
SEGMENTATION_ID = 'provider:segmentation_id'
ATTRIBUTES = (NETWORK_TYPE, PHYSICAL_NETWORK, SEGMENTATION_ID)
# Common definitions for maximum string field length
NETWORK_TYPE_MAX_LEN = 32
PHYSICAL_NETWORK_MAX_LEN = 64
EXTENDED_ATTRIBUTES_2_0 = {
'networks': {
NETWORK_TYPE: {'allow_post': True, 'allow_put': True,
'validate': {'type:string': None},
'validate': {'type:string': NETWORK_TYPE_MAX_LEN},
'default': attributes.ATTR_NOT_SPECIFIED,
'enforce_policy': True,
'is_visible': True},
PHYSICAL_NETWORK: {'allow_post': True, 'allow_put': True,
'validate': {'type:string': None},
'validate': {'type:string':
PHYSICAL_NETWORK_MAX_LEN},
'default': attributes.ATTR_NOT_SPECIFIED,
'enforce_policy': True,
'is_visible': True},

3
neutron/extensions/securitygroup.py
View File

@ -193,8 +193,9 @@ RESOURCE_ATTRIBUTE_MAP = {
'primary_key': True},
'name': {'allow_post': True, 'allow_put': True,
'is_visible': True, 'default': '',
'validate': {'type:name_not_default': None}},
'validate': {'type:name_not_default': attr.NAME_MAX_LEN}},
'description': {'allow_post': True, 'allow_put': True,
'validate': {'type:string': attr.DESCRIPTION_MAX_LEN},
'is_visible': True, 'default': ''},
'tenant_id': {'allow_post': True, 'allow_put': False,
'required_by_policy': True,

4
neutron/plugins/nec/extensions/packetfilter.py
View File

@ -99,10 +99,10 @@ PACKET_FILTER_ATTR_PARAMS = {
'validate': {'type:uuid': None},
'is_visible': True},
'name': {'allow_post': True, 'allow_put': True, 'default': '',
'validate': {'type:string': None},
'validate': {'type:string': attributes.NAME_MAX_LEN},
'is_visible': True},
'tenant_id': {'allow_post': True, 'allow_put': False,
'validate': {'type:string': None},
'validate': {'type:string': attributes.TENANT_ID_MAX_LEN},
'required_by_policy': True,
'is_visible': True},
'network_id': {'allow_post': True, 'allow_put': False,

10
neutron/plugins/vmware/extensions/networkgw.py
View File

@ -37,7 +37,7 @@ RESOURCE_ATTRIBUTE_MAP = {
'id': {'allow_post': False, 'allow_put': False,
'is_visible': True},
'name': {'allow_post': True, 'allow_put': True,
'validate': {'type:string': None},
'validate': {'type:string': attributes.NAME_MAX_LEN},
'is_visible': True, 'default': ''},
'default': {'allow_post': False, 'allow_put': False,
'is_visible': True},
@ -48,7 +48,8 @@ RESOURCE_ATTRIBUTE_MAP = {
'default': [],
'is_visible': True},
'tenant_id': {'allow_post': True, 'allow_put': False,
'validate': {'type:string': None},
'validate': {'type:string':
attributes.TENANT_ID_MAX_LEN},
'required_by_policy': True,
'is_visible': True}
},
@ -56,7 +57,7 @@ RESOURCE_ATTRIBUTE_MAP = {
'id': {'allow_post': False, 'allow_put': False,
'is_visible': True},
'name': {'allow_post': True, 'allow_put': True,
'validate': {'type:string': None},
'validate': {'type:string': attributes.NAME_MAX_LEN},
'is_visible': True, 'default': ''},
'client_certificate': {'allow_post': True, 'allow_put': True,
'validate': {'type:string': None},
@ -68,7 +69,8 @@ RESOURCE_ATTRIBUTE_MAP = {
'validate': {'type:ip_address': None},
'is_visible': True},
'tenant_id': {'allow_post': True, 'allow_put': False,
'validate': {'type:string': None},
'validate': {'type:string':
attributes.TENANT_ID_MAX_LEN},
'required_by_policy': True,
'is_visible': True},
'status': {'allow_post': False, 'allow_put': False,

4
neutron/plugins/vmware/extensions/qos.py
View File

@ -112,7 +112,7 @@ RESOURCE_ATTRIBUTE_MAP = {
'convert_to': attr.convert_to_boolean,
'is_visible': True, 'default': False},
'name': {'allow_post': True, 'allow_put': False,
'validate': {'type:string': None},
'validate': {'type:string': attr.NAME_MAX_LEN},
'is_visible': True, 'default': ''},
'min': {'allow_post': True, 'allow_put': False,
'is_visible': True, 'default': '0',
@ -128,7 +128,7 @@ RESOURCE_ATTRIBUTE_MAP = {
'convert_to': convert_to_unsigned_int_or_none_max_63},
'tenant_id': {'allow_post': True, 'allow_put': False,
'required_by_policy': True,
'validate': {'type:string': None},
'validate': {'type:string': attr.TENANT_ID_MAX_LEN},
'is_visible': True},
},
}

10
neutron/tests/unit/test_api_v2.py
View File

@ -844,6 +844,16 @@ class JSONV2TestCase(APIv2TestBase, testlib_api.WebTestCase):
'status': "ACTIVE"}}
self._test_create_failure_bad_request('networks', data)
def test_create_with_too_long_name(self):
data = {'network': {'name': "12345678" * 32,
'admin_state_up': True,
'tenant_id': _uuid()}}
res = self.api.post(_get_path('networks', fmt=self.fmt),
self.serialize(data),
content_type='application/' + self.fmt,
expect_errors=True)
self.assertEqual(res.status_int, exc.HTTPBadRequest.code)
def test_create_bulk(self):
data = {'networks': [{'name': 'net1',
'admin_state_up': True,