Merge pull request #4852 from rommapp/fix/7z-hash-timeout-scales-with-size

fix(scan): raise the 7-Zip timeout default and keep partial bytes out of the fallback hash

backend/config/__init__.py

@@ -52,7 +52,7 @@ ROM_UPLOAD_TTL: Final[int] = 86400  # 24 hours
 ROM_UPLOAD_ASSEMBLING_EXT: Final[str] = "assembling"
 
 # SEVEN ZIP
-SEVEN_ZIP_TIMEOUT: Final[int] = safe_int(_get_env("SEVEN_ZIP_TIMEOUT"), 60)
+SEVEN_ZIP_TIMEOUT: Final[int] = safe_int(_get_env("SEVEN_ZIP_TIMEOUT"), 180)
 
 # ROM PATCHER
 ROM_PATCHER_TIMEOUT: Final[int] = safe_int(_get_env("ROM_PATCHER_TIMEOUT"), 120)

backend/handler/filesystem/roms_handler.py

@@ -47,8 +47,8 @@ from utils.archives import (
     ArchiveReadError,
     detect_mime_type,
     extract_chd_hash,
+    hash_largest_7z_member,
     is_chd_file,
-    process_7z_file,
     read_7z_archive_files,
     read_basic_file,
     read_bz2_file,
@@ -1027,10 +1027,20 @@ class FSRomsHandler(FSHandler):
                     update_hashes(chunk)
 
             elif extension == ".7z" or file_type == "application/x-7z-compressed":
-                process_7z_file(
-                    file_path=file_path,
-                    fn_hash_update=update_hashes,
+                # A failed extraction may have fed part of the member already,
+                # so the raw-archive fallback starts from the hashers' prior state.
+                snapshot = (
+                    crc_c,
+                    rom_crc_c,
+                    md5_h.copy(),
+                    sha1_h.copy(),
+                    rom_md5_h.copy() if accumulate else rom_md5_h,
+                    rom_sha1_h.copy() if accumulate else rom_sha1_h,
                 )
+                if not hash_largest_7z_member(file_path, update_hashes):
+                    crc_c, rom_crc_c, md5_h, sha1_h, rom_md5_h, rom_sha1_h = snapshot
+                    for chunk in read_basic_file(file_path):
+                        update_hashes(chunk)
 
             elif extension == ".bz2" or file_type == "application/x-bzip2":
                 for chunk in read_bz2_file(file_path):

backend/tests/handler/filesystem/test_roms_handler.py

@@ -1,3 +1,4 @@
+import binascii
 import hashlib
 import os
 import shutil
@@ -1352,6 +1353,44 @@ class TestFSRomsHandler:
             if test_file.exists():
                 test_file.unlink()
 
+    def test_failed_7z_extraction_hashes_only_the_raw_archive(
+        self, handler: FSRomsHandler, tmp_path
+    ):
+        """Bytes streamed before a 7z extraction fails must not leak into the
+        raw-archive fallback hash, nor into the ROM-level accumulators."""
+        archive_bytes = b"7z\xbc\xaf\x27\x1c raw archive bytes"
+        archive = tmp_path / "game.7z"
+        archive.write_bytes(archive_bytes)
+
+        def _partial_then_fail(_path, fn_hash_update):
+            fn_hash_update(b"partial inner member bytes")
+            return False
+
+        rom_md5_h = hashlib.md5(b"earlier file", usedforsecurity=False)
+        rom_sha1_h = hashlib.sha1(b"earlier file", usedforsecurity=False)
+        rom_crc_c = binascii.crc32(b"earlier file")
+
+        with patch(
+            "handler.filesystem.roms_handler.hash_largest_7z_member",
+            side_effect=_partial_then_fail,
+        ):
+            crc_c, rom_crc_c, md5_h, rom_md5_h, sha1_h, rom_sha1_h = (
+                handler._calculate_rom_hashes(archive, rom_crc_c, rom_md5_h, rom_sha1_h)
+            )
+
+        assert crc_c == binascii.crc32(archive_bytes)
+        assert md5_h.hexdigest() == hashlib.md5(archive_bytes).hexdigest()
+        assert sha1_h.hexdigest() == hashlib.sha1(archive_bytes).hexdigest()
+        assert rom_crc_c == binascii.crc32(b"earlier file" + archive_bytes)
+        assert (
+            rom_md5_h.hexdigest()
+            == hashlib.md5(b"earlier file" + archive_bytes).hexdigest()
+        )
+        assert (
+            rom_sha1_h.hexdigest()
+            == hashlib.sha1(b"earlier file" + archive_bytes).hexdigest()
+        )
+
     async def test_compressed_file_handling(self, handler: FSRomsHandler):
         """Test handling of compressed ROM files"""
         # Test with the ZIP file

backend/tests/utils/test_archives.py

@@ -155,7 +155,7 @@ def test_largest_member_hashing_terminates_switches_before_the_member():
         patch.object(subprocess, "run", return_value=listing),
         patch.object(subprocess, "Popen", popen),
     ):
-        assert archives._process_largest_7z_member(Path("/fake/game.7z"), MagicMock())
+        assert archives.hash_largest_7z_member(Path("/fake/game.7z"), MagicMock())
 
     command = popen.call_args[0][0]
     assert command[-2:] == ["--", "-x"]

backend/utils/archives.py

@@ -132,13 +132,14 @@ def read_gz_file(file_path: Path) -> Iterator[bytes]:
     return read_tar_file(file_path, "r:gz")
 
 
-def _process_largest_7z_member(
+def hash_largest_7z_member(
     file_path: Path,
     fn_hash_update: Callable[[bytes | bytearray], None],
 ) -> bool:
     """Stream the largest member of a 7z archive through `fn_hash_update`.
 
-    Returns True on success, False if listing/extraction fails or times out.
+    Returns True on success, False if listing/extraction fails or times out,
+    in which case `fn_hash_update` may already have seen part of the member.
     """
     try:
         result = subprocess.run(
@@ -216,15 +217,6 @@ def _process_largest_7z_member(
         return False
 
 
-def process_7z_file(
-    file_path: Path,
-    fn_hash_update: Callable[[bytes | bytearray], None],
-) -> None:
-    if not _process_largest_7z_member(file_path, fn_hash_update):
-        for chunk in read_basic_file(file_path):
-            fn_hash_update(chunk)
-
-
 def read_bz2_file(file_path: Path) -> Iterator[bytes]:
     try:
         with bz2.BZ2File(file_path, "rb") as f:

docs/BACKEND_ARCHITECTURE.md

@@ -1921,7 +1921,7 @@ Falls back to `FakeRedis` in test mode.
 | `SCHEDULED_RESCAN_CRON`                | `0 3 * * *` | Rescan schedule                 |
 | `ENABLE_RESCAN_ON_FILESYSTEM_CHANGE`   | `false`     | Watch for file changes          |
 | `RESCAN_ON_FILESYSTEM_CHANGE_DELAY`    | `5`         | Debounce delay (minutes)        |
-| `SEVEN_ZIP_TIMEOUT`                    |             | Timeout for 7-Zip extraction    |
+| `SEVEN_ZIP_TIMEOUT`                    | `180`       | Timeout for 7-Zip extraction    |
 | `REFRESH_RETROACHIEVEMENTS_CACHE_DAYS` |             | RA cache TTL (days)             |
 
 #### Device Sync

env.template

@@ -101,7 +101,7 @@ SCAN_TIMEOUT=14400  # Timeout for background scan/rescan tasks in seconds
 SCAN_WORKERS=4  # How many ROMs a scan processes at once
 TASK_TIMEOUT=300  # Timeout for other background tasks in seconds
 TASK_RESULT_TTL=86400  # How long to keep task results in Valkey in seconds
-SEVEN_ZIP_TIMEOUT=60  # Timeout for 7-Zip operations in seconds
+SEVEN_ZIP_TIMEOUT=180  # Timeout for 7-Zip operations in seconds
 ENABLE_RESCAN_ON_FILESYSTEM_CHANGE=false  # Re-scan the library automatically when the filesystem changes
 RESCAN_ON_FILESYSTEM_CHANGE_DELAY=5  # Delay in minutes before re-scanning after a filesystem change
 ENABLE_SCHEDULED_RESCAN=false  # Enable scheduled library re-scans