Issue #22637: avoid using a shell in uuid

vstinner · vstinner · commit b9d0199c073f · 2014-10-21T22:33:10.000+02:00
Replace os.popen() with subprocess.Popen() in the uuid module.
diff --git a/Lib/test/test_uuid.py b/Lib/test/test_uuid.py
@@ -1,9 +1,10 @@
-import unittest
+import unittest.mock
 from test import support
 import builtins
 import io
 import os
 import shutil
+import subprocess
 import uuid
 
 def importable(name):
@@ -361,28 +362,27 @@ def test_getnode(self):
 
     @unittest.skipUnless(os.name == 'posix', 'requires Posix')
     def test_find_mac(self):
-        data = '''\
-
+        data = '''
 fake hwaddr
 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
 '''
-        def mock_popen(cmd):
-            return io.StringIO(data)
-
-        if shutil.which('ifconfig') is None:
-            path = os.pathsep.join(('/sbin', '/usr/sbin'))
-            if shutil.which('ifconfig', path=path) is None:
-                self.skipTest('requires ifconfig')
-
-        with support.swap_attr(os, 'popen', mock_popen):
-            mac = uuid._find_mac(
-                command='ifconfig',
-                args='',
-                hw_identifiers=['hwaddr'],
-                get_index=lambda x: x + 1,
-            )
-            self.assertEqual(mac, 0x1234567890ab)
+
+        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 = uuid._find_mac(
+                    command='ifconfig',
+                    arg='',
+                    hw_identifiers=[b'hwaddr'],
+                    get_index=lambda x: x + 1,
+                )
+
+        self.assertEqual(mac, 0x1234567890ab)
 
     @unittest.skipUnless(importable('ctypes'), 'requires ctypes')
     def test_uuid1(self):
diff --git a/Lib/uuid.py b/Lib/uuid.py
@@ -304,8 +304,8 @@ def version(self):
         if self.variant == RFC_4122:
             return int((self.int >> 76) & 0xf)
 
-def _find_mac(command, args, hw_identifiers, get_index):
-    import os, shutil
+def _find_mac(command, arg, hw_identifiers, get_index):
+    import os, shutil, subprocess
     executable = shutil.which(command)
     if executable is None:
         path = os.pathsep.join(('/sbin', '/usr/sbin'))
@@ -314,18 +314,26 @@ def _find_mac(command, args, hw_identifiers, get_index):
             return None
 
     try:
-        # LC_ALL to ensure English output, 2>/dev/null 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.)
-        cmd = 'LC_ALL=C %s %s 2>/dev/null' % (executable, args)
-        with os.popen(cmd) as pipe:
-            for line in pipe:
+        # 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'
+        cmd = [executable]
+        if arg:
+            cmd.append(arg)
+        proc = subprocess.Popen(cmd,
+                                stdout=subprocess.PIPE,
+                                stderr=subprocess.DEVNULL,
+                                env=env)
+        with proc:
+            for line in proc.stdout:
                 words = line.lower().split()
                 for i in range(len(words)):
                     if words[i] in hw_identifiers:
                         try:
                             return int(
-                                words[get_index(i)].replace(':', ''), 16)
+                                words[get_index(i)].replace(b':', b''), 16)
                         except (ValueError, IndexError):
                             # Virtual interfaces, such as those provided by
                             # VPNs, do not have a colon-delimited MAC address
@@ -341,20 +349,20 @@ def _ifconfig_getnode():
 
     # This works on Linux ('' or '-a'), Tru64 ('-av'), but not all Unixes.
     for args in ('', '-a', '-av'):
-        mac = _find_mac('ifconfig', args, ['hwaddr', 'ether'], lambda i: i+1)
+        mac = _find_mac('ifconfig', args, [b'hwaddr', b'ether'], lambda i: i+1)
         if mac:
             return mac
 
     import socket
     ip_addr = socket.gethostbyname(socket.gethostname())
 
     # Try getting the MAC addr from arp based on our IP address (Solaris).
-    mac = _find_mac('arp', '-an', [ip_addr], lambda i: -1)
+    mac = _find_mac('arp', '-an', [os.fsencode(ip_addr)], lambda i: -1)
     if mac:
         return mac
 
     # This might work on HP-UX.
-    mac = _find_mac('lanscan', '-ai', ['lan0'], lambda i: 0)
+    mac = _find_mac('lanscan', '-ai', [b'lan0'], lambda i: 0)
     if mac:
         return mac
 
