bpo-28009: Fix uuid.uuid1() and uuid.get_node() on AIX (GH-8672)

3 files changed, 177 insertions, 93 deletions

diff --git a/Lib/test/test_uuid.py b/Lib/test/test_uuid.py
index 92642d2..ddf7e6d 100644
--- a/Lib/test/test_uuid.py
+++ b/Lib/test/test_uuid.py
@@ -1,4 +1,4 @@
-import unittest.mock
+import unittest
 from test import support
 import builtins
 import contextlib
@@ -15,7 +15,6 @@ from unittest import mock
 py_uuid = support.import_fresh_module('uuid', blocked=['_uuid'])
 c_uuid = support.import_fresh_module('uuid', fresh=['_uuid'])
 
-
 def importable(name):
     try:
         __import__(name)
@@ -459,7 +458,7 @@ class BaseTestUUID:
         # uuid.getnode to fall back on uuid._random_getnode, which will
         # generate a valid value.
         too_large_getter = lambda: 1 << 48
-        with unittest.mock.patch.multiple(
+        with mock.patch.multiple(
             self.uuid,
             _node=None,  # Ignore any cached node value.
             _GETTERS=[too_large_getter],
@@ -538,8 +537,8 @@ class BaseTestUUID:
         f = self.uuid._generate_time_safe
         if f is None:
             self.skipTest('need uuid._generate_time_safe')
-        with unittest.mock.patch.object(self.uuid, '_generate_time_safe',
-                                        lambda: (f()[0], safe_value)):
+        with mock.patch.object(self.uuid, '_generate_time_safe',
+                               lambda: (f()[0], safe_value)):
             yield
 
     @unittest.skipUnless(os.name == 'posix', 'POSIX-only test')
@@ -674,27 +673,57 @@ class TestUUIDWithExtModule(BaseTestUUID, unittest.TestCase):
 class BaseTestInternals:
     _uuid = py_uuid
 
-    @unittest.skipUnless(os.name == 'posix', 'requires Posix')
-    def test_find_mac(self):
+
+    def test_find_under_heading(self):
+        data = '''\
+Name  Mtu   Network     Address           Ipkts Ierrs    Opkts Oerrs  Coll
+en0   1500  link#2      fe.ad.c.1.23.4   1714807956     0 711348489     0     0
+                        01:00:5e:00:00:01
+en0   1500  192.168.129 x071             1714807956     0 711348489     0     0
+                        224.0.0.1
+en0   1500  192.168.90  x071             1714807956     0 711348489     0     0
+                        224.0.0.1
+'''
+
+        def mock_get_command_stdout(command, args):
+            return io.BytesIO(data.encode())
+
+        # The above data is from AIX - with '.' as _MAC_DELIM and strings
+        # shorter than 17 bytes (no leading 0). (_MAC_OMITS_LEADING_ZEROES=True)
+        with mock.patch.multiple(self.uuid,
+                                 _MAC_DELIM=b'.',
+                                 _MAC_OMITS_LEADING_ZEROES=True,
+                                 _get_command_stdout=mock_get_command_stdout):
+            mac = self.uuid._find_mac_under_heading(
+                command='netstat',
+                args='-ian',
+                heading=b'Address',
+            )
+
+        self.assertEqual(mac, 0xfead0c012304)
+
+    def test_find_mac_near_keyword(self):
+        # key and value are on the same line
         data = '''
-fake hwaddr
+fake      Link encap:UNSPEC  hwaddr 00-00
 cscotun0  Link encap:UNSPEC  HWaddr 00-00-00-00-00-00-00-00-00-00-00-00-00-00-00-00
 eth0      Link encap:Ethernet  HWaddr 12:34:56:78:90:ab
 '''
 
-        popen = unittest.mock.MagicMock()
-        popen.stdout = io.BytesIO(data.encode())
-
-        with unittest.mock.patch.object(shutil, 'which',
-                                        return_value='/sbin/ifconfig'):
-            with unittest.mock.patch.object(subprocess, 'Popen',
-                                            return_value=popen):
-                mac = self.uuid._find_mac(
-                    command='ifconfig',
-                    args='',
-                    hw_identifiers=[b'hwaddr'],
-                    get_index=lambda x: x + 1,
-                )
+        def mock_get_command_stdout(command, args):
+            return io.BytesIO(data.encode())
+
+        # The above data will only be parsed properly on non-AIX unixes.
+        with mock.patch.multiple(self.uuid,
+                                 _MAC_DELIM=b':',
+                                 _MAC_OMITS_LEADING_ZEROES=False,
+                                 _get_command_stdout=mock_get_command_stdout):
+            mac = self.uuid._find_mac_near_keyword(
+                command='ifconfig',
+                args='',
+                keywords=[b'hwaddr'],
+                get_word_index=lambda x: x + 1,
+            )
 
         self.assertEqual(mac, 0x1234567890ab)
 
diff --git a/Lib/uuid.py b/Lib/uuid.py
index 5f3bc9e..6a436d3 100644
--- a/Lib/uuid.py
+++ b/Lib/uuid.py
@@ -59,6 +59,12 @@ _DARWIN  = platform.system() == 'Darwin'
 _LINUX   = platform.system() == 'Linux'
 _WINDOWS = platform.system() == 'Windows'
 
+_MAC_DELIM = b':'
+_MAC_OMITS_LEADING_ZEROES = False
+if _AIX:
+    _MAC_DELIM = b'.'
+    _MAC_OMITS_LEADING_ZEROES = True
+
 RESERVED_NCS, RFC_4122, RESERVED_MICROSOFT, RESERVED_FUTURE = [
     'reserved for NCS compatibility', 'specified in RFC 4122',
     'reserved for Microsoft compatibility', 'reserved for future definition']
@@ -347,24 +353,32 @@ class UUID:
         if self.variant == RFC_4122:
             return int((self.int >> 76) & 0xf)
 
-def _popen(command, *args):
-    import os, shutil, subprocess
-    executable = shutil.which(command)
-    if executable is None:
-        path = os.pathsep.join(('/sbin', '/usr/sbin'))
-        executable = shutil.which(command, path=path)
+
+def _get_command_stdout(command, *args):
+    import io, os, shutil, subprocess
+
+    try:
+        path_dirs = os.environ.get('PATH', os.defpath).split(os.pathsep)
+        path_dirs.extend(['/sbin', '/usr/sbin'])
+        executable = shutil.which(command, path=os.pathsep.join(path_dirs))
         if executable is None:
             return None
-    # LC_ALL=C to ensure English output, stderr=DEVNULL to prevent output
-    # on stderr (Note: we don't have an example where the words we search
-    # for are actually localized, but in theory some system could do so.)
-    env = dict(os.environ)
-    env['LC_ALL'] = 'C'
-    proc = subprocess.Popen((executable,) + args,
-                            stdout=subprocess.PIPE,
-                            stderr=subprocess.DEVNULL,
-                            env=env)
-    return proc
+        # LC_ALL=C to ensure English output, stderr=DEVNULL to prevent output
+        # on stderr (Note: we don't have an example where the words we search
+        # for are actually localized, but in theory some system could do so.)
+        env = dict(os.environ)
+        env['LC_ALL'] = 'C'
+        proc = subprocess.Popen((executable,) + args,
+                                stdout=subprocess.PIPE,
+                                stderr=subprocess.DEVNULL,
+                                env=env)
+        if not proc:
+            return None
+        stdout, stderr = proc.communicate()
+        return io.BytesIO(stdout)
+    except (OSError, subprocess.SubprocessError):
+        return None
+
 
 # For MAC (a.k.a. IEEE 802, or EUI-48) addresses, the second least significant
 # bit of the first octet signifies whether the MAC address is universally (0)
@@ -384,40 +398,101 @@ def _popen(command, *args):
 def _is_universal(mac):
     return not (mac & (1 << 41))
 
-def _find_mac(command, args, hw_identifiers, get_index):
+
+def _find_mac_near_keyword(command, args, keywords, get_word_index):
+    """Searches a command's output for a MAC address near a keyword.
+
+    Each line of words in the output is case-insensitively searched for
+    any of the given keywords.  Upon a match, get_word_index is invoked
+    to pick a word from the line, given the index of the match.  For
+    example, lambda i: 0 would get the first word on the line, while
+    lambda i: i - 1 would get the word preceding the keyword.
+    """
+    stdout = _get_command_stdout(command, args)
+    if stdout is None:
+        return None
+
     first_local_mac = None
+    for line in stdout:
+        words = line.lower().rstrip().split()
+        for i in range(len(words)):
+            if words[i] in keywords:
+                try:
+                    word = words[get_word_index(i)]
+                    mac = int(word.replace(_MAC_DELIM, b''), 16)
+                except (ValueError, IndexError):
+                    # Virtual interfaces, such as those provided by
+                    # VPNs, do not have a colon-delimited MAC address
+                    # as expected, but a 16-byte HWAddr separated by
+                    # dashes. These should be ignored in favor of a
+                    # real MAC address
+                    pass
+                else:
+                    if _is_universal(mac):
+                        return mac
+                    first_local_mac = first_local_mac or mac
+    return first_local_mac or None
+
+
+def _find_mac_under_heading(command, args, heading):
+    """Looks for a MAC address under a heading in a command's output.
+
+    The first line of words in the output is searched for the given
+    heading. Words at the same word index as the heading in subsequent
+    lines are then examined to see if they look like MAC addresses.
+    """
+    stdout = _get_command_stdout(command, args)
+    if stdout is None:
+        return None
+
+    keywords = stdout.readline().rstrip().split()
     try:
-        proc = _popen(command, *args.split())
-        if not proc:
-            return None
-        with proc:
-            for line in proc.stdout:
-                words = line.lower().rstrip().split()
-                for i in range(len(words)):
-                    if words[i] in hw_identifiers:
-                        try:
-                            word = words[get_index(i)]
-                            mac = int(word.replace(b':', b''), 16)
-                            if _is_universal(mac):
-                                return mac
-                            first_local_mac = first_local_mac or mac
-                        except (ValueError, IndexError):
-                            # Virtual interfaces, such as those provided by
-                            # VPNs, do not have a colon-delimited MAC address
-                            # as expected, but a 16-byte HWAddr separated by
-                            # dashes. These should be ignored in favor of a
-                            # real MAC address
-                            pass
-    except OSError:
-        pass
+        column_index = keywords.index(heading)
+    except ValueError:
+        return None
+
+    first_local_mac = None
+    for line in stdout:
+        try:
+            words = line.rstrip().split()
+            word = words[column_index]
+            if len(word) == 17:
+                mac = int(word.replace(_MAC_DELIM, b''), 16)
+            elif _MAC_OMITS_LEADING_ZEROES:
+                # (Only) on AIX the macaddr value given is not prefixed by 0, e.g.
+                # en0   1500  link#2      fa.bc.de.f7.62.4 110854824     0 160133733     0     0
+                # not
+                # en0   1500  link#2      fa.bc.de.f7.62.04 110854824     0 160133733     0     0
+                parts = word.split(_MAC_DELIM)
+                if len(parts) == 6 and all(0 < len(p) <= 2 for p in parts):
+                    hexstr = b''.join(p.rjust(2, b'0') for p in parts)
+                    mac = int(hexstr, 16)
+                else:
+                    continue
+            else:
+                continue
+        except (ValueError, IndexError):
+            # Virtual interfaces, such as those provided by
+            # VPNs, do not have a colon-delimited MAC address
+            # as expected, but a 16-byte HWAddr separated by
+            # dashes. These should be ignored in favor of a
+            # real MAC address
+            pass
+        else:
+            if _is_universal(mac):
+                return mac
+            first_local_mac = first_local_mac or mac
     return first_local_mac or None
 
+
+# The following functions call external programs to 'get' a macaddr value to
+# be used as basis for an uuid
 def _ifconfig_getnode():
     """Get the hardware address on Unix by running ifconfig."""
     # This works on Linux ('' or '-a'), Tru64 ('-av'), but not all Unixes.
     keywords = (b'hwaddr', b'ether', b'address:', b'lladdr')
     for args in ('', '-a', '-av'):
-        mac = _find_mac('ifconfig', args, keywords, lambda i: i+1)
+        mac = _find_mac_near_keyword('ifconfig', args, keywords, lambda i: i+1)
         if mac:
             return mac
         return None
@@ -425,7 +500,7 @@ def _ifconfig_getnode():
 def _ip_getnode():
     """Get the hardware address on Unix by running ip."""
     # This works on Linux with iproute2.
-    mac = _find_mac('ip', 'link', [b'link/ether'], lambda i: i+1)
+    mac = _find_mac_near_keyword('ip', 'link', [b'link/ether'], lambda i: i+1)
     if mac:
         return mac
     return None
@@ -439,17 +514,17 @@ def _arp_getnode():
         return None
 
     # Try getting the MAC addr from arp based on our IP address (Solaris).
-    mac = _find_mac('arp', '-an', [os.fsencode(ip_addr)], lambda i: -1)
+    mac = _find_mac_near_keyword('arp', '-an', [os.fsencode(ip_addr)], lambda i: -1)
     if mac:
         return mac
 
     # This works on OpenBSD
-    mac = _find_mac('arp', '-an', [os.fsencode(ip_addr)], lambda i: i+1)
+    mac = _find_mac_near_keyword('arp', '-an', [os.fsencode(ip_addr)], lambda i: i+1)
     if mac:
         return mac
 
     # This works on Linux, FreeBSD and NetBSD
-    mac = _find_mac('arp', '-an', [os.fsencode('(%s)' % ip_addr)],
+    mac = _find_mac_near_keyword('arp', '-an', [os.fsencode('(%s)' % ip_addr)],
                     lambda i: i+2)
     # Return None instead of 0.
     if mac:
@@ -459,36 +534,12 @@ def _arp_getnode():
 def _lanscan_getnode():
     """Get the hardware address on Unix by running lanscan."""
     # This might work on HP-UX.
-    return _find_mac('lanscan', '-ai', [b'lan0'], lambda i: 0)
+    return _find_mac_near_keyword('lanscan', '-ai', [b'lan0'], lambda i: 0)
 
 def _netstat_getnode():
     """Get the hardware address on Unix by running netstat."""
-    # This might work on AIX, Tru64 UNIX.
-    first_local_mac = None
-    try:
-        proc = _popen('netstat', '-ia')
-        if not proc:
-            return None
-        with proc:
-            words = proc.stdout.readline().rstrip().split()
-            try:
-                i = words.index(b'Address')
-            except ValueError:
-                return None
-            for line in proc.stdout:
-                try:
-                    words = line.rstrip().split()
-                    word = words[i]
-                    if len(word) == 17 and word.count(b':') == 5:
-                        mac = int(word.replace(b':', b''), 16)
-                        if _is_universal(mac):
-                            return mac
-                        first_local_mac = first_local_mac or mac
-                except (ValueError, IndexError):
-                    pass
-    except OSError:
-        pass
-    return first_local_mac or None
+    # This works on AIX and might work on Tru64 UNIX.
+    return _find_mac_under_heading('netstat', '-ian', b'Address')
 
 def _ipconfig_getnode():
     """Get the hardware address on Windows by running ipconfig.exe."""
diff --git a/Misc/NEWS.d/next/Library/2018-08-04-12-26-11.bpo-28009.4JcHZb.rst b/Misc/NEWS.d/next/Library/2018-08-04-12-26-11.bpo-28009.4JcHZb.rst
new file mode 100644
index 0000000..40f6382
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2018-08-04-12-26-11.bpo-28009.4JcHZb.rst
@@ -0,0 +1,4 @@
+Fix uuid.getnode() on platforms with '.' as MAC Addr delimiter as well
+fix for MAC Addr format that omits a leading 0 in MAC Addr values.
+Currently, AIX is the only know platform with these settings.
+Patch by Michael Felt.