From b9390333a146d0435710f85d365318268e267fc3 Mon Sep 17 00:00:00 2001 From: Aaron Rosen Date: Fri, 14 Mar 2014 22:57:09 -0700 Subject: [PATCH] Fix _validate_mac_address method The method _validate_mac_address previously used: netaddr.EUI(data) which would convert data passed in to the correct mac_address representation. For example if data=123 it would return: EUI('00-00-00-00-00-7B'). To fix this issue I used netaddr.valid_mac() instead which returns a bool if a mac is valid or not and does not raise. Note this file needs some improvements to improve how exception handling is done that will come later. Change-Id: I4fb868e40abbad9e743028cc768f47ca9b3e0e70 Closes-bug: 1291163 --- neutron/api/v2/attributes.py | 15 +++++++++++---- neutron/tests/unit/test_attributes.py | 9 +++++++-- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/neutron/api/v2/attributes.py b/neutron/api/v2/attributes.py index 1b90bb73a..5be3d16f9 100644 --- a/neutron/api/v2/attributes.py +++ b/neutron/api/v2/attributes.py @@ -153,12 +153,19 @@ def _validate_no_whitespace(data): def _validate_mac_address(data, valid_values=None): + valid_mac = False try: - netaddr.EUI(_validate_no_whitespace(data)) + valid_mac = netaddr.valid_mac(_validate_no_whitespace(data)) except Exception: - msg = _("'%s' is not a valid MAC address") % data - LOG.debug(msg) - return msg + pass + finally: + # TODO(arosen): The code in this file should be refactored + # so it catches the correct exceptions. _validate_no_whitespace + # raises AttributeError if data is None. + if valid_mac is False: + msg = _("'%s' is not a valid MAC address") % data + LOG.debug(msg) + return msg def _validate_mac_address_or_none(data, valid_values=None): diff --git a/neutron/tests/unit/test_attributes.py b/neutron/tests/unit/test_attributes.py index b4a44a6ed..bab45fde0 100644 --- a/neutron/tests/unit/test_attributes.py +++ b/neutron/tests/unit/test_attributes.py @@ -176,14 +176,19 @@ class TestAttributes(base.BaseTestCase): mac_addr = "ffa:16:3e:4f:00:00" msg = validator(mac_addr) - self.assertEqual(msg, "'%s' is not a valid MAC address" % mac_addr) + err_msg = "'%s' is not a valid MAC address" + self.assertEqual(msg, err_msg % mac_addr) + + mac_addr = "123" + msg = validator(mac_addr) + self.assertEqual(msg, err_msg % mac_addr) mac_addr = None msg = validator(mac_addr) if allow_none: self.assertIsNone(msg) else: - self.assertEqual(msg, "'None' is not a valid MAC address") + self.assertEqual(msg, err_msg % mac_addr) def test_validate_mac_address(self): self._test_validate_mac_address(attributes._validate_mac_address)