diff --git a/CHANGELOG.md b/CHANGELOG.md index 637c7cd..9f8ccfb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -235,6 +235,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 engine (`app.js`, `highway.js`, `playSong`, `showScreen`, the capability registry). ### Fixed +- **GP8 asset resolution honours the directory the registry named.** + `` is matched on filename stem so a format variant of the + same recording can win (an `.ogg` beside the declared `.mp3` is copied out + losslessly instead of transcoded) — but the search was not restricted to the + declared directory, so an unrelated file elsewhere in the archive that merely + shared the stem could stand in for the declared asset. That is the exact + substitution the registry lookup exists to prevent. Candidates are now + confined to the registry path's own directory; a genuinely absent asset + falls through as documented. - **Guitar Pro 8: the right backing track is extracted when a file carries more than one.** `BackingTrack/AssetId` is a key into the GPIF's `` registry — `` names the exact path inside the archive — but it diff --git a/lib/gp8_audio_sync.py b/lib/gp8_audio_sync.py index 5a9aeb1..d4d14e6 100644 --- a/lib/gp8_audio_sync.py +++ b/lib/gp8_audio_sync.py @@ -165,12 +165,20 @@ def _resolve_audio_asset(zf, root=None) -> tuple[str, str | None]: # stale/edited entry must fall through, not resolve to nothing. registry_path = _asset_path_from_registry(root, declared) if registry_path: + # Matched on STEM, not the whole path, so a format variant of the + # same recording can win (see _prefer_ogg) — but constrained to the + # directory the registry actually named. Without that constraint an + # unrelated file that merely shares the stem could stand in for the + # declared asset, which is the failure the registry lookup exists + # to prevent. + declared_path = Path(registry_path) same_stem = [ n for n in audio_files - if Path(n).stem == Path(registry_path).stem + if Path(n).stem == declared_path.stem + and Path(n).parent == declared_path.parent ] if same_stem: - return Path(registry_path).stem, _prefer_ogg(same_stem) + return declared_path.stem, _prefer_ogg(same_stem) _log.warning( 'gp8_audio_sync: AssetId %r maps to %r, which is not an audio ' 'asset in the archive; falling back', diff --git a/tests/test_gp_audio_sync.py b/tests/test_gp_audio_sync.py index 20c1c01..a81fe11 100644 --- a/tests/test_gp_audio_sync.py +++ b/tests/test_gp_audio_sync.py @@ -375,3 +375,51 @@ def test_legacy_stem_match_still_works_without_a_registry(): stem, path = _resolve_audio_asset(zf) assert stem == "abc-123" assert path == "Content/Assets/abc-123.ogg" + + +def test_a_same_stem_file_in_another_directory_cannot_stand_in(): + """The registry names a PATH, not just a name. + + Resolution matches on stem so a format variant of the same recording can + win, but an unrelated file that merely shares the stem must not satisfy + the declaration — that substitution is what the registry lookup exists to + prevent. The declared asset is genuinely absent here, so the right answer + is the documented fall-through, not the decoy. + + ZIP order matters to this test: `real.ogg` is written FIRST so the + fall-through target differs from the decoy. Otherwise both the fixed and + unfixed code return the same file and the test proves nothing. + """ + buf = io.BytesIO() + with zipfile.ZipFile(buf, "w") as zf: + zf.writestr("Content/score.gpif", _gpif_bytes( + "0", {"0": "Content/Audio/track.ogg"})) + zf.writestr("Content/Assets/real.ogg", b"fake") # fall-through target + zf.writestr("Content/Assets/track.ogg", b"decoy") # shares the stem only + buf.seek(0) + _, path = _resolve_audio_asset(zipfile.ZipFile(buf)) + assert path == "Content/Assets/real.ogg", ( + "a same-stem file in a directory the registry never named must not " + "satisfy the declaration" + ) + + +def test_the_declared_directory_still_resolves_its_own_format_variants(): + """The directory constraint must not cost us the OGG preference. + + The shallower decoy is written FIRST, so unfixed code (which searches + every directory) picks it and this test fails. + """ + buf = io.BytesIO() + with zipfile.ZipFile(buf, "w") as zf: + zf.writestr("Content/score.gpif", _gpif_bytes( + "0", {"0": "Content/Assets/nested/take.mp3"})) + zf.writestr("Content/Assets/take.ogg", b"decoy-one-level-up") + zf.writestr("Content/Assets/nested/take.mp3", b"declared") + zf.writestr("Content/Assets/nested/take.ogg", b"same-take-lossless") + buf.seek(0) + stem, path = _resolve_audio_asset(zipfile.ZipFile(buf)) + assert path == "Content/Assets/nested/take.ogg", ( + "the OGG variant in the DECLARED directory wins over a shallower decoy" + ) + assert stem == "take"