Fixed #31 : Ensured that location are resolved

 * the key issue in #31 was relative paths
 * we now resolve to an absolute path before extracting
 * archive.extract_twice "else" block was incorrect and displaying
   a warning all times.
 * paths are resolved too in extract.py and at the lower level in
   sevenzip and libarchive2.
 * added test for RPM with xz-compressed CPIO 
 * added tests for relative path extraction at all levels
   including from the cli and in archive.extract_twice

Author: pombredanne

src/extractcode/archive.py

@@ -27,13 +27,12 @@
 from collections import namedtuple
 import functools
 import logging
+import os
 
 from commoncode import fileutils
 from commoncode import filetype
 import typecode
 
-import extractcode
-
 from extractcode import all_kinds
 from extractcode import regular
 from extractcode import package
@@ -46,10 +45,20 @@
 from extractcode import patch
 from extractcode import sevenzip
 from extractcode import libarchive2
+from extractcode import extracted_files
 from extractcode.uncompress import uncompress_gzip
 from extractcode.uncompress import uncompress_bzip2
 
 
+logger = logging.getLogger(__name__)
+DEBUG = True
+DEBUG_DEEP = False
+# import sys
+# logging.basicConfig(level=logging.DEBUG, stream=sys.stdout)
+# logger.setLevel(logging.DEBUG)
+
+
+
 """
 Archive formats handling. The purpose of this module is to select an extractor
 suitable for the accurate extraction of a given kind of archive. An extractor is
@@ -98,12 +107,6 @@
 extract_ishield = sevenzip.extract
 extract_Z = sevenzip.extract
 
-DEBUG = False
-logger = logging.getLogger(__name__)
-# import sys
-# logging.basicConfig(level=logging.DEBUG, stream=sys.stdout)
-# logger.setLevel(logging.DEBUG)
-
 
 Handler = namedtuple('Handler', ['name', 'types', 'mimes', 'exts', 'kind', 'extractors'])
 
@@ -112,7 +115,6 @@ def can_extract(location):
     """
     Return True if this location can be extracted by some handler.
     """
-    assert location
     handlers = list(get_handlers(location))
     if handlers:
         return True
@@ -123,8 +125,7 @@ def should_extract(location, kinds):
     Return True if this location should be extracted based on the provided
     kinds
     """
-    assert location
-    assert kinds
+    location = os.path.abspath(os.path.expanduser(location))
     if get_extractor(location, kinds):
         return True
 
@@ -135,7 +136,7 @@ def get_extractor(location, kinds=all_kinds):
     an None if no extract function is found.
     """
     assert location
-    assert kinds
+    location = os.path.abspath(os.path.expanduser(location))
     extractors = get_extractors(location, kinds)
     if not extractors:
         return None
@@ -157,14 +158,15 @@ def get_extractors(location, kinds=all_kinds):
     Return a list of extractors that can extract the file at
     location or an empty list.
     """
+    location = os.path.abspath(os.path.expanduser(location))
     if filetype.is_file(location):
-        handlers = get_handlers(location)
+        handlers = list(get_handlers(location))
         if handlers:
             candidates = score_handlers(handlers)
-        if candidates:
-            best = pick_best_handler(candidates, kinds)
-            if best:
-                return best.extractors
+            if candidates:
+                best = pick_best_handler(candidates, kinds)
+                if best:
+                    return best.extractors
     return []
 
 
@@ -174,7 +176,6 @@ def get_handlers(location):
     extension_matched,) for this `location`.
     """
     if filetype.is_file(location):
-
         T = typecode.contenttype.get_type(location)
         ftype = T.filetype_file.lower()
         mtype = T.mimetype_file
@@ -199,13 +200,13 @@ def get_handlers(location):
             if handler.exts:
                 extension_matched = location.lower().endswith(handler.exts)
 
-            if DEBUG:
+            if DEBUG_DEEP:
                 logger.debug('get_handlers: %(location)s: ftype: %(ftype)s, mtype: %(mtype)s ' % locals())
                 logger.debug('get_handlers: %(location)s: matched type: %(type_matched)s, mime: %(mime_matched)s, ext: %(extension_matched)s' % locals())
 
 
             if type_matched or mime_matched or extension_matched:
-                if DEBUG:
+                if DEBUG_DEEP:
                     logger.debug('get_handlers: %(location)s: matched type: %(type_matched)s, mime: %(mime_matched)s, ext: %(extension_matched)s' % locals())
                     logger.debug('get_handlers: %(location)s: handler: %(handler)r' % locals())
                 yield handler, type_matched, mime_matched, extension_matched
@@ -307,21 +308,33 @@ def extract_twice(location, target_dir, extractor1, extractor2):
     the `extractor1` function to a temporary directory then the `extractor2`
     function on the extracted payload of `extractor1`.
 
-    Return a mapping of path->warning_message.
+    Return a list of warning messages. Raise exceptions on errors.
 
     Typical nested archives include compressed tarballs and RPMs (containing a
     compressed cpio).
+
+    Note: it would be easy to support deeper extractor chains, but this gets
+    hard to trace and debug very quickly. A depth of two is simple and sane and
+    covers most common cases.
     """
