archive_viewer: implement full front-to-back file scan & fix crash on…

… exit (#5554)

* archive: CArchiveReader: implement full back-to-front file search for MAGIC

Allow archive-viewer to open binaries with more than 4096 bytes
of additional data past the appended package (e.g., macOS binaries
with code signature).

* cliutils: archive_viewer: fix crash when quitting or moving up a level

Calling arch.lib.close() when cleaning up the archive when either
moving up a level or quitting the archive_viewer application
triggers the assert in pyimod02_archive.ArchiveFile.__getattr__.

This is because local file object is set only between __enter__
and __exit__ calls, i.e., while in the `with arch.lib:` block.
Which also means that there's no need for attempting to close
those file handles in the first place, and the error is probably
a regression from when the thread-local file objects were
introduced.

PyInstaller/archive/readers.py

@@ -17,6 +17,7 @@
 # TODO clean up this module
 
 import struct
+import os
 
 
 from PyInstaller.loader.pyimod02_archive import ArchiveReader
@@ -142,19 +143,36 @@ def checkmagic(self):
         if self.length:
             self.lib.seek(self.start + self.length, 0)
         else:
-            self.lib.seek(0, 2)
-        filelen = self.lib.tell()
-
-        self.lib.seek(max(0, filelen-4096))
-        searchpos = self.lib.tell()
-        buf = self.lib.read(min(filelen, 4096))
-        pos = buf.rfind(self.MAGIC)
-        if pos == -1:
+            self.lib.seek(0, os.SEEK_END)
+        end_pos = self.lib.tell()
+
+        SEARCH_CHUNK_SIZE = 8192
+        magic_offset = -1
+        while end_pos >= len(self.MAGIC):
+            start_pos = max(end_pos - SEARCH_CHUNK_SIZE, 0)
+            chunk_size = end_pos - start_pos
+            # Is the remaining chunk large enough to hold the pattern?
+            if chunk_size < len(self.MAGIC):
+                break
+            # Read and scan the chunk
+            self.lib.seek(start_pos, os.SEEK_SET)
+            buf = self.lib.read(chunk_size)
+            pos = buf.rfind(self.MAGIC)
+            if pos != -1:
+                magic_offset = start_pos + pos
+                break
+            # Adjust search location for next chunk; ensure proper
+            # overlap
+            end_pos = start_pos + len(self.MAGIC) - 1
+        if magic_offset == -1:
             raise RuntimeError("%s is not a valid %s archive file" %
                                (self.path, self.__class__.__name__))
-        filelen = searchpos + pos + self._cookie_size
+        filelen = magic_offset + self._cookie_size
+        # Read the whole cookie
+        self.lib.seek(magic_offset, os.SEEK_SET)
+        buf = self.lib.read(self._cookie_size)
         (magic, totallen, tocpos, toclen, pyvers, pylib_name) = struct.unpack(
-            self._cookie_format, buf[pos:pos+self._cookie_size])
+            self._cookie_format, buf)
         if magic != self.MAGIC:
             raise RuntimeError("%s is not a valid %s archive file" %
                                (self.path, self.__class__.__name__))

PyInstaller/utils/cliutils/archive_viewer.py

@@ -65,7 +65,6 @@ def main(name, brief, debug, rec_debug, **unused_options):
         if cmd == 'U':
             if len(stack) > 1:
                 arch = stack[-1][1]
-                arch.lib.close()
                 del stack[-1]
             name, arch = stack[-1]
             show(name, arch)
@@ -106,8 +105,6 @@ def main(name, brief, debug, rec_debug, **unused_options):
 
 def do_cleanup():
     global stack, cleanup
-    for (name, arch) in stack:
-        arch.lib.close()
     stack = []
     for filename in cleanup:
         try:

news/2372.bugfix.rst

@@ -0,0 +1,3 @@
+``CArchiveReader`` now performs full back-to-front file search for
+``MAGIC``, allowing ``pyi-archive_viewer`` to open binaries with extra
+appended data after embedded package (e.g., digital signature).

news/5554.bugfix.rst

@@ -0,0 +1,2 @@
+Fix a crash in ``pyi-archive_viewer`` when quitting the application or
+moving up a level.