Skip to content

Commit

Permalink
bpo-28009: Fix uuid SkipUnless logic to be based on platform programs…
Browse files Browse the repository at this point in the history
… capable of introspection (pythonGH-12777)

uuid could try fallback methods that had no chance of working on a particular
platform, and this could cause spurious test failures, as well as degraded
performance as fallback options were tried and failed.

This fixes both the uuid module and its test's SkipUnless logic to use a
prefiltered list of techniques that may at least potentially work on that platform.

Patch by Michael Felt (aixtools).
  • Loading branch information
aixtools authored and lisroach committed Sep 9, 2019
1 parent d7d6b1c commit 0619ae5
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 19 deletions.
20 changes: 12 additions & 8 deletions Lib/test/test_uuid.py
Original file line number Diff line number Diff line change
Expand Up @@ -462,8 +462,7 @@ def test_uuid1_eui64(self):
with unittest.mock.patch.multiple(
self.uuid,
_node=None, # Ignore any cached node value.
_NODE_GETTERS_WIN32=[too_large_getter],
_NODE_GETTERS_UNIX=[too_large_getter],
_GETTERS=[too_large_getter],
):
node = self.uuid.getnode()
self.assertTrue(0 < node < (1 << 48), '%012x' % node)
Expand Down Expand Up @@ -673,7 +672,7 @@ class TestUUIDWithExtModule(BaseTestUUID, unittest.TestCase):


class BaseTestInternals:
uuid = None
_uuid = py_uuid

@unittest.skipUnless(os.name == 'posix', 'requires Posix')
def test_find_mac(self):
Expand Down Expand Up @@ -708,27 +707,32 @@ def check_node(self, node, requires=None):
self.assertTrue(0 < node < (1 << 48),
"%s is not an RFC 4122 node ID" % hex)

@unittest.skipUnless(os.name == 'posix', 'requires Posix')
@unittest.skipUnless(_uuid._ifconfig_getnode in _uuid._GETTERS,
"ifconfig is not used for introspection on this platform")
def test_ifconfig_getnode(self):
node = self.uuid._ifconfig_getnode()
self.check_node(node, 'ifconfig')

@unittest.skipUnless(os.name == 'posix', 'requires Posix')
@unittest.skipUnless(_uuid._ip_getnode in _uuid._GETTERS,
"ip is not used for introspection on this platform")
def test_ip_getnode(self):
node = self.uuid._ip_getnode()
self.check_node(node, 'ip')

@unittest.skipUnless(os.name == 'posix', 'requires Posix')
@unittest.skipUnless(_uuid._arp_getnode in _uuid._GETTERS,
"arp is not used for introspection on this platform")
def test_arp_getnode(self):
node = self.uuid._arp_getnode()
self.check_node(node, 'arp')

@unittest.skipUnless(os.name == 'posix', 'requires Posix')
@unittest.skipUnless(_uuid._lanscan_getnode in _uuid._GETTERS,
"lanscan is not used for introspection on this platform")
def test_lanscan_getnode(self):
node = self.uuid._lanscan_getnode()
self.check_node(node, 'lanscan')

@unittest.skipUnless(os.name == 'posix', 'requires Posix')
@unittest.skipUnless(_uuid._netstat_getnode in _uuid._GETTERS,
"netstat is not used for introspection on this platform")
def test_netstat_getnode(self):
node = self.uuid._netstat_getnode()
self.check_node(node, 'netstat')
Expand Down
43 changes: 32 additions & 11 deletions Lib/uuid.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,20 @@
"""

import os
import platform
import sys

from enum import Enum


__author__ = 'Ka-Ping Yee <ping@zesty.ca>'

# The recognized platforms - known behaviors
_AIX = platform.system() == 'AIX'
_DARWIN = platform.system() == 'Darwin'
_LINUX = platform.system() == 'Linux'
_WINDOWS = platform.system() == 'Windows'

RESERVED_NCS, RFC_4122, RESERVED_MICROSOFT, RESERVED_FUTURE = [
'reserved for NCS compatibility', 'specified in RFC 4122',
'reserved for Microsoft compatibility', 'reserved for future definition']
Expand Down Expand Up @@ -673,12 +680,31 @@ def _random_getnode():
return random.getrandbits(48) | (1 << 40)


_node = None

_NODE_GETTERS_WIN32 = [_windll_getnode, _netbios_getnode, _ipconfig_getnode]
# _OS_GETTERS, when known, are targetted for a specific OS or platform.
# The order is by 'common practice' on the specified platform.
# Note: 'posix' and 'windows' _OS_GETTERS are prefixed by a dll/dlload() method
# which, when successful, means none of these "external" methods are called.
# _GETTERS is (also) used by test_uuid.py to SkipUnless(), e.g.,
# @unittest.skipUnless(_uuid._ifconfig_getnode in _uuid._GETTERS, ...)
if _LINUX:
_OS_GETTERS = [_ip_getnode, _ifconfig_getnode]
elif _DARWIN:
_OS_GETTERS = [_ifconfig_getnode, _arp_getnode, _netstat_getnode]
elif _WINDOWS:
_OS_GETTERS = [_netbios_getnode, _ipconfig_getnode]
elif _AIX:
_OS_GETTERS = [_netstat_getnode]
else:
_OS_GETTERS = [_ifconfig_getnode, _ip_getnode, _arp_getnode,
_netstat_getnode, _lanscan_getnode]
if os.name == 'posix':
_GETTERS = [_unix_getnode] + _OS_GETTERS
elif os.name == 'nt':
_GETTERS = [_windll_getnode] + _OS_GETTERS
else:
_GETTERS = _OS_GETTERS

_NODE_GETTERS_UNIX = [_unix_getnode, _ifconfig_getnode, _ip_getnode,
_arp_getnode, _lanscan_getnode, _netstat_getnode]
_node = None

def getnode(*, getters=None):
"""Get the hardware address as a 48-bit positive integer.
Expand All @@ -692,12 +718,7 @@ def getnode(*, getters=None):
if _node is not None:
return _node

if sys.platform == 'win32':
getters = _NODE_GETTERS_WIN32
else:
getters = _NODE_GETTERS_UNIX

for getter in getters + [_random_getnode]:
for getter in _GETTERS + [_random_getnode]:
try:
_node = getter()
except:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Modify the test_uuid logic to test when a program is available
AND can be used to obtain a MACADDR as basis for an UUID.
Patch by M. Felt

0 comments on commit 0619ae5

Please sign in to comment.