+    abs_location = os.path.abspath(os.path.expanduser(location))
+    abs_target_dir = os.path.abspath(os.path.expanduser(target_dir))
     # extract first the intermediate payload to a temp dir
     temp_target = fileutils.get_temp_dir('extract')
-    warnings = extractor1(location, temp_target)
+    warnings = extractor1(abs_location, temp_target)
+    if DEBUG:
+        logger.debug('extract_twice: temp_target: %(temp_target)r' % locals())
 
     # extract this intermediate payload to the final target_dir
     try:
-        for extracted1_loc in extractcode.extracted_files(temp_target):
-            warnings.extend(extractor2(extracted1_loc, target_dir))
+        inner_archives = list(extracted_files(temp_target))
+        if not inner_archives:
+            warnings.append(location + ': No files found in archive.')
         else:
-            warnings.append(location+ ': No files found in archive.')
+            for extracted1_loc in inner_archives:
+                if DEBUG:
+                    logger.debug('extract_twice: extractor2: %(extracted1_loc)r' % locals())
+                warnings.extend(extractor2(extracted1_loc, target_dir))
     finally:
         # cleanup the temporary output from extractor1
         fileutils.delete(temp_target)

src/extractcode/extract.py

@@ -27,6 +27,8 @@
 from collections import namedtuple
 from functools import partial
 import logging
+from os.path import abspath
+from os.path import expanduser
 from os.path import join
 
 from commoncode import fileutils
@@ -117,11 +119,11 @@ def extract(location, kinds=extractcode.default_kinds, recurse=False):
     """
     ignored = partial(ignore.is_ignored, ignores=ignore.default_ignores, unignores={})
     if DEBUG:
-        logger.debug('extract:start:' + location + ' recurse:' + repr(recurse) + '\n')
-
-    for top, dirs, files in fileutils.walk(location, ignored):
+        logger.debug('extract:start: %(location)r  recurse: %(recurse)r\n' % locals())
+    abs_location = abspath(expanduser(location))
+    for top, dirs, files in fileutils.walk(abs_location, ignored):
         if DEBUG:
-            logger.debug('extract:walk: top:' + top + ' dirs:' + repr(dirs) + ' files:' + repr(files))
+            logger.debug('extract:walk: top:  %(top)r dirs: %(dirs)r files: r(files)r' % locals())
 
         if not recurse:
             if DEBUG:
@@ -135,26 +137,28 @@ def extract(location, kinds=extractcode.default_kinds, recurse=False):
             loc = join(top, f)
             if not recurse and extractcode.is_extraction_path(loc):
                 if DEBUG:
-                    logger.debug('extract:walk not recurse: skipped  file:' + loc)
+                    logger.debug('extract:walk not recurse: skipped  file: %(loc)r' % locals())
                 continue
 
             if not archive.should_extract(loc, kinds):
                 if DEBUG:
-                    logger.debug('extract:walk: skipped file: not should_extract:' + loc)
+                    logger.debug('extract:walk: skipped file: not should_extract: %(loc)r' % locals())
                 continue
 
-            target = join(top, extractcode.get_extraction_path(loc))
+            target = join(abspath(top), extractcode.get_extraction_path(loc))
+            if DEBUG:
+                logger.debug('extract:target: %(target)r' % locals())
             for xevent in extract_file(loc, target, kinds):
                 if DEBUG:
-                    logger.debug('extract:walk:extraction event:' + repr(xevent))
+                    logger.debug('extract:walk:extraction event: %(xevent)r' % locals())
                 yield xevent
 
             if recurse:
                 if DEBUG:
-                    logger.debug('extract:walk: recursing on:' + target)
+                    logger.debug('extract:walk: recursing on target: %(target)r' % locals())
                 for xevent in extract(target, kinds, recurse):
                     if DEBUG:
-                        logger.debug('extract:walk:recurse:extraction event:' + repr(xevent))
+                        logger.debug('extract:walk:recurse:extraction event: %(xevent)r' % locals())
                     yield xevent
 
 
@@ -167,8 +171,7 @@ def extract_file(location, target, kinds=extractcode.default_kinds):
     errors = []
     extractor = archive.get_extractor(location, kinds)
     if DEBUG:
-        logger.debug('extract_file: extractor: for:' + location
-                     + ' with kinds: ' + repr(kinds) + ': '
+        logger.debug('extract_file: extractor: for: %(location)r with kinds: r(kinds)r : ' % locals()
                      + getattr(extractor, '__module__', '')
                      + '.' + getattr(extractor, '__name__', ''))
     if extractor:
@@ -178,12 +181,13 @@ def extract_file(location, target, kinds=extractcode.default_kinds):
             # if there is an error,  the extracted files will not be moved
             # to target
             tmp_tgt = fileutils.get_temp_dir('extract')
-            warnings.extend(extractor(location, tmp_tgt))
+            abs_location= abspath(expanduser(location))
+            warnings.extend(extractor(abs_location, tmp_tgt))
             fileutils.copytree(tmp_tgt, target)
             fileutils.delete(tmp_tgt)
         except Exception, e:
             if DEBUG:
-                logger.debug('extract_file: ERROR: %(errors)r, %(e)r.\n' % locals())
+                logger.debug('extract_file: ERROR: %(location)r: %(errors)r, %(e)r.\n' % locals())
             errors = [str(e).strip(' \'"')]
         finally:
             yield ExtractEvent(location, target, done=True, warnings=warnings, errors=errors)

src/extractcode/libarchive2.py

@@ -101,13 +101,14 @@ def extract(location, target_dir):
     """
     assert location
     assert target_dir
