diff --git a/horizon/test/tests/utils.py b/horizon/test/tests/utils.py index 7077bfcddc..c6bbb8b832 100644 --- a/horizon/test/tests/utils.py +++ b/horizon/test/tests/utils.py @@ -206,6 +206,30 @@ class ValidatorsTests(test.TestCase): validators.validate_port_range, port) + def test_icmp_type_validator(self): + VALID_ICMP_TYPES = (1, 0, 255, -1) + INVALID_ICMP_TYPES = (256, None, -2) + + for icmp_type in VALID_ICMP_TYPES: + self.assertIsNone(validators.validate_icmp_type_range(icmp_type)) + + for icmp_type in INVALID_ICMP_TYPES: + self.assertRaises(ValidationError, + validators.validate_icmp_type_range, + icmp_type) + + def test_icmp_code_validator(self): + VALID_ICMP_CODES = (1, 0, 255, None, -1,) + INVALID_ICMP_CODES = (256, -2) + + for icmp_code in VALID_ICMP_CODES: + self.assertIsNone(validators.validate_icmp_code_range(icmp_code)) + + for icmp_code in INVALID_ICMP_CODES: + self.assertRaises(ValidationError, + validators.validate_icmp_code_range, + icmp_code) + def test_ip_proto_validator(self): VALID_PROTO = (0, 255) INVALID_PROTO = (-1, 256) diff --git a/horizon/utils/validators.py b/horizon/utils/validators.py index d2103bf0c8..c9cd662e61 100644 --- a/horizon/utils/validators.py +++ b/horizon/utils/validators.py @@ -28,6 +28,20 @@ def validate_port_range(port): raise ValidationError(_("Not a valid port number")) +def validate_icmp_type_range(icmp_type): + if not netutils.is_valid_icmp_type(icmp_type): + if icmp_type == -1: + return + raise ValidationError(_("Not a valid ICMP type")) + + +def validate_icmp_code_range(icmp_code): + if not netutils.is_valid_icmp_code(icmp_code): + if icmp_code == -1: + return + raise ValidationError(_("Not a valid ICMP code")) + + def validate_ip_protocol(ip_proto): if ip_proto not in range(0, 256): raise ValidationError(_("Not a valid IP protocol number")) diff --git a/openstack_dashboard/dashboards/project/access_and_security/security_groups/forms.py b/openstack_dashboard/dashboards/project/access_and_security/security_groups/forms.py index b3ad2de54f..3870a08e28 100644 --- a/openstack_dashboard/dashboards/project/access_and_security/security_groups/forms.py +++ b/openstack_dashboard/dashboards/project/access_and_security/security_groups/forms.py @@ -189,7 +189,8 @@ class AddRule(forms.SelfHandlingForm): 'data-switch-on': 'rule_menu', 'data-rule_menu-icmp': _('Type')}), validators=[ - utils_validators.validate_port_range]) + utils_validators. + validate_icmp_type_range]) icmp_code = forms.IntegerField(label=_("Code"), required=True, @@ -200,7 +201,8 @@ class AddRule(forms.SelfHandlingForm): 'data-switch-on': 'rule_menu', 'data-rule_menu-icmp': _('Code')}), validators=[ - utils_validators.validate_port_range]) + utils_validators. + validate_icmp_code_range]) remote = forms.ChoiceField(label=_('Remote'), choices=[('cidr', _('CIDR')), @@ -293,18 +295,16 @@ class AddRule(forms.SelfHandlingForm): icmp_code = cleaned_data.get("icmp_code", None) self._update_and_pop_error(cleaned_data, 'ip_protocol', rule_menu) - if icmp_type is None: - msg = _('The ICMP type is invalid.') + if icmp_type == -1 and icmp_code != -1: + msg = _('ICMP code is provided but ICMP type is missing.') raise ValidationError(msg) - if icmp_code is None: - msg = _('The ICMP code is invalid.') - raise ValidationError(msg) - if icmp_type not in range(-1, 256): + if self.errors.get('icmp_type'): msg = _('The ICMP type not in range (-1, 255)') raise ValidationError(msg) - if icmp_code not in range(-1, 256): + if self.errors.get('icmp_code'): msg = _('The ICMP code not in range (-1, 255)') raise ValidationError(msg) + self._update_and_pop_error(cleaned_data, 'from_port', icmp_type) self._update_and_pop_error(cleaned_data, 'to_port', icmp_code) self._update_and_pop_error(cleaned_data, 'port', None) diff --git a/openstack_dashboard/dashboards/project/access_and_security/security_groups/tests.py b/openstack_dashboard/dashboards/project/access_and_security/security_groups/tests.py index 58fe1e21a6..cbe4e64661 100644 --- a/openstack_dashboard/dashboards/project/access_and_security/security_groups/tests.py +++ b/openstack_dashboard/dashboards/project/access_and_security/security_groups/tests.py @@ -478,20 +478,17 @@ class SecurityGroupsViewTests(test.TestCase): sec_group_list = self.security_groups.list() icmp_rule = self.security_group_rules.list()[1] - # Call POST 4 times - for i in range(4): + # Call POST 5 times (*2 if Django >= 1.9) + call_post = 5 + if django.VERSION >= (1, 9): + call_post *= 2 + + for i in range(call_post): api.network.security_group_backend( IsA(http.HttpRequest)).AndReturn(self.secgroup_backend) api.network.security_group_list( IsA(http.HttpRequest)).AndReturn(sec_group_list) - if django.VERSION >= (1, 9): - for i in range(4): - api.network.security_group_backend( - IsA(http.HttpRequest)).AndReturn(self.secgroup_backend) - api.network.security_group_list( - IsA(http.HttpRequest)).AndReturn(sec_group_list) - self.mox.ReplayAll() formData = {'method': 'AddRule', @@ -528,7 +525,7 @@ class SecurityGroupsViewTests(test.TestCase): 'remote': 'cidr'} res = self.client.post(self.edit_url, formData) self.assertNoMessages() - self.assertContains(res, "The ICMP code is invalid") + self.assertContains(res, "The ICMP code not in range (-1, 255)") formData = {'method': 'AddRule', 'id': sec_group.id, @@ -540,7 +537,20 @@ class SecurityGroupsViewTests(test.TestCase): 'remote': 'cidr'} res = self.client.post(self.edit_url, formData) self.assertNoMessages() - self.assertContains(res, "The ICMP type is invalid") + self.assertContains(res, "The ICMP type not in range (-1, 255)") + + formData = {'method': 'AddRule', + 'id': sec_group.id, + 'port_or_range': 'port', + 'icmp_type': -1, + 'icmp_code': icmp_rule.to_port, + 'rule_menu': icmp_rule.ip_protocol, + 'cidr': icmp_rule.ip_range['cidr'], + 'remote': 'cidr'} + res = self.client.post(self.edit_url, formData) + self.assertNoMessages() + self.assertContains( + res, "ICMP code is provided but ICMP type is missing.") @test.create_stubs({api.network: ('security_group_rule_create', 'security_group_list',