From b0aa83aca39bc268e2ad82019ac16c4e9499662e Mon Sep 17 00:00:00 2001 From: "hobo.kengo" Date: Mon, 8 Aug 2016 02:23:54 +0000 Subject: [PATCH] Add string validation on security group's name This patch disallows cases that following name is specified. 1. name whose type is not String. 2. name whose characters is more than 255. Change-Id: Ib72a4e480b62a22da2171ed24449321f2b27258b Closes-Bug: #1610764 --- neutron/extensions/securitygroup.py | 5 +- .../api/test_security_groups_negative.py | 35 ++++++++++++ .../unit/extensions/test_securitygroup.py | 54 +++++++++++++++++++ 3 files changed, 93 insertions(+), 1 deletion(-) diff --git a/neutron/extensions/securitygroup.py b/neutron/extensions/securitygroup.py index fd8f7987329..a4b5172d40e 100644 --- a/neutron/extensions/securitygroup.py +++ b/neutron/extensions/securitygroup.py @@ -194,7 +194,10 @@ def convert_ip_prefix_to_cidr(ip_prefix): raise exceptions.InvalidCIDR(input=ip_prefix) -def _validate_name_not_default(data, valid_values=None): +def _validate_name_not_default(data, max_len=db_const.NAME_FIELD_SIZE): + msg = validators.validate_string(data, max_len) + if msg: + return msg if data.lower() == "default": raise SecurityGroupDefaultAlreadyExists() diff --git a/neutron/tests/tempest/api/test_security_groups_negative.py b/neutron/tests/tempest/api/test_security_groups_negative.py index 9202060363e..1d841b15c4c 100644 --- a/neutron/tests/tempest/api/test_security_groups_negative.py +++ b/neutron/tests/tempest/api/test_security_groups_negative.py @@ -13,12 +13,15 @@ # License for the specific language governing permissions and limitations # under the License. +from neutron_lib.db import constants as db_const from tempest.lib import decorators from tempest.lib import exceptions as lib_exc from tempest import test from neutron.tests.tempest.api import base_security_groups as base +LONG_NAME_NG = 'x' * (db_const.NAME_FIELD_SIZE + 1) + class NegativeSecGroupTest(base.BaseSecGroupTest): @@ -27,6 +30,20 @@ class NegativeSecGroupTest(base.BaseSecGroupTest): def resource_setup(cls): super(NegativeSecGroupTest, cls).resource_setup() + @test.attr(type='negative') + @decorators.idempotent_id('594edfa8-9a5b-438e-9344-49aece337d49') + def test_create_security_group_with_too_long_name(self): + self.assertRaises(lib_exc.BadRequest, + self.client.create_security_group, + name=LONG_NAME_NG) + + @test.attr(type='negative') + @decorators.idempotent_id('b6b79838-7430-4d3f-8e07-51dfb61802c2') + def test_create_security_group_with_boolean_type_name(self): + self.assertRaises(lib_exc.BadRequest, + self.client.create_security_group, + name=True) + @test.attr(type='negative') @decorators.idempotent_id('55100aa8-b24f-333c-0bef-64eefd85f15c') def test_update_default_security_group_name(self): @@ -35,6 +52,24 @@ class NegativeSecGroupTest(base.BaseSecGroupTest): self.assertRaises(lib_exc.Conflict, self.client.update_security_group, sg['id'], name='test') + @test.attr(type='negative') + @decorators.idempotent_id('c8510dd8-c3a8-4df9-ae44-24354db50960') + def test_update_security_group_with_too_long_name(self): + sg_list = self.client.list_security_groups(name='default') + sg = sg_list['security_groups'][0] + self.assertRaises(lib_exc.BadRequest, + self.client.update_security_group, + sg['id'], name=LONG_NAME_NG) + + @test.attr(type='negative') + @decorators.idempotent_id('d9a14917-f66f-4eca-ab72-018563917f1b') + def test_update_security_group_with_boolean_type_name(self): + sg_list = self.client.list_security_groups(name='default') + sg = sg_list['security_groups'][0] + self.assertRaises(lib_exc.BadRequest, + self.client.update_security_group, + sg['id'], name=True) + class NegativeSecGroupIPv6Test(NegativeSecGroupTest): _ip_version = 6 diff --git a/neutron/tests/unit/extensions/test_securitygroup.py b/neutron/tests/unit/extensions/test_securitygroup.py index 0e238180c39..fd4840f8251 100644 --- a/neutron/tests/unit/extensions/test_securitygroup.py +++ b/neutron/tests/unit/extensions/test_securitygroup.py @@ -19,6 +19,7 @@ import mock from neutron_lib.api import validators from neutron_lib import constants as const from neutron_lib import context +from neutron_lib.db import constants as db_const from neutron_lib.plugins import directory from oslo_config import cfg import oslo_db.exception as exc @@ -37,6 +38,8 @@ from neutron.tests.unit.db import test_db_base_plugin_v2 DB_PLUGIN_KLASS = ('neutron.tests.unit.extensions.test_securitygroup.' 'SecurityGroupTestPlugin') +LONG_NAME_OK = 'x' * (db_const.NAME_FIELD_SIZE) +LONG_NAME_NG = 'x' * (db_const.NAME_FIELD_SIZE + 1) class SecurityGroupTestExtensionManager(object): @@ -378,6 +381,39 @@ class TestSecurityGroups(SecurityGroupDBTestCase): self.assertEqual(data['security_group']['description'], res['security_group']['description']) + def test_update_security_group_with_max_name_length(self): + with self.security_group() as sg: + data = {'security_group': {'name': LONG_NAME_OK, + 'description': 'new_desc'}} + req = self.new_update_request('security-groups', + data, + sg['security_group']['id']) + res = self.deserialize(self.fmt, req.get_response(self.ext_api)) + self.assertEqual(data['security_group']['name'], + res['security_group']['name']) + self.assertEqual(data['security_group']['description'], + res['security_group']['description']) + + def test_update_security_group_with_too_long_name(self): + with self.security_group() as sg: + data = {'security_group': {'name': LONG_NAME_NG, + 'description': 'new_desc'}} + req = self.new_update_request('security-groups', + data, + sg['security_group']['id']) + res = req.get_response(self.ext_api) + self.assertEqual(webob.exc.HTTPBadRequest.code, res.status_int) + + def test_update_security_group_with_boolean_type_name(self): + with self.security_group() as sg: + data = {'security_group': {'name': True, + 'description': 'new_desc'}} + req = self.new_update_request('security-groups', + data, + sg['security_group']['id']) + res = req.get_response(self.ext_api) + self.assertEqual(webob.exc.HTTPBadRequest.code, res.status_int) + def test_check_default_security_group_description(self): with self.network(): res = self.new_list_request('security-groups') @@ -405,6 +441,24 @@ class TestSecurityGroups(SecurityGroupDBTestCase): self.deserialize(self.fmt, res) self.assertEqual(webob.exc.HTTPConflict.code, res.status_int) + def test_create_security_group_with_max_name_length(self): + description = 'my webservers' + res = self._create_security_group(self.fmt, LONG_NAME_OK, description) + self.deserialize(self.fmt, res) + self.assertEqual(webob.exc.HTTPCreated.code, res.status_int) + + def test_create_security_group_with_too_long_name(self): + description = 'my webservers' + res = self._create_security_group(self.fmt, LONG_NAME_NG, description) + self.deserialize(self.fmt, res) + self.assertEqual(webob.exc.HTTPBadRequest.code, res.status_int) + + def test_create_security_group_with_boolean_type_name(self): + description = 'my webservers' + res = self._create_security_group(self.fmt, True, description) + self.deserialize(self.fmt, res) + self.assertEqual(webob.exc.HTTPBadRequest.code, res.status_int) + def test_list_security_groups(self): with self.security_group(name='sg1', description='sg') as v1,\ self.security_group(name='sg2', description='sg') as v2,\