From a269f9b677157b017bba3a26b97cde8f3785a768 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Wed, 12 Aug 2026 17:04:29 +0200 Subject: [PATCH] refactor: lift two preambles out of the verifiers Resolving the profiles a run names, which refuses an alias or a launcher because neither has requirements of its own, and the structural checks on an archive that hold whatever it contains: a duplicate entry, an absolute path, a traversal, a zero-byte member. Complexity 58 to 50 and 63 to 51. Extracting the second surfaced a Counter it had been reading from the enclosing module, which pyflakes caught before the tests did. --- scripts/packverify.py | 56 +++++++++++++++++++++++++++---------------- scripts/verify.py | 47 ++++++++++++++++++++++++------------ 2 files changed, 68 insertions(+), 35 deletions(-) diff --git a/scripts/packverify.py b/scripts/packverify.py index 768d1748..97e99980 100644 --- a/scripts/packverify.py +++ b/scripts/packverify.py @@ -6,6 +6,8 @@ coverage report that disagree describe different collections.""" from __future__ import annotations +from collections import Counter + from auto_fetch import DEFAULT_BIOS_DIR from artifacts import _build_timestamp from packextras import _collect_emulator_extras @@ -405,6 +407,39 @@ def _intentional_hash_exclusion( return False return True +def _structural_errors(zf, zip_set: set) -> list[str]: + """Faults in the archive itself, before any file is looked up. + + A duplicate entry, an absolute path or a traversal makes the pack + unsafe to extract whatever it contains, and a zero-byte member is a + file the user will believe they have. The markers Dolphin ships for + its graphics mods are legitimately empty. + """ + errors: list[str] = [] + # Structural checks + dupes = sum(1 for c in Counter(zf.namelist()).values() if c > 1) + if dupes: + errors.append(f"{dupes} duplicate entries") + for n in zip_set: + if "//" in n: + errors.append(f"double slash: {n}") + if n.startswith("/"): + errors.append(f"absolute path: {n}") + if ".." in n: + errors.append(f"path traversal: {n}") + + # Zero-byte check (exclude Dolphin GraphicMods markers) + for info in zf.infolist(): + if info.file_size == 0 and not info.is_dir(): + if "GraphicMods" not in info.filename and info.filename not in ( + "manifest.json", + "README.txt", + ): + errors.append(f"zero-byte: {info.filename}") + + return errors + + def verify_pack_against_platform( zip_path: str, platform_name: str, @@ -468,26 +503,7 @@ def verify_pack_against_platform( if n not in ("README.txt", "manifest.json") and not n.endswith("/") ) - # Structural checks - dupes = sum(1 for c in Counter(zf.namelist()).values() if c > 1) - if dupes: - errors.append(f"{dupes} duplicate entries") - for n in zip_set: - if "//" in n: - errors.append(f"double slash: {n}") - if n.startswith("/"): - errors.append(f"absolute path: {n}") - if ".." in n: - errors.append(f"path traversal: {n}") - - # Zero-byte check (exclude Dolphin GraphicMods markers) - for info in zf.infolist(): - if info.file_size == 0 and not info.is_dir(): - if "GraphicMods" not in info.filename and info.filename not in ( - "manifest.json", - "README.txt", - ): - errors.append(f"zero-byte: {info.filename}") + errors.extend(_structural_errors(zf, zip_set)) # 1. Baseline file presence + native hash check verification_mode = config.get("verification_mode", "existence") diff --git a/scripts/verify.py b/scripts/verify.py index 1b0a246a..86ff0cbf 100644 --- a/scripts/verify.py +++ b/scripts/verify.py @@ -1249,24 +1249,18 @@ def _effective_validation_label(details: list[dict], validation_index: dict) -> return "+".join(sorted(all_checks)) -def verify_emulator( +def _select_profiles( profile_names: list[str], - emulators_dir: str, - db: dict, - standalone: bool = False, - regions: list[str] | None = None, -) -> dict: - """Verify files for specific emulator profiles. + all_profiles: dict, + standalone: bool, +) -> list[tuple[str, dict]]: + """Resolve the named profiles, refusing the ones that answer for others. - A region priority list narrows the report the same way a pack built with the - same list would be narrowed. One group per profile, as in generate_pack. + An alias is the same binary under another name and a launcher only + starts an emulator, so neither has requirements of its own: naming + one is a question about the wrong profile, and the answer says which + one to ask instead. """ - load_emulator_profiles(emulators_dir) - zip_contents = build_zip_contents_index(db) - - # Also load aliases for redirect messages - all_profiles = load_emulator_profiles(emulators_dir, skip_aliases=False) - # Resolve profile names, reject alias/launcher selected: list[tuple[str, dict]] = [] for name in profile_names: @@ -1303,6 +1297,29 @@ def verify_emulator( sys.exit(1) selected.append((name, p)) + return selected + + +def verify_emulator( + profile_names: list[str], + emulators_dir: str, + db: dict, + standalone: bool = False, + regions: list[str] | None = None, +) -> dict: + """Verify files for specific emulator profiles. + + A region priority list narrows the report the same way a pack built with the + same list would be narrowed. One group per profile, as in generate_pack. + """ + load_emulator_profiles(emulators_dir) + zip_contents = build_zip_contents_index(db) + + # Also load aliases for redirect messages + all_profiles = load_emulator_profiles(emulators_dir, skip_aliases=False) + + selected = _select_profiles(profile_names, all_profiles, standalone) + # Build validation index from selected profiles only selected_profiles = {n: p for n, p in selected} validation_index = _build_validation_index(selected_profiles)