Skip to content
Merged
Show file tree
Hide file tree
Changes from 38 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
4ed41e9
uuid: correct the field seperator value of ethernet adapter MAC addre…
aixtools Aug 4, 2018
ba47933
Merge branch 'master' into bpo-28009-1
aixtools Aug 4, 2018
c6029a4
correct test logic, cut/paste error
aixtools Aug 4, 2018
7e1874d
specify shorter getters list per https://bugs.python.org/issue28009#m…
aixtools Aug 5, 2018
6a5bccb
write _notAIX as non-negative constant and use 'not AIX'
aixtools Aug 22, 2018
a98309b
simplify NEWS entry
aixtools Aug 22, 2018
53ef750
skip a test the right way
aixtools Aug 22, 2018
6fd5f8e
capitalize constants
aixtools Aug 22, 2018
3bb1c08
oops - restore missing assignment
aixtools Aug 22, 2018
1af8e3d
skip both tests the right way
aixtools Aug 22, 2018
d3eaab9
write _notAIX as non-negative constant and use 'not _AIX'
aixtools Aug 22, 2018
3469a01
added additional 'skip' entries for commands that do not exist on AIX
aixtools Aug 22, 2018
399e8ec
Use and set a single _NODE_GETTERS constant with platform test outsid…
aixtools Aug 24, 2018
6e2a9bf
modify the lambda function so it does not look like a bug
aixtools Sep 16, 2018
bb3a460
resolve differences in imports
aixtools Sep 16, 2018
db6767f
Switch back to using sys.platform.startswith() to resolve conflicts
aixtools Sep 16, 2018
25d3ef1
still trying to get differencs to resolve...
aixtools Sep 16, 2018
fa9b43b
changes per request. Thx for the review!
aixtools Oct 28, 2018
4a0c8f0
resolve merge conflict
aixtools Oct 28, 2018
6463707
resolve merge conflict, 2nd try
aixtools Oct 28, 2018
a0ef760
remove unused added import
taleinat Nov 12, 2018
095e221
Per requested changes
aixtools Nov 14, 2018
ec4c0e8
Merge branch 'bpo-28009-1' of github.com:aixtools/cpython into bpo-28…
aixtools Nov 14, 2018
70a45f0
Merge branch 'master' into bpo-28009-1
ncoghlan Dec 26, 2018
b1b4952
sync with master
aixtools Jun 14, 2019
8f0687a
rename find_mac_routines (in test_uuid.py)
aixtools Jun 14, 2019
4756670
fix sync issues
aixtools Jun 14, 2019
c55714a
sync with master
aixtools Jun 16, 2019
10f272e
add inline comments, correct typos
aixtools Jun 17, 2019
27b6c32
Improve blurb
aixtools Jun 21, 2019
33969b9
refactor more of getting a command's stdout into the helper func
taleinat Jun 24, 2019
7a57734
reduce scope of try/except blocks, for clarity
taleinat Jun 24, 2019
4628cea
make tests run regardless of host OS
taleinat Jun 24, 2019
d2830a2
remove no longer necessary _AIX definition in the test file
taleinat Jul 14, 2019
0688727
fix unittest import and indentation in one place
taleinat Jul 14, 2019
ff1ee20
simplify macaddr parsing and control flow in _find_mac_nextlines()
taleinat Jul 14, 2019
083e9c6
no need for extra return value check in _netstart_getnode()
taleinat Jul 14, 2019
30cd017
improve wording of NEWS entry
taleinat Jul 14, 2019
27a972b
make AIX formatted MAC address handling AIX-specific
taleinat Jul 15, 2019
543e66d
Stop reverse DNS lookups with netstat
aixtools Jul 15, 2019
588fda7
add _MAC_OMITS_LEADING_ZEROES, better function names and doc-strings
taleinat Jul 17, 2019
28f7a01
update blurb and comments in test_uuid.py
aixtools Aug 30, 2019
6fc2129
Update News blurb
aixtools Sep 3, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
74 changes: 53 additions & 21 deletions Lib/test/test_uuid.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import unittest.mock
import unittest
from test import support
import builtins
import contextlib
Expand All @@ -15,7 +15,6 @@
py_uuid = support.import_fresh_module('uuid', blocked=['_uuid'])
c_uuid = support.import_fresh_module('uuid', fresh=['_uuid'])


def importable(name):
try:
__import__(name)
Expand Down Expand Up @@ -459,7 +458,7 @@ def test_uuid1_eui64(self):
# 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],
Expand Down Expand Up @@ -538,8 +537,8 @@ def mock_generate_time_safe(self, safe_value):
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')
Expand Down Expand Up @@ -674,27 +673,60 @@ 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_mac_nextlines(self):
# key is on line X, value is on line X+1 aka 'nextline'
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 specific to AIX - with '.' as _MAC_DELIM
# and strings shorter than 17 bytes (no leading 0).
# The above data will only be parsed properly on non-AIX unixes.
Comment thread
taleinat marked this conversation as resolved.
Outdated
with mock.patch.multiple(self.uuid,
_AIX=True,
_MAC_DELIM=b'.',
_get_command_stdout=mock_get_command_stdout):
mac = self.uuid._find_mac_nextlines(
command='netstat',
args='-ia',
hw_identifiers=b'Address',
f_index=lambda x: x,
)

self.assertEqual(mac, 0xfead0c012304)
Comment thread
aixtools marked this conversation as resolved.

