Fix UnicodeDecodeError when parsing hosts file with non-ascii

salt/modules/mac_sysctl.py

@@ -179,13 +179,10 @@ def persist(name, value, config='/etc/sysctl.conf', apply_change=False):
                     rest = rest[len(rest_v):]
                 if rest_v == value:
                     return 'Already set'
-                new_line = '{0}={1}'.format(name, value)
-                nlines.append(new_line)
-                nlines.append('\n')
+                nlines.append('{0}={1}\n'.format(name, value))
                 edited = True
     if not edited:
-        nlines.append('{0}={1}'.format(name, value))
-        nlines.append('\n')
+        nlines.append('{0}={1}\n'.format(name, value))
     nlines = [salt.utils.stringutils.to_str(_l) for _l in nlines]
     with salt.utils.files.fopen(config, 'w+') as ofile:
         ofile.writelines(nlines)

salt/utils/network.py

@@ -148,14 +148,18 @@ def first(self):
                 addr=hosts.first() or 'localhost (N/A)', message=socket.gaierror)
             )
     # Universal method for everywhere (Linux, Slowlaris, Windows etc)
-    for f_name in ['/etc/hostname', '/etc/nodename', '/etc/hosts',
-                   r'{win}\system32\drivers\etc\hosts'.format(win=os.getenv('WINDIR'))]:
-        if not os.path.exists(f_name):
-            continue
-        with salt.utils.files.fopen(f_name) as f_hdl:
-            for hst in (line.strip().split('#')[0].strip().split() or None for line in f_hdl.read().split(os.linesep)):
-                if hst and (hst[0][:4] in ['127.', '::1'] or len(hst) == 1):
-                    hosts.extend(hst)
+    for f_name in ('/etc/hostname', '/etc/nodename', '/etc/hosts',
+                   r'{win}\system32\drivers\etc\hosts'.format(win=os.getenv('WINDIR'))):
+        try:
+            with salt.utils.files.fopen(f_name) as f_hdl:
+                for line in f_hdl:
+                    line = salt.utils.stringutils.to_unicode(line)
+                    hst = line.strip().split('#')[0].strip().split()
+                    if hst:
+                        if hst[0][:4] in ('127.', '::1') or len(hst) == 1:
+                            hosts.extend(hst)
+        except IOError:
+            pass
 
     # include public and private ipaddresses
     return hosts.extend([addr for addr in ip_addrs()

tests/support/mock.py

@@ -15,10 +15,13 @@
 # pylint: disable=unused-import,function-redefined,blacklisted-module,blacklisted-external-module
 
 from __future__ import absolute_import
+import errno
+import fnmatch
 import sys
 
 # Import salt libs
 from salt.ext import six
+import salt.utils.stringutils
 
 try:
     from mock import (
@@ -98,30 +101,33 @@ def __call__(self, *args, **kwargs):
 
 
 def _iterate_read_data(read_data):
-    # Helper for mock_open:
-    # Retrieve lines from read_data via a generator so that separate calls to
-    # readline, read, and readlines are properly interleaved
-    if six.PY3 and isinstance(read_data, six.binary_type):
-        data_as_list = ['{0}\n'.format(l.decode(__salt_system_encoding__)) for l in read_data.split(b'\n')]
-    else:
-        data_as_list = ['{0}\n'.format(l) for l in read_data.split('\n')]
-
-    if data_as_list[-1] == '\n':
+    '''
+    Helper for mock_open:
+    Retrieve lines from read_data via a generator so that separate calls to
+    readline, read, and readlines are properly interleaved
+    '''
+    # Newline will always be a bytestring on PY2 because mock_open will have
+    # normalized it to one.
+    newline = b'\n' if isinstance(read_data, six.binary_type) else '\n'
+
+    read_data = [line + newline for line in read_data.split(newline)]
+
+    if read_data[-1] == newline:
         # If the last line ended in a newline, the list comprehension will have an
-        # extra entry that's just a newline.  Remove this.
-        data_as_list = data_as_list[:-1]
+        # extra entry that's just a newline. Remove this.
+        read_data = read_data[:-1]
     else:
         # If there wasn't an extra newline by itself, then the file being
-        # emulated doesn't have a newline to end the last line  remove the
-        # newline that our naive format() added
-        data_as_list[-1] = data_as_list[-1][:-1]
+        # emulated doesn't have a newline to end the last line, so remove the
+        # newline that we added in the list comprehension.
+        read_data[-1] = read_data[-1][:-1]
 
-    for line in data_as_list:
+    for line in read_data:
         yield line
 
 
-def mock_open(mock=None, read_data=''):
-    """
+def mock_open(mock=None, read_data='', match=None):
+    '''
     A helper function to create a mock to replace the use of `open`. It works
     for `open` called directly or used as a context manager.
 
@@ -131,7 +137,18 @@ def mock_open(mock=None, read_data=''):
 
     `read_data` is a string for the `read` methoddline`, and `readlines` of the
     file handle to return.  This is an empty string by default.
-    """
+
+    If passed, `match` can be either a string or an iterable containing
+    patterns to attempt to match using fnmatch.fnmatch(). A side_effect will be
+    added to the mock object returned, which will cause an IOError(2, 'No such
+    file or directory') to be raised when the file path is not a match. This
+    allows you to make your mocked filehandle only work for certain file paths.
+    '''
+    # Normalize read_data, Python 2 filehandles should never produce unicode
+    # types on read.
+    if six.PY2:
+        read_data = salt.utils.stringutils.to_str(read_data)
+
     def _readlines_side_effect(*args, **kwargs):
         if handle.readlines.return_value is not None:
             return handle.readlines.return_value
@@ -140,7 +157,8 @@ def _readlines_side_effect(*args, **kwargs):
     def _read_side_effect(*args, **kwargs):
         if handle.read.return_value is not None:
             return handle.read.return_value
-        return ''.join(_data)
+        joiner = b'' if isinstance(read_data, six.binary_type) else ''
+        return joiner.join(_data)
 
     def _readline_side_effect():
         if handle.readline.return_value is not None:
@@ -170,10 +188,25 @@ def _readline_side_effect():
     handle.readline.return_value = None
     handle.readlines.return_value = None
 
+    # Support iteration via for loop
+    handle.__iter__ = lambda x: _readline_side_effect()
+
     # This is salt specific and not in the upstream mock
     handle.read.side_effect = _read_side_effect
     handle.readline.side_effect = _readline_side_effect()
     handle.readlines.side_effect = _readlines_side_effect
 
+    if match is not None:
+        if isinstance(match, six.string_types):
+            match = [match]
+
+        def fopen_side_effect(name, *args, **kwargs):
+            for pat in match:
+                if fnmatch.fnmatch(name, pat):
+                    return DEFAULT
+            raise IOError(errno.ENOENT, 'No such file or directory', name)
+
+        mock.side_effect = fopen_side_effect
+
     mock.return_value = handle
     return mock

tests/unit/grains/test_core.py

@@ -7,6 +7,7 @@
 from __future__ import absolute_import, print_function, unicode_literals
 import logging
 import os
+import textwrap
 
 # Import Salt Testing Libs
 try:
@@ -66,9 +67,8 @@ def setup_loader_modules(self):
     def test_parse_etc_os_release(self, path_isfile_mock):
         path_isfile_mock.side_effect = lambda x: x == "/usr/lib/os-release"
         with salt.utils.files.fopen(os.path.join(OS_RELEASE_DIR, "ubuntu-17.10")) as os_release_file:
-            os_release_content = os_release_file.readlines()
-        with patch("salt.utils.files.fopen", mock_open()) as os_release_file:
-            os_release_file.return_value.__iter__.return_value = os_release_content
+            os_release_content = os_release_file.read()
+        with patch("salt.utils.files.fopen", mock_open(read_data=os_release_content)):
             os_release = core._parse_os_release(["/etc/os-release", "/usr/lib/os-release"])
         self.assertEqual(os_release, {
             "NAME": "Ubuntu",
@@ -269,34 +269,26 @@ def _import_mock(name, *args):
             return orig_import(name, *args)
 
         # Skip the first if statement
-        with patch.object(salt.utils.platform, 'is_proxy',
-                          MagicMock(return_value=False)):
-            # Skip the selinux/systemd stuff (not pertinent)
-            with patch.object(core, '_linux_bin_exists',
-                              MagicMock(return_value=False)):
-                # Skip the init grain compilation (not pertinent)
-                with patch.object(os.path, 'exists', path_isfile_mock):
-                    # Ensure that lsb_release fails to import
-                    with patch('{0}.__import__'.format(built_in),
-                               side_effect=_import_mock):
-                        # Skip all the /etc/*-release stuff (not pertinent)
-                        with patch.object(os.path, 'isfile', path_isfile_mock):
-                            with patch.object(core, '_parse_os_release', os_release_mock):
-                                # Mock linux_distribution to give us the OS
-                                # name that we want.
-                                distro_mock = MagicMock(
-                                    return_value=os_release_map['linux_distribution']
-                                )
-                                with patch('salt.utils.files.fopen', mock_open()) as suse_release_file:
-                                    suse_release_file.return_value.__iter__.return_value = \
-                                        os_release_map.get('suse_release_file', '').splitlines()
-                                    with patch.object(core, 'linux_distribution', distro_mock):
-                                        with patch.object(core, '_linux_gpu_data', empty_mock):
-                                            with patch.object(core, '_linux_cpudata', empty_mock):
-                                                with patch.object(core, '_virtual', empty_mock):
-                                                    # Mock the osarch
-                                                    with patch.dict(core.__salt__, {'cmd.run': osarch_mock}):
-                                                        os_grains = core.os_data()
+        # Skip the selinux/systemd stuff (not pertinent)
+        # Skip the init grain compilation (not pertinent)
+        # Ensure that lsb_release fails to import
+        # Skip all the /etc/*-release stuff (not pertinent)
+        #  - Mock linux_distribution to give us the OS name that we want.
+        # Mock the osarch
+	distro_mock = MagicMock(return_value=os_release_map['linux_distribution'])
+        with patch.object(salt.utils.platform, 'is_proxy', MagicMock(return_value=False)), \
+		patch.object(core, '_linux_bin_exists', MagicMock(return_value=False)), \
+                patch.object(os.path, 'exists', path_isfile_mock), \
+                patch('{0}.__import__'.format(built_in), side_effect=_import_mock), \
+                patch.object(os.path, 'isfile', path_isfile_mock), \
+                patch.object(core, '_parse_os_release', os_release_mock), \
+                patch('salt.utils.files.fopen', mock_open(read_data=os_release_map.get('suse_release_file', ''))), \
+                patch.object(core, 'linux_distribution', distro_mock), \
+                patch.object(core, '_linux_gpu_data', empty_mock), \
+                patch.object(core, '_linux_cpudata', empty_mock), \
+                patch.object(core, '_virtual', empty_mock), \
+                patch.dict(core.__salt__, {'cmd.run': osarch_mock}):
+            os_grains = core.os_data()
 
         grains = {k: v for k, v in os_grains.items()
                   if k in set(["os", "os_family", "osfullname", "oscodename", "osfinger",
@@ -315,10 +307,11 @@ def test_suse_os_grains_sles11sp3(self):
         Test if OS grains are parsed correctly in SLES 11 SP3
         '''
         _os_release_map = {
-            'suse_release_file': '''SUSE Linux Enterprise Server 11 (x86_64)
-VERSION = 11
-PATCHLEVEL = 3
-''',
+            'suse_release_file': textwrap.dedent('''
+                SUSE Linux Enterprise Server 11 (x86_64)
+                VERSION = 11
+                PATCHLEVEL = 3
+                '''),
             'files': ["/etc/SuSE-release"],
         }
         expectation = {
@@ -587,8 +580,9 @@ def test_linux_memdata(self):
         )
         empty_mock = MagicMock(return_value={})
 
-        _proc_meminfo_file = '''MemTotal:       16277028 kB
-SwapTotal:       4789244 kB'''
+        _proc_meminfo = textwrap.dedent('''\
+            MemTotal:       16277028 kB
+            SwapTotal:       4789244 kB''')
 
         orig_import = __import__
         if six.PY2:
@@ -601,49 +595,28 @@ def _import_mock(name, *args):
                 raise ImportError('No module named lsb_release')
             return orig_import(name, *args)
 
-        # Skip the first if statement
-        with patch.object(salt.utils.platform, 'is_proxy',
-                          MagicMock(return_value=False)):
-            # Skip the selinux/systemd stuff (not pertinent)
-            with patch.object(core, '_linux_bin_exists',
-                              MagicMock(return_value=False)):
-                # Skip the init grain compilation (not pertinent)
-                with patch.object(os.path, 'exists', path_exists_mock):
-                    # Ensure that lsb_release fails to import
-                    with patch('{0}.__import__'.format(built_in),
-                               side_effect=_import_mock):
-                        # Skip all the /etc/*-release stuff (not pertinent)
-                        with patch.object(os.path, 'isfile', path_isfile_mock):
-                            # Make a bunch of functions return empty dicts,
-                            # we don't care about these grains for the
-                            # purposes of this test.
-                            with patch.object(
-                                    core,
-                                    '_linux_cpudata',
-                                    empty_mock):
-                                with patch.object(
-                                        core,
-                                        '_linux_gpu_data',
-                                        empty_mock):
-                                    with patch('salt.utils.files.fopen', mock_open()) as _proc_meminfo:
-                                        _proc_meminfo.return_value.__iter__.return_value = _proc_meminfo_file.splitlines()
-                                        with patch.object(
-                                                core,
-                                                '_hw_data',
-                                                empty_mock):
-                                            with patch.object(
-                                                    core,
-                                                    '_virtual',
-                                                    empty_mock):
-                                                with patch.object(
-                                                        core,
-                                                        '_ps',
-                                                        empty_mock):
-                                                    # Mock the osarch
-                                                    with patch.dict(
-                                                            core.__salt__,
-                                                            {'cmd.run': cmd_run_mock}):
-                                                        os_grains = core.os_data()
+        # Mock a bunch of stuff so we can isolate the mem stuff:
+        # - Skip the first if statement
+        # - Skip the init grain compilation (not pertinent)
+        # - Ensure that lsb_release fails to import
+        # - Skip all the /etc/*-release stuff (not pertinent)
+        # - Make a bunch of functions return empty dicts, we don't care
+        #   about these grains for the purposes of this test.
+        # - Mock the osarch
+        # - And most importantly, mock the contents of /proc/meminfo
+        with patch.object(salt.utils.platform, 'is_proxy', MagicMock(return_value=False)), \
+                patch.object(core, '_linux_bin_exists', MagicMock(return_value=False)), \
+                patch.object(os.path, 'exists', path_exists_mock), \
+                patch('{0}.__import__'.format(built_in), side_effect=_import_mock), \
+                patch.object(os.path, 'isfile', path_isfile_mock), \
+                patch.object(core, '_linux_cpudata', empty_mock), \
+                patch.object(core, '_linux_gpu_data', empty_mock), \
+                patch.object(core, '_hw_data', empty_mock), \
+                patch.object(core, '_virtual', empty_mock), \
+                patch.object(core, '_ps', empty_mock), \
+                patch.dict(core.__salt__, {'cmd.run': cmd_run_mock}), \
+                patch('salt.utils.files.fopen', mock_open(read_data=_proc_meminfo)):
+            os_grains = core.os_data()
 
         self.assertEqual(os_grains.get('mem_total'), 15895)
         self.assertEqual(os_grains.get('swap_total'), 4676)

tests/unit/grains/test_iscsi.py

@@ -5,6 +5,7 @@
 # Import Python libs
 from __future__ import absolute_import, print_function, unicode_literals
 import os
+import textwrap
 
 # Import Salt Testing Libs
 from tests.support.unit import TestCase, skipIf
@@ -51,15 +52,16 @@ def test_aix_iscsi_iqn_grains(self):
                          ['iqn.localhost.hostid.7f000001'])
 
     def test_linux_iscsi_iqn_grains(self):
-        _iscsi_file = '## DO NOT EDIT OR REMOVE THIS FILE!\n' \
-                      '## If you remove this file, the iSCSI daemon will not start.\n' \
-                      '## If you change the InitiatorName, existing access control lists\n' \
-                      '## may reject this initiator.  The InitiatorName must be unique\n' \
-                      '## for each iSCSI initiator.  Do NOT duplicate iSCSI InitiatorNames.\n' \
-                      'InitiatorName=iqn.1993-08.org.debian:01:d12f7aba36\n'
+        _iscsi_file = textwrap.dedent('''\
+            ## DO NOT EDIT OR REMOVE THIS FILE!
+            ## If you remove this file, the iSCSI daemon will not start.
+            ## If you change the InitiatorName, existing access control lists
+            ## may reject this initiator.  The InitiatorName must be unique
+            ## for each iSCSI initiator.  Do NOT duplicate iSCSI InitiatorNames.
+            InitiatorName=iqn.1993-08.org.debian:01:d12f7aba36
+            ''')
 
-        with patch('salt.utils.files.fopen', mock_open()) as iscsi_initiator_file:
-            iscsi_initiator_file.return_value.__iter__.return_value = _iscsi_file.splitlines()
+        with patch('salt.utils.files.fopen', mock_open(read_data=_iscsi_file)):
             iqn = iscsi._linux_iqn()
 
         assert isinstance(iqn, list)