Validate string length at API level

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

diff --git a/neutron/api/v2/attributes.py b/neutron/api/v2/attributes.py
index 7fb4a44097300e1c9ecc92c5bb296a7ceb51a663..4e17d4c50282b98c07e3c89114ed4c3acbce5f66 100644 (file)
--- a/neutron/api/v2/attributes.py
+++ b/neutron/api/v2/attributes.py
@@ -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,

diff --git a/neutron/db/models_v2.py b/neutron/db/models_v2.py
index c0578003970d06df92eaf00c44f5d72a6be0b04d..1b83190b645e0efa91c8c24d23304b2f5ebad8b8 100644 (file)
--- a/neutron/db/models_v2.py
+++ b/neutron/db/models_v2.py
@@ -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")

diff --git a/neutron/extensions/agent.py b/neutron/extensions/agent.py
index 0d4224787a31fb0b6daa1d8a8b0a2e0456630817..4dee27c6ed73e9ca584fcd94b994d384db53376f 100644 (file)
--- a/neutron/extensions/agent.py
+++ b/neutron/extensions/agent.py
@@ -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}},
     },
 }
 

diff --git a/neutron/extensions/extra_dhcp_opt.py b/neutron/extensions/extra_dhcp_opt.py
index 20370de1d676c9c7775091bff1769a1c9a1fc594..6bc8d6c3adfae069472b8da7284bc88e70b51fce 100644 (file)
--- a/neutron/extensions/extra_dhcp_opt.py
+++ b/neutron/extensions/extra_dhcp_opt.py
@@ -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],

diff --git a/neutron/extensions/l3.py b/neutron/extensions/l3.py
index ac9b07d522c3e7e1eba596a5d8bb9bcea17c84c7..e6c566f569b07db98685caacbeae7e287e1dc593 100644 (file)
--- a/neutron/extensions/l3.py
+++ b/neutron/extensions/l3.py
@@ -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},

diff --git a/neutron/extensions/providernet.py b/neutron/extensions/providernet.py
index 4bb19fac75d7606deb94efe295428ee5b9adc346..6a60ca9a22d77855a662029122e4109144fce448 100644 (file)
--- a/neutron/extensions/providernet.py
+++ b/neutron/extensions/providernet.py
@@ -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},

diff --git a/neutron/extensions/securitygroup.py b/neutron/extensions/securitygroup.py
index 39b25d6b76ef8a2c35c9f5baf6a94cf8af80ba25..7cb738ae837d46668edac9a025c47a912ff7af2b 100644 (file)
--- a/neutron/extensions/securitygroup.py
+++ b/neutron/extensions/securitygroup.py
@@ -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,

diff --git a/neutron/plugins/nec/extensions/packetfilter.py b/neutron/plugins/nec/extensions/packetfilter.py
index e6384f478c0312169c0770e08417a5335a12c53c..3711995ea347b9bd5bf660a04d7de2c1af28e939 100644 (file)
--- a/neutron/plugins/nec/extensions/packetfilter.py
+++ b/neutron/plugins/nec/extensions/packetfilter.py
@@ -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,

diff --git a/neutron/plugins/vmware/extensions/networkgw.py b/neutron/plugins/vmware/extensions/networkgw.py
index 1be3809b5db4885b9aba789875932b2d89b73efb..851c7553d2a3e116276fa1d170a99729cf0017c0 100644 (file)
--- a/neutron/plugins/vmware/extensions/networkgw.py
+++ b/neutron/plugins/vmware/extensions/networkgw.py
@@ -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,

diff --git a/neutron/plugins/vmware/extensions/qos.py b/neutron/plugins/vmware/extensions/qos.py
index 54cc64dccc474fdfa94ecb9ac18c8fc8ff6fa18a..30e6f8c7d023acc095774ccfa4027dfc776394e1 100644 (file)
--- a/neutron/plugins/vmware/extensions/qos.py
+++ b/neutron/plugins/vmware/extensions/qos.py
@@ -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},
     },
 }

diff --git a/neutron/tests/unit/test_api_v2.py b/neutron/tests/unit/test_api_v2.py
index ecbbbcc975d702e6c279ea6d2e3a13c4b0e6c259..a23157d891ed42d5836011ef715496538c697c50 100644 (file)
--- a/neutron/tests/unit/test_api_v2.py
+++ b/neutron/tests/unit/test_api_v2.py
@@ -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,