From a1fa4ce14007483e3de9c406d9e13c27a480ec4a Mon Sep 17 00:00:00 2001 From: saphid <4596216+saphid@users.noreply.github.com> Date: Mon, 28 Sep 2026 15:15:11 +1000 Subject: [PATCH] Fix the cross-provider review's findings on APK alternatives One malformed or unreachable repo no longer hides the others; skip bad index entries; a refreshed raw index outdates its reduced copy; style the dialog like the others; validate package ids with PKG_RE. Co-Authored-By: Claude Opus 5.5 (1M context) --- tests/test_frame_apk_versions.py | 28 ++++++++++++++++++++++++++++ ui/frame_apk_versions.py | 2 +- ui/frame_catalog.py | 13 ++++++++++--- ui/index.html | 8 ++++---- ui/server.py | 2 +- 5 files changed, 44 insertions(+), 9 deletions(-) diff --git a/tests/test_frame_apk_versions.py b/tests/test_frame_apk_versions.py index 5d76c64..8cf7701 100644 --- a/tests/test_frame_apk_versions.py +++ b/tests/test_frame_apk_versions.py @@ -4,6 +4,7 @@ import json import os import sys import tempfile +import time import unittest from concurrent.futures import ThreadPoolExecutor from unittest.mock import patch @@ -95,6 +96,33 @@ class VersionsTest(unittest.TestCase): fetch.assert_called_once() self.assertTrue(os.path.exists(raw)) + def test_malformed_entries_are_skipped(self): + raw = os.path.join(self.tmp.name, 'odd.json') + with open(raw, 'w') as f: + json.dump({'packages': {'a.b': {'versions': {'x': {'manifest': {}}, 'y': None, 'z': build(4)}}, + 'c.d': {'versions': None}, 'e.f': []}}, f) + self.assertEqual([v['version_code'] for v in frame_catalog._reduce_index(raw)['a.b']], [4]) + + def test_one_failing_repo_keeps_the_others(self): + real = frame_catalog.load_index + def load(repo, cached_only=False): + if 'izzy' in repo: + raise KeyError('file') + return real(repo, cached_only=cached_only) + with patch.object(frame_catalog, 'load_index', side_effect=load): + result = versions.alternatives('org.example.app') + self.assertEqual([v['version_code'] for v in result['versions']], [3, 2, 1]) + self.assertEqual(len(result['errors']), 1) + + def test_newer_raw_index_outdates_reduced_copy(self): + repo = versions.REPOS[0][1] + frame_catalog.load_index(repo) + raw = os.path.join(self.tmp.name, 'data', 'index-v2.json') + with open(raw, 'w') as f: + json.dump({'packages': {'org.example.app': {'versions': {'x': build(8)}}}}, f) + os.utime(raw, ns=(time.time_ns() + 10**9,) * 2) + self.assertEqual(frame_catalog.load_index(repo)['org.example.app'][0]['version_code'], 8) + def test_concurrent_requests_share_download(self): raw = os.path.join(self.tmp.name, 'data', 'index-v2.json') os.remove(raw) diff --git a/ui/frame_apk_versions.py b/ui/frame_apk_versions.py index 81f3102..e5964f4 100644 --- a/ui/frame_apk_versions.py +++ b/ui/frame_apk_versions.py @@ -50,7 +50,7 @@ def _versions(package, cached_only=False): for source, repo in REPOS: try: index = frame_catalog.load_index(repo, cached_only=cached_only) - except (OSError, ValueError) as e: + except Exception as e: # one bad repo (dropped download, odd index) mustn't hide the others errors.append(f'Could not check {source}: {e}') continue for v in index.get(package, []): diff --git a/ui/frame_catalog.py b/ui/frame_catalog.py index 8a99513..d5b808e 100644 --- a/ui/frame_catalog.py +++ b/ui/frame_catalog.py @@ -92,8 +92,13 @@ def _reduce_index(path): continue found = True for package in reader.members(): - records = [] - for v in reader.value().get('versions', {}).values(): + records, entry = [], reader.value() + versions = entry.get('versions') if isinstance(entry, dict) else None + for v in (versions.values() if isinstance(versions, dict) else ()): + # Skip malformed entries rather than losing the whole repo. + if not (isinstance(v, dict) and isinstance(v.get('manifest'), dict) + and isinstance(v.get('file'), dict) and v['file'].get('name')): + continue if not installable(v): continue m, file = v['manifest'], v['file'] @@ -121,7 +126,9 @@ def load_index(repo, cached_only=False): path = raw + '.installable-v1' with _index_lock: mtime = os.stat(path).st_mtime_ns if os.path.exists(path) else None - if mtime is not None and (cached_only or time.time() - mtime / 1e9 < 86400): + # A newer raw index (the catalogue script refreshed it) outdates the reduced copy. + newer_raw = mtime is not None and os.path.exists(raw) and os.stat(raw).st_mtime_ns > mtime + if mtime is not None and (cached_only or (time.time() - mtime / 1e9 < 86400 and not newer_raw)): cached = _indexes.get(path) if cached is None or cached[0] != mtime: with open(path) as f: diff --git a/ui/index.html b/ui/index.html index 3ebf51d..926e0e1 100644 --- a/ui/index.html +++ b/ui/index.html @@ -221,10 +221,10 @@ .and-grid { display: grid; grid-template-columns: minmax(0, 1fr) minmax(0, 2fr); gap: 22px; align-items: start; } .and-col { display: grid; gap: 22px; align-content: start; } .rep-item .s { white-space: normal; } - #repDlg, #titleDlg, #wiDlg { background: #1e2329; color: var(--text); border: 1px solid rgba(255,255,255,.1); border-radius: 4px; + #repDlg, #titleDlg, #wiDlg, #apkAltDlg { background: #1e2329; color: var(--text); border: 1px solid rgba(255,255,255,.1); border-radius: 4px; padding: 22px; width: min(560px, 92vw); box-shadow: 0 20px 60px rgba(0,0,0,.6); } - #repDlg::backdrop, #titleDlg::backdrop, #wiDlg::backdrop { background: rgba(0,0,0,.55); } - #repDlg h2, #titleDlg h2, #wiDlg h2 { margin: 0 0 14px; font-size: 15px; letter-spacing: 1.5px; text-transform: uppercase; color: var(--bright); } + #repDlg::backdrop, #titleDlg::backdrop, #wiDlg::backdrop, #apkAltDlg::backdrop { background: rgba(0,0,0,.55); } + #repDlg h2, #titleDlg h2, #wiDlg h2, #apkAltDlg h2 { margin: 0 0 14px; font-size: 15px; letter-spacing: 1.5px; text-transform: uppercase; color: var(--bright); } #repForm label, #titleForm label { display: block; font-size: 12.5px; color: var(--muted); margin-top: 10px; } #repForm label input[type=text], #repForm textarea, #titleForm label input, #titleForm label select { margin-top: 5px; } #titleForm select { width: 100%; background: rgba(0,0,0,.28); color: var(--text); border: 1px solid transparent; @@ -555,7 +555,7 @@

Try another APK version

-
+

diff --git a/ui/server.py b/ui/server.py index 6ecdfa1..9be93bc 100755 --- a/ui/server.py +++ b/ui/server.py @@ -448,7 +448,7 @@ def open_thing(body): def apk_versions(query): args = parse_qs(query, keep_blank_values=True) packages, codes = args.get('package', []), args.get('code', []) - if len(packages) != 1 or not re.fullmatch(r'[A-Za-z][A-Za-z0-9_]*(?:\.[A-Za-z][A-Za-z0-9_]*)+', packages[0]): + if len(packages) != 1 or not frame_android.PKG_RE.match(packages[0]): raise Failure('invalid Android package id', 400) if codes and (len(codes) != 1 or not re.fullmatch(r'[0-9]{1,19}', codes[0])): raise Failure('invalid version code', 400)