From 1c280b7b3f234108a1a19d951bc1e45ed2189e1f Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Sat, 17 Oct 2026 23:55:49 +0200 Subject: [PATCH] refactor: hash each zip member once per run --- scripts/ziptools.py | 46 ++++++++++++++++++++++++++------- tests/test_audit_regressions.py | 27 +++++++++++++++++++ 2 files changed, 63 insertions(+), 10 deletions(-) diff --git a/scripts/ziptools.py b/scripts/ziptools.py index 6acb14c1..a14d6f0f 100644 --- a/scripts/ziptools.py +++ b/scripts/ziptools.py @@ -18,26 +18,52 @@ from hashing import md5sum def check_inside_zip(container: str, file_name: str, expected_md5: str) -> str: - """Check a ROM inside a ZIP -replicates Batocera checkInsideZip(). + """Check a ROM inside a ZIP, as Batocera's checkInsideZip() does. Returns "ok", "untested", "not_in_zip", or "error". """ + try: + info = os.stat(container) + except OSError: + return "error" + found = _member_md5( + container, info.st_size, info.st_mtime_ns, file_name.casefold(), + expected_md5 != "", + ) + if found in ("not_in_zip", "error"): + return found + if expected_md5 == "": + return "ok" + return "ok" if found == expected_md5 else "untested" + + +@functools.lru_cache(maxsize=8192) +def _member_md5( + container: str, size: int, mtime_ns: int, folded: str, hashed: bool +) -> str: + """The md5 of the member named `folded` (case-folded), or why there is none. + + One archive member is asked about by every md5 a platform accepts for it + and by every step that checks it: `verify --all` decompressed 355 + members 4 788 times. Keyed by the archive's size and mtime, so a + rewritten archive is read again. + """ try: with zipfile.ZipFile(container) as archive: for fname in archive.namelist(): - if fname.casefold() == file_name.casefold(): - info = archive.getinfo(fname) - if info.file_size > 512 * 1024 * 1024: - return "error" - if expected_md5 == "": - return "ok" - with archive.open(fname) as entry: - actual = md5sum(entry) - return "ok" if actual == expected_md5 else "untested" + if fname.casefold() != folded: + continue + if archive.getinfo(fname).file_size > 512 * 1024 * 1024: + return "error" + if not hashed: + return "present" + with archive.open(fname) as entry: + return md5sum(entry) return "not_in_zip" except (zipfile.BadZipFile, OSError, KeyError): return "error" + @functools.lru_cache(maxsize=4096) def _member_crcs(path: str, mtime: float) -> frozenset[str]: try: diff --git a/tests/test_audit_regressions.py b/tests/test_audit_regressions.py index 7d325d6f..07a58a0d 100644 --- a/tests/test_audit_regressions.py +++ b/tests/test_audit_regressions.py @@ -3671,5 +3671,32 @@ class AManifestHashesAFileOnce(unittest.TestCase): self.assertEqual(calls[0], frozenset({"sha1", "sha256"})) + +class AZipMemberIsHashedOnce(unittest.TestCase): + """check_inside_zip decompressed a member for every md5 candidate and + every step: 4 788 times for 355 members in verify --all.""" + + def test_repeated_questions_read_the_archive_once(self): + from scripts import ziptools + + with tempfile.TemporaryDirectory(dir=TMP_ROOT) as tmp: + archive = Path(tmp, "set.zip") + with zipfile.ZipFile(archive, "w") as zf: + zf.writestr("ROM.bin", b"abc") + good = hashlib.md5(b"abc").hexdigest() + opened = [] + real = ziptools.zipfile.ZipFile + + def counting(*args, **kwargs): + opened.append(args[0]) + return real(*args, **kwargs) + + with mock.patch.object(ziptools.zipfile, "ZipFile", counting): + answers = [ziptools.check_inside_zip(str(archive), "rom.bin", md5) + for md5 in (good, "0" * 32, good)] + self.assertEqual(answers, ["ok", "untested", "ok"]) + self.assertEqual(len(opened), 1) + + if __name__ == "__main__": unittest.main()