[diffoscope] Prevent overwriting the destination when extracting with libarchive

[Ximin Luo]

Hey Lunar,

This:

commit c8c4bc40f8123f0bfbdb628911618024297bc7fe
Author: Jérémy Bobbio <lunar at debian.org>
Date:   Thu Dec 3 12:14:26 2015 +0000

    Prevent overwriting the destination when extracting with libarchive
    
    Thanks Baptiste Daroussin for the inspiration.

diff --git a/diffoscope/comparators/libarchive.py b/diffoscope/comparators/libarchive.py
index f8d5b4f..7d93d44 100644
--- a/diffoscope/comparators/libarchive.py
+++ b/diffoscope/comparators/libarchive.py
@@ -120,7 +120,10 @@ class LibarchiveContainer(Archive):
         return member_names
 
     def extract(self, member_name, dest_dir):
-        dest_path = os.path.join(dest_dir, os.path.basename(member_name))
+        dest_name = os.path.basename(member_name)
+        if not dest_name:
+            raise ValueError('member_name should not be a directory')
+        dest_path = os.path.join(dest_dir, dest_name)
         logger.debug('libarchive extracting %s to %s', member_name, dest_path)
         with libarchive.file_reader(self.source.path) as archive:
             for entry in archive:

is causing errors with the attached files, which are artefacts of glibc, specifically of libc6-dev-i386_2.24-5_amd64.deb. They contain ar archive entries called "/SYM64/".

What was the original reason for the change? I am considering this patch instead:

commit a986edc1997723ea0f4a5eaae08d25ae33117c06
Author: Ximin Luo <infinity0 at debian.org>
Date:   Wed Nov 9 20:13:25 2016 +0100

    Handle /SYM64/ entries in .a archives

diff --git a/diffoscope/comparators/libarchive.py b/diffoscope/comparators/libarchive.py
index e2536c1..8edafa4 100644
--- a/diffoscope/comparators/libarchive.py
+++ b/diffoscope/comparators/libarchive.py
@@ -167,9 +167,10 @@ class LibarchiveContainer(Archive):
         return member_names
 
     def extract(self, member_name, dest_dir):
-        dest_name = os.path.basename(member_name)
+        # sometimes symbol names are like "/SYM64/" and we need to strip the /
+        dest_name = os.path.basename(member_name.rstrip("/"))
         if not dest_name:
-            raise ValueError('member_name should not be a directory')
+            raise ValueError('member_name should not be a directory: %s' % member_name)
         dest_path = os.path.join(dest_dir, dest_name)
         logger.debug('libarchive extracting %s to %s', member_name, dest_path)
         with libarchive.file_reader(self.source.path) as archive:

but I'm not sure if it will mess with the reasoning for your original commit.

X

-- 
GPG: ed25519/56034877E1F87C35
GPG: rsa4096/1318EFAC5FBBDBCE
https://github.com/infinity0/pubkeys.git
-------------- next part --------------
A non-text attachment was scrubbed...
Name: libBrokenLocale.a.0
Type: application/octet-stream
Size: 1462 bytes
Desc: not available
URL: <http://lists.reproducible-builds.org/pipermail/diffoscope/attachments/20161109/b1db29d0/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: libBrokenLocale.a.1
Type: application/octet-stream
Size: 1462 bytes
Desc: not available
URL: <http://lists.reproducible-builds.org/pipermail/diffoscope/attachments/20161109/b1db29d0/attachment-0001.obj>