From 878d144358c88eead6c62be2aa4454440ef76990 Mon Sep 17 00:00:00 2001
From: Bo Maryniuk <bo@maryniuk.net>
Date: Wed, 4 Jul 2018 10:28:57 +0200
Subject: [PATCH] Accounting for when files in an archive contain
 non-ascii characters (#95)

Updating integration/modules/test_archive to include filenames with unicode characters.

only convert to bytes when using Python2

Updating with requested changes.

Ensure member names are decoded before adding to various lists.

Adding a test to ensure archive.list returns the right results when a tar file contains a file with unicode in it's name.
---
 salt/modules/archive.py                   | 13 +++---
 salt/states/archive.py                    |  4 +-
 tests/integration/modules/test_archive.py | 52 ++++++++++++++++++++++-
 3 files changed, 59 insertions(+), 10 deletions(-)

diff --git a/salt/modules/archive.py b/salt/modules/archive.py
index f9750b938d..a0ad5989a7 100644
--- a/salt/modules/archive.py
+++ b/salt/modules/archive.py
@@ -186,12 +186,13 @@ def list_(name,
                 else {'fileobj': cached.stdout, 'mode': 'r|'}
             with contextlib.closing(tarfile.open(**open_kwargs)) as tar_archive:
                 for member in tar_archive.getmembers():
+                    _member = salt.utils.data.decode(member.name)
                     if member.issym():
-                        links.append(member.name)
+                        links.append(_member)
                     elif member.isdir():
-                        dirs.append(member.name + '/')
+                        dirs.append(_member + '/')
                     else:
-                        files.append(member.name)
+                        files.append(_member)
             return dirs, files, links
 
         except tarfile.ReadError:
@@ -410,9 +411,9 @@ def list_(name,
                 item.sort()
 
         if verbose:
-            ret = {'dirs': sorted(dirs),
-                   'files': sorted(files),
-                   'links': sorted(links)}
+            ret = {'dirs': sorted(salt.utils.data.decode_list(dirs)),
+                   'files': sorted(salt.utils.data.decode_list(files)),
+                   'links': sorted(salt.utils.data.decode_list(links))}
             ret['top_level_dirs'] = [x for x in ret['dirs']
                                      if x.count('/') == 1]
             ret['top_level_files'] = [x for x in ret['files']
diff --git a/salt/states/archive.py b/salt/states/archive.py
index c4ea3019b1..ff1101bf6a 100644
--- a/salt/states/archive.py
+++ b/salt/states/archive.py
@@ -1100,7 +1100,7 @@ def extracted(name,
                                          and not stat.S_ISDIR(x)),
                      (contents['links'], stat.S_ISLNK)):
                 for path in path_list:
-                    full_path = os.path.join(name, path)
+                    full_path = salt.utils.path.join(name, path)
                     try:
                         path_mode = os.lstat(full_path.rstrip(os.sep)).st_mode
                         if not func(path_mode):
@@ -1269,7 +1269,7 @@ def extracted(name,
                 if options is None:
                     try:
                         with closing(tarfile.open(cached, 'r')) as tar:
-                            tar.extractall(name)
+                            tar.extractall(salt.utils.stringutils.to_str(name))
                             files = tar.getnames()
                             if trim_output:
                                 files = files[:trim_output]
diff --git a/tests/integration/modules/test_archive.py b/tests/integration/modules/test_archive.py
index 59fe2f5f61..4301b9e3b0 100644
--- a/tests/integration/modules/test_archive.py
+++ b/tests/integration/modules/test_archive.py
@@ -47,7 +47,7 @@ class ArchiveTest(ModuleCase):
         self.arch = os.path.join(self.base_path, 'archive.{0}'.format(arch_fmt))
         self.dst = os.path.join(self.base_path, '{0}_dst_dir'.format(arch_fmt))
 
-    def _set_up(self, arch_fmt):
+    def _set_up(self, arch_fmt, unicode_filename=False):
         '''
         Create source file tree and destination directory
 
@@ -62,7 +62,11 @@ class ArchiveTest(ModuleCase):
 
         # Create source
         os.makedirs(self.src)
-        with salt.utils.files.fopen(os.path.join(self.src, 'file'), 'w') as theorem:
+        if unicode_filename:
+            filename = 'file®'
+        else:
+            filename = 'file'
+        with salt.utils.files.fopen(os.path.join(self.src, filename), 'w') as theorem:
             theorem.write(textwrap.dedent(salt.utils.stringutils.to_str(r'''\
                 Compression theorem of computational complexity theory:
 
@@ -150,6 +154,50 @@ class ArchiveTest(ModuleCase):
 
         self._tear_down()
 
+    @skipIf(not salt.utils.path.which('tar'), 'Cannot find tar executable')
+    def test_tar_pack_unicode(self):
+        '''
+        Validate using the tar function to create archives
+        '''
+        self._set_up(arch_fmt='tar', unicode_filename=True)
+
+        # Test create archive
+        ret = self.run_function('archive.tar', ['-cvf', self.arch], sources=self.src)
+        self.assertTrue(isinstance(ret, list), six.text_type(ret))
+        self._assert_artifacts_in_ret(ret)
+
+        self._tear_down()
+
+    @skipIf(not salt.utils.path.which('tar'), 'Cannot find tar executable')
+    def test_tar_unpack_unicode(self):
+        '''
+        Validate using the tar function to extract archives
+        '''
+        self._set_up(arch_fmt='tar', unicode_filename=True)
+        self.run_function('archive.tar', ['-cvf', self.arch], sources=self.src)
+
+        # Test extract archive
+        ret = self.run_function('archive.tar', ['-xvf', self.arch], dest=self.dst)
+        self.assertTrue(isinstance(ret, list), six.text_type(ret))
+        self._assert_artifacts_in_ret(ret)
+
+        self._tear_down()
+
+    @skipIf(not salt.utils.path.which('tar'), 'Cannot find tar executable')
+    def test_tar_list_unicode(self):
+        '''
+        Validate using the tar function to extract archives
+        '''
+        self._set_up(arch_fmt='tar', unicode_filename=True)
+        self.run_function('archive.tar', ['-cvf', self.arch], sources=self.src)
+
+        # Test list archive
+        ret = self.run_function('archive.list', name=self.arch)
+        self.assertTrue(isinstance(ret, list), six.text_type(ret))
+        self._assert_artifacts_in_ret(ret)
+
+        self._tear_down()
+
     @skipIf(not salt.utils.path.which('gzip'), 'Cannot find gzip executable')
     def test_gzip(self):
         '''
-- 
2.17.1


