#390 Improve handling for archives with unicode names.

 * Add extract_with_fallback wrapper for 7zip extraction
 * New tests for achives with unicode. 

Signed-off-by: Philippe Ombredanne <[email protected]>

Author: pombredanne

src/extractcode/archive.py

@@ -1,5 +1,5 @@
 #
-# Copyright (c) 2015 nexB Inc. and others. All rights reserved.
+# Copyright (c) 2016 nexB Inc. and others. All rights reserved.
 # http://nexb.com and https://github.com/nexB/scancode-toolkit/
 # The ScanCode software is licensed under the Apache License version 2.0.
 # Data generated with ScanCode require an acknowledgment.
@@ -85,29 +85,6 @@
  - http://en.wikipedia.org/wiki/List_of_file_formats#Archive_and_compressed
 """
 
-# high level aliases to lower level extraction functions
-extract_tar = libarchive2.extract
-extract_patch = patch.extract
-
-extract_deb = libarchive2.extract
-extract_ar = libarchive2.extract
-extract_msi = sevenzip.extract
-extract_cpio = libarchive2.extract
-extract_7z = libarchive2.extract
-extract_zip = libarchive2.extract
-
-extract_iso = sevenzip.extract
-extract_rar = sevenzip.extract
-extract_rpm = sevenzip.extract
-extract_xz = sevenzip.extract
-extract_lzma = sevenzip.extract
-extract_squashfs = sevenzip.extract
-extract_cab = sevenzip.extract
-extract_nsis = sevenzip.extract
-extract_ishield = sevenzip.extract
-extract_Z = sevenzip.extract
-
-
 # if strict, all hanlders criteria must be matched for it to be selected
 Handler = namedtuple('Handler', ['name', 'filetypes', 'mimetypes', 'extensions', 'kind', 'extractors', 'strict'])
 
@@ -319,9 +296,9 @@ def extract_twice(location, target_dir, extractor1, extractor2):
     covers most common cases.
     """
     abs_location = os.path.abspath(os.path.expanduser(location))
-    abs_target_dir = os.path.abspath(os.path.expanduser(target_dir))
+    abs_target_dir = unicode(os.path.abspath(os.path.expanduser(target_dir)))
     # extract first the intermediate payload to a temp dir
-    temp_target = fileutils.get_temp_dir('extract')
+    temp_target = unicode(fileutils.get_temp_dir('extract'))
     warnings = extractor1(abs_location, temp_target)
     if DEBUG:
         logger.debug('extract_twice: temp_target: %(temp_target)r' % locals())
@@ -335,16 +312,73 @@ def extract_twice(location, target_dir, extractor1, extractor2):
             for extracted1_loc in inner_archives:
                 if DEBUG:
                     logger.debug('extract_twice: extractor2: %(extracted1_loc)r' % locals())
-                warnings.extend(extractor2(extracted1_loc, target_dir))
+                warnings.extend(extractor2(extracted1_loc, abs_target_dir))
     finally:
         # cleanup the temporary output from extractor1
         fileutils.delete(temp_target)
     return warnings
 
 
-"""
-List of archive handlers.
-"""
+def extract_with_fallback(location, target_dir, extractor1, extractor2):
+    """
+    Extract archive at `location` to `target_dir` trying first `extractor1` function.
+    If extract fails, attempt extraction again with the `extractor2` function.
+    Return a list of warning messages. Raise exceptions on errors.
+
+    Note: there are a few cases where the primary extractor for a type may fail and
+    a secondary extractor will succeed.
+    """
+    abs_location = os.path.abspath(os.path.expanduser(location))
+    abs_target_dir = unicode(os.path.abspath(os.path.expanduser(target_dir)))
+    # attempt extract first to a temp dir
+    temp_target1 = unicode(fileutils.get_temp_dir('extract1'))
+    try:
+        warnings = extractor1(abs_location, temp_target1)
+        if DEBUG:
+            logger.debug('extract_with_fallback: temp_target1: %(temp_target1)r' % locals())
+        fileutils.copytree(temp_target1, abs_target_dir)
+    except:
+        try:
+            temp_target2 = unicode(fileutils.get_temp_dir('extract2'))
+            warnings = extractor2(abs_location, temp_target2)
+            if DEBUG:
+                logger.debug('extract_with_fallback: temp_target2: %(temp_target2)r' % locals())
+            fileutils.copytree(temp_target2, abs_target_dir)
+        finally:
+            fileutils.delete(temp_target2)
+    finally:
+        fileutils.delete(temp_target1)
+    return warnings
+
+
+# High level aliases to lower level extraction functions
+########################################################
+extract_tar = libarchive2.extract
+extract_patch = patch.extract
+
+extract_deb = libarchive2.extract
+extract_ar = libarchive2.extract
+extract_msi = sevenzip.extract
+extract_cpio = libarchive2.extract
+
+# sevenzip should be best at extracting 7zip but most often libarchive is better first
+extract_7z = functools.partial(extract_with_fallback, extractor1=libarchive2.extract, extractor2=sevenzip.extract)
+
+extract_zip = libarchive2.extract
+extract_iso = sevenzip.extract
+extract_rar = sevenzip.extract
+extract_rpm = sevenzip.extract
+extract_xz = sevenzip.extract
+extract_lzma = sevenzip.extract
+extract_squashfs = sevenzip.extract
+extract_cab = sevenzip.extract
+extract_nsis = sevenzip.extract
+extract_ishield = sevenzip.extract
+extract_Z = sevenzip.extract
+
+
+# Archive handlers.
+####################
 
 TarHandler = Handler(
     name='Tar',
@@ -795,6 +829,7 @@ def extract_twice(location, target_dir, extractor1, extractor2):
     strict=True
 )
 