-
+    abs_location = os.path.abspath(os.path.expanduser(location))
+    abs_target_dir = os.path.abspath(os.path.expanduser(target_dir))
     warnings = []
 
-    for entry in list_entries(location):
+    for entry in list_entries(abs_location):
         if not (entry.isdir or entry.isfile):
             continue
-        _target_path = entry.write(target_dir, transform_path=paths.resolve)
+        _target_path = entry.write(abs_target_dir, transform_path=paths.resolve)
         if entry.warnings:
             msgs = [w.strip('"\' ') for w in entry.warnings if w.strip('"\' ')]
             msgs = msgs or ['No message provided']
@@ -121,10 +122,12 @@ def list_entries(location):
     """
     Return a list entries of archive file at `location`.
     """
-    assert os.path.exists(location)
-    assert os.path.isfile(location)
+    assert location
+    abs_location = os.path.abspath(os.path.expanduser(location))
+    assert os.path.isfile(abs_location)
+
     # TODO: harden error handling
-    with Archive(location) as archive:
+    with Archive(abs_location) as archive:
         for entry in archive:
             yield entry
 
@@ -314,16 +317,6 @@ def write(self, target_dir, transform_path=lambda x: x):
                 self.warnings.append(msg)
             return target_path
 
-#         except ArchiveError, ae:
-#             if ae.msg and ae.msg.startswith('Encrypted file is unsupported'):
-#                 raise ArchiveErrorPasswordProtected(root_ex=ae)
-#             else:
-#                 raise
-# 
-#         except Exception, e:
-#             raise
-#             raise ArchiveError(root_ex=e)
-
     def __repr__(self):
         return ('Entry('
                     'path=%(path)r,'

src/extractcode/sevenzip.py

@@ -39,7 +39,7 @@
 logger = logging.getLogger('extractcode')
 # logging.basicConfig(level=logging.DEBUG)
 
-root_dir = os.path.join(os.path.dirname(__file__), 'bin')
+root_dir = os.path.abspath(os.path.join(os.path.dirname(__file__), 'bin'))
 
 
 """
@@ -123,6 +123,8 @@ def extract(location, target_dir, arch_type='*'):
     """
     assert location
     assert target_dir
+    abs_location = os.path.abspath(os.path.expanduser(location))
+    abs_target_dir = os.path.abspath(os.path.expanduser(target_dir))
 
     # note: there are some issues with the extraction of debian .deb ar files
     # see sevenzip bug http://sourceforge.net/p/sevenzip/bugs/1472/
@@ -159,17 +161,17 @@ def extract(location, target_dir, arch_type='*'):
 
     # Note: 7z does extract in the current directory so we cwd to target_dir
     args = [extract, yes_to_all, auto_rename_dupe_names,
-            arch_type, password, location]
+            arch_type, password, abs_location]
     rc, stdout, _stderr = command.execute(cmd='7z',
                                           args=args,
-                                          cwd=target_dir,
+                                          cwd=abs_target_dir,
                                           env=timezone,
                                           root_dir=root_dir)
     if rc != 0:
         error = get_7z_errors(stdout) or 'No error returned'
         raise ExtractErrorFailedToExtract(error)
 
-    extractcode.remove_backslashes(target_dir)
+    extractcode.remove_backslashes(abs_target_dir)
     return get_7z_warnings(stdout)
 
 
@@ -181,6 +183,7 @@ def list_entries(location, arch_type='*'):
     None).
     """
     assert location
+    abs_location = os.path.abspath(os.path.expanduser(location))
     # 7z arguments
     listing = 'l'
 
@@ -203,8 +206,7 @@ def list_entries(location, arch_type='*'):
     # not work on Windows, because 7z is not using the TZ env var there.
     timezone = os.environ.update({'TZ': 'GMT'})
 
-    # Note: 7z does listing in the current directory so we cwd to target_dir
-    args = [listing, tech_info, arch_type, output_as_utf, password, location]
+    args = [listing, tech_info, arch_type, output_as_utf, password, abs_location]
     rc, stdout, _stderr = command.execute(cmd='7z',
                                           args=args,
                                           env=timezone,
@@ -344,4 +346,3 @@ def parse_7z_listing(location, utf=False):
             entries.append(as_entry(infos))
 
     return entries
-