From 934ccef0bd0fb3acf5a20160478a989c1bcba017 Mon Sep 17 00:00:00 2001 From: He Jie Xu Date: Tue, 27 Dec 2016 14:27:42 +0800 Subject: [PATCH] Limit the min length of string for integer JSON-Schema The JSON-Schema of integer string is without min length limits. That leads to the user can input empty string, and nova API code can't handle that case. Finally the user will get 500 returned. The create_backup and multi_create API are fixed. The networks and tenant_networks API have same problem on few parameters, but due to the nova-network is deprecated, so just keep those nova-network specific API behaviour as before. Change-Id: I521e914fc48b7a221431f41c567f2cb4b9b4ab99 Closes-bug: #1652719 --- nova/api/openstack/compute/schemas/networks.py | 6 +++--- .../api/openstack/compute/schemas/tenant_networks.py | 7 ++++--- nova/api/validation/parameter_types.py | 10 ++++++++-- .../unit/api/openstack/compute/test_create_backup.py | 12 ++++++++++++ .../api/openstack/compute/test_multiple_create.py | 8 ++++---- 5 files changed, 31 insertions(+), 12 deletions(-) diff --git a/nova/api/openstack/compute/schemas/networks.py b/nova/api/openstack/compute/schemas/networks.py index f91dfdb199ce..2fd687094391 100644 --- a/nova/api/openstack/compute/schemas/networks.py +++ b/nova/api/openstack/compute/schemas/networks.py @@ -48,9 +48,9 @@ create = { 'allowed_end': parameter_types.ip_address, 'enable_dhcp': parameter_types.boolean, 'share_address': parameter_types.boolean, - 'mtu': parameter_types.positive_integer, - 'vlan': parameter_types.positive_integer, - 'vlan_start': parameter_types.positive_integer, + 'mtu': parameter_types.positive_integer_with_empty_str, + 'vlan': parameter_types.positive_integer_with_empty_str, + 'vlan_start': parameter_types.positive_integer_with_empty_str, 'vpn_start': { 'type': 'string', }, diff --git a/nova/api/openstack/compute/schemas/tenant_networks.py b/nova/api/openstack/compute/schemas/tenant_networks.py index fd749d7d4645..f5f4c03ac3aa 100644 --- a/nova/api/openstack/compute/schemas/tenant_networks.py +++ b/nova/api/openstack/compute/schemas/tenant_networks.py @@ -26,9 +26,10 @@ create = { 'ipam': parameter_types.boolean, 'cidr': parameter_types.cidr, 'cidr_v6': parameter_types.cidr, - 'vlan_start': parameter_types.positive_integer, - 'network_size': parameter_types.positive_integer, - 'num_networks': parameter_types.positive_integer + 'vlan_start': parameter_types.positive_integer_with_empty_str, + 'network_size': + parameter_types.positive_integer_with_empty_str, + 'num_networks': parameter_types.positive_integer_with_empty_str }, 'required': ['label'], 'oneOf': [ diff --git a/nova/api/validation/parameter_types.py b/nova/api/validation/parameter_types.py index c096a5e4e4d7..15935cbd8265 100644 --- a/nova/api/validation/parameter_types.py +++ b/nova/api/validation/parameter_types.py @@ -201,15 +201,21 @@ none = { positive_integer = { 'type': ['integer', 'string'], - 'pattern': '^[0-9]*$', 'minimum': 1 + 'pattern': '^[0-9]*$', 'minimum': 1, 'minLength': 1 } non_negative_integer = { 'type': ['integer', 'string'], - 'pattern': '^[0-9]*$', 'minimum': 0 + 'pattern': '^[0-9]*$', 'minimum': 0, 'minLength': 1 } +# This only be used by nova-network specific APIs. It will be removed when +# those API removed. +positive_integer_with_empty_str = { + 'type': ['integer', 'string'], + 'pattern': '^[0-9]*$', 'minimum': 1, +} hostname = { 'type': 'string', 'minLength': 1, 'maxLength': 255, diff --git a/nova/tests/unit/api/openstack/compute/test_create_backup.py b/nova/tests/unit/api/openstack/compute/test_create_backup.py index 2b43eb51da67..d2bd23e6faed 100644 --- a/nova/tests/unit/api/openstack/compute/test_create_backup.py +++ b/nova/tests/unit/api/openstack/compute/test_create_backup.py @@ -165,6 +165,18 @@ class CreateBackupTestsV21(admin_only_action_common.CommonMixin, self.controller._create_backup, self.req, fakes.FAKE_UUID, body=body) + def test_create_backup_rotation_with_empty_string(self): + body = { + 'createBackup': { + 'name': 'Backup 1', + 'backup_type': 'daily', + 'rotation': '', + }, + } + self.assertRaises(self.validation_error, + self.controller._create_backup, + self.req, fakes.FAKE_UUID, body=body) + def test_create_backup_no_backup_type(self): # Backup Type (daily or weekly) is required for backup requests. body = { diff --git a/nova/tests/unit/api/openstack/compute/test_multiple_create.py b/nova/tests/unit/api/openstack/compute/test_multiple_create.py index a869e805eda3..fc699b946f36 100644 --- a/nova/tests/unit/api/openstack/compute/test_multiple_create.py +++ b/nova/tests/unit/api/openstack/compute/test_multiple_create.py @@ -185,8 +185,8 @@ class MultiCreateExtensionTestV21(test.TestCase): 'server': { multiple_create_v21.MIN_ATTRIBUTE_NAME: '', 'name': 'server_test', - 'image_ref': image_href, - 'flavor_ref': flavor_ref, + 'imageRef': image_href, + 'flavorRef': flavor_ref, } } self.assertRaises(self.validation_error, @@ -202,8 +202,8 @@ class MultiCreateExtensionTestV21(test.TestCase): 'server': { multiple_create_v21.MAX_ATTRIBUTE_NAME: '', 'name': 'server_test', - 'image_ref': image_href, - 'flavor_ref': flavor_ref, + 'imageRef': image_href, + 'flavorRef': flavor_ref, } } self.assertRaises(self.validation_error,