+# Actual list of handlers
 
 archive_handlers = [
     TarHandler,

tests/extractcode/data/archive/7z/7zip_unicode.7z

@@ -96,14 +96,7 @@ def test_get_extractors(self):
         for test_file, expected in test_data:
             test_loc = self.get_test_loc(test_file)
             extractors = archive.get_extractors(test_loc)
-            ft = typecode.contenttype.get_type(test_loc).filetype_file
-            mt = typecode.contenttype.get_type(test_loc).mimetype_file
-            fe = fileutils.file_extension(test_loc).lower()
-            msg = (expected[0].__module__ + '.' + expected[0].__name__
-                   + '!='
-                   + extractors[0].__module__ + '.' + extractors[0].__name__
-                   + ' for %(test_file)r') % locals()
-            assert expected == extractors, msg
+            assert expected == extractors
 
     def test_get_extractors_with_kinds(self):
         test_data = [
@@ -394,6 +387,15 @@ def test_extract_targz_from_apache_should_not_return_errors(self):
         assert [] == result
         assert os.listdir(test_dir)
 
+    def test_extract_targz_with_unicode_path_should_extract_without_error(self):
+        test_file = self.get_test_loc('archive/tgz/tgz_unicode.tgz')
+        test_dir = self.get_temp_dir()
+        extractor = archive.get_extractor(test_file)
+        assert archive.extract_tar == extractor
+        result = archive.extract_tar(test_file, test_dir)
+        assert [] == result
+        assert os.listdir(test_dir)
+
 
 class TestGzip(BaseArchiveTestCase):
     def test_uncompress_gzip_basic(self):
@@ -841,6 +843,13 @@ def test_extract_zip_can_extract_windows_media_player_skins(self):
         expected = ['32px.png', 'go.js', 'go.wms']
         check_files(test_dir, expected)
 
+    def test_extract_zip_with_unicode_path_should_extract_without_error(self):
+        test_file = self.get_test_loc('archive/zip/zip_unicode.zip')
+        test_dir = self.get_temp_dir()
+        result = archive.extract_zip(test_file, test_dir)
+        assert [] == result
+        assert os.listdir(test_dir)
+
 
 class TestLibarch(BaseArchiveTestCase):
     def test_extract_zip_with_relative_path_libarchive(self):
@@ -1588,12 +1597,11 @@ def test_extract_7z_with_broken_archive_with7z(self):
         msg = 'No error returned'
         self.assertRaisesInstance(ExtractErrorFailedToExtract(msg), sevenzip.extract, test_file, test_dir)
 
-    def test_extract_7z_with_broken_archive(self):
+    def test_extract_7z_with_broken_archive_does_not_fail_when_using_fallback(self):
         test_file = self.get_test_loc('archive/7z/corrupted7z.7z')
         test_dir = self.get_temp_dir()
-        expected = libarchive2.ArchiveError()
-        expected.msg = 'Damaged 7-Zip archive'
-        self.assertRaisesInstance(expected, archive.extract_7z, test_file, test_dir)
+        msg = 'No error returned'
+        self.assertRaisesInstance(ExtractErrorFailedToExtract(msg), archive.extract_7z, test_file, test_dir)
 
     def test_extract_7z_with_non_existing_archive(self):
         test_file = 'archive/7z/I_DO_NOT_EXIST.zip'
@@ -1647,9 +1655,31 @@ def test_extract_7z_with_password_with_7z(self):
     def test_extract_7z_with_password(self):
         test_file = self.get_test_loc('archive/7z/7zip_password.7z')
         test_dir = self.get_temp_dir()
-        expected = Exception("'The file content is encrypted, but currently not supported'")
+        expected = Exception('Password protected archive, unable to extract')
         self.assertRaisesInstance(expected, archive.extract_7z, test_file, test_dir)
 
+    def test_extract_7zip_native_with_unicode_path_should_extract_without_error(self):
+        test_file = self.get_test_loc('archive/7z/7zip_unicode.7z')
+        test_dir = self.get_temp_dir()
+        result = sevenzip.extract(test_file, test_dir)
+        assert [] == result
+        assert 2 == len(os.listdir(os.path.join(test_dir, 'zip')))
+
+    def test_extract_7zip_with_fallback_with_unicode_path_should_extract_without_error(self):
+        test_file = self.get_test_loc('archive/7z/7zip_unicode.7z')
+        test_dir = self.get_temp_dir()
+        result = archive.extract_7z(test_file, test_dir)
+        assert [] == result
+        assert 2 == len(os.listdir(os.path.join(test_dir, 'zip')))
+
+    def test_extract_7zip_libarchive_with_unicode_path_extracts_with_errors(self):
+        test_file = self.get_test_loc('archive/7z/7zip_unicode.7z')
+        test_dir = self.get_temp_dir()
+        try:
+            archive.extract_7z(test_file, test_dir)
+        except libarchive2.ArchiveError, e:
+            assert 'Damaged 7-Zip archive' in e.msg
+
 
 class TestIso(BaseArchiveTestCase):
     def test_extract_iso_basic(self):