[openstack-dev] [Horizon] Redundant checks in form values

Suraj Deshmukh surajdeshmukh9595 at gmail.com
Thu Nov 19 04:25:19 UTC 2015


On Thu, Nov 19, 2015 at 3:10 AM, Akihiro Motoki <amotoki at gmail.com> wrote:
> In the current AddRule form, validate_port_range is used as a validator,
> so a value from 1 to 65535 is accepted.
> I think this is the reason that _clean_rule_icmp check the exact value range.
> You can improve it as you think.
>
> bug 1511748 is a regression of validate_port_range change in
> https://review.openstack.org/#/c/116508/.
> Previously it accepts -1 and we can use it both for TCP/UDP and ICMP,
> but after that change it no longer cannot be applied to ICMP.
>
> IMO we need to use separate validators for TCP/UDP and ICMP.
>
> Akihiro
>

Yes we need seperate validators for ICMP's `icmp_type` and
`icmp_code`, I figured that out earlier and hence wrote validator code
in `oslo_utils.netutils` [1] and [2].

So `icmp_type` can be any integer between 0 to 255 and `icmp_code` can
be either None or integer between 0 to 255, so code at [2] does take
care of that. More information on ICMP can be found at [3].

So `_clean_rule_icmp` checks related to `icmp_type` and `icmp_code`
can go away. Since while validating itself Django sets the data to
None if `ValidationError` is raised by validator.


[1] https://review.openstack.org/#/c/240661/
[2] https://github.com/openstack/oslo.utils/blob/master/oslo_utils/netutils.py#L207
[3] http://www.nthelp.com/icmp.html



More information about the OpenStack-dev mailing list