def test_find_mac_inline(self):
# key and value are on the same line aka 'inline'
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,
_AIX=False,
_MAC_DELIM=b':',
_get_command_stdout=mock_get_command_stdout):
mac = self.uuid._find_mac_inline(
command='ifconfig',
args='',
hw_identifiers=[b'hwaddr'],
f_index=lambda x: x + 1,
)

self.assertEqual(mac, 0x1234567890ab)

Expand Down
178 changes: 106 additions & 72 deletions Lib/uuid.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
_DARWIN = platform.system() == 'Darwin'
_LINUX = platform.system() == 'Linux'
_WINDOWS = platform.system() == 'Windows'
_MAC_DELIM = b':' if not _AIX else b'.'
Comment thread
taleinat marked this conversation as resolved.
Outdated

RESERVED_NCS, RFC_4122, RESERVED_MICROSOFT, RESERVED_FUTURE = [
'reserved for NCS compatibility', 'specified in RFC 4122',
Expand Down Expand Up @@ -347,24 +348,33 @@ def version(self):
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:
executable = shutil.which(command)
if executable is None:
path = os.pathsep.join(('/sbin', '/usr/sbin'))
Comment thread
taleinat marked this conversation as resolved.
Outdated
executable = shutil.which(command, path=path)
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)
if not proc:
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
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)
Expand All @@ -384,48 +394,96 @@ def _popen(command, *args):
def _is_universal(mac):
return not (mac & (1 << 41))

def _find_mac(command, args, hw_identifiers, get_index):

# In the next two fucnctions:
Copy link
Copy Markdown
Contributor

@zestyping zestyping Jul 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I understand the general approach here, and it makes sense. These two functions need a better explanation of what they do, though—the comments here aren't enough for a reader to understand how they're intended to work, and the argument names hw_identifiers and f_index are quite cryptic.

Here's an attempt at some better names and a docstring:

def _find_mac_near_keyword(command, args, keywords, get_word_index):
    """Searches the output of a command for a MAC address near a keyword.

    Each line of words in the output is lowercased and 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 (e.g. lambda i: 0 gets the
    first word on the line; lambda i: i - 1 gets the word preceding the keyword)."""

Given that _find_mac_nextlines is supposed to look for words under column headings, there's no need for it to take an f_index function, and the third argument can be named more clearly. Indeed, it would be misleading to call the third argument hw_identifiers, as it's a single string, not a list of strings. Here's a try:

def _find_mac_under_heading(command, args, heading):
    """Looks for a MAC address under a heading in the output of a command.

    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."""

Another concern I have is that the logic about what looks like a MAC address is mixed in with the logic about where to find it. It would be ideal to factor it out, like this:

def _find_near_keyword(command, args, keywords, get_word_index):
    """Yields selected nearby words for keywords found in the output of a command.
    ...
        yield _parse_mac(word)
    ...

def _find_mac_under_heading(command, args, heading):
    """Yields words found under a heading in the output of a command.

    The first line of words in the output is searched for the given heading.
    For each subsequent line, the word at the same word index as the heading
    in subsequent lines is then yielded."""
    
def _parse_mac(word):
    """Converts a string to a 48-bit integer if it looks like a MAC address, or returns None."""
    # A MAC address is six bytes, each printed as one or two hex digits.
    # They may be separated by ':' or '.' depending on the platform.
    parts = word.replace('.', ':').split(':')
    if len(parts) == 6 and all(len(part) <= 2 for part in parts):
        hexstr = b''.join(part.rjust(2, b'0') for part in parts)
        try:
            return int(hexstr, 16)
        except ValueError:
            pass

def _select_best_mac(macs):
    """Chooses a MAC from the given iterable, preferring any universally administered
    MAC address over locally administered MAC addresses."""
    local_macs = []
    for mac in macs:
        if _is_universal(mac):
            return mac
        elif mac:
            local_macs.append(mac)
    if local_macs:
        return local_macs[0]

Then:

_select_best_mac(_find_near_keyword('ifconfig', args, keywords, lambda i: i + 1))
_select_best_mac(_find_under_heading('netstat', '-ia', b'Address')

etc.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.
Very interesting to read up better ways to do things, e.g., "parts = word.replace('.', ':').split(':')"

# command: name of command to run
# args: arguments passed to command
# hw_identifers: keywords used to locate a value
# f_index: lambda function to modify, if needed, an index value
# keyword and value are on the same line aka 'inline'
def _find_mac_inline(command, args, hw_identifiers, f_index):
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 hw_identifiers:
try:
word = words[f_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


# Keyword is only on firstline - values on remaining lines
def _find_mac_nextlines(command, args, hw_identifiers, f_index):
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
i = keywords.index(hw_identifiers)
except ValueError:
return None
# we have the index (i) into the data that follows

first_local_mac = None
for line in stdout:
try:
words = line.rstrip().split()
word = words[f_index(i)]
# (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)
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_inline('ifconfig', args, keywords, lambda i: i+1)
if mac:
return mac
return None

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_inline('ip', 'link', [b'link/ether'], lambda i: i+1)
if mac:
return mac
return None
Expand All @@ -439,17 +497,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_inline('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_inline('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_inline('arp', '-an', [os.fsencode('(%s)' % ip_addr)],
lambda i: i+2)
# Return None instead of 0.
if mac:
Expand All @@ -459,36 +517,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_inline('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_nextlines('netstat', '-ia', b'Address', lambda i: i)

def _ipconfig_getnode():
"""Get the hardware address on Windows by running ipconfig.exe."""
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fix uuid.getnode() on AIX to properly parse MAC addresses in netstat's output.
Patch by Michael Felt.