Fix the follow-up review's findings

- APK reader: read members with a bounded read, since ZipFile.read inflates
  a member fully before trimming it to a forged declared size; the reference
  walk counts every entry it examines, dead ends and cycles included.
- Titles: a path that exists under the root wins over stripping the archive
  prefix; the prefix is taken before a linked folder is staged elsewhere
  (another drive on Windows); the install dialog loads a fresh title list
  first and says so if it couldn't check.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
saphidandClaude Opus 5.5 committed 2026-09-26 22:44:07 +10:00
1 parent 0530a6d045
commit dfacf43e55
5 files changed
+85 -15

No files matched your search

+15 -6
View File
@@ -18,6 +18,7 @@ MAX_MANIFEST = 16 * 1024**2
MAX_ARSC = 128 * 1024**2 # real ones are a few MB; the largest apps' tens of MB
MAX_ICON = 8 * 1024**2
MAX_VALUES = 256 # resolved values per reference, across all its hops
MAX_STEPS = 4096 # entries examined per reference, dead ends and cycles included
class ApkError(Exception):
@@ -135,18 +136,20 @@ class Resources:
resid = (pid << 24) | (tid << 16) | index
self.entries.setdefault(resid, []).append((language, density, dtype, value))
def values(self, resid, depth=0, seen=frozenset()):
"""[(language, density, type, data)] with references followed, at most
MAX_VALUES of them and never round a cycle."""
def values(self, resid, depth=0, seen=frozenset(), steps=None):
"""[(language, density, type, data)] with references followed: never round a
cycle, at most MAX_VALUES results and MAX_STEPS entries examined in all."""
steps = steps if steps is not None else [MAX_STEPS]
out = []
seen = seen | {resid}
for lang, dens, dtype, value in self.entries.get(resid, []):
if len(out) >= MAX_VALUES:
steps[0] -= 1
if steps[0] < 0 or len(out) >= MAX_VALUES:
break
if dtype == T_REF and depth < 5:
if value not in seen:
out += [(lang or l2, dens or d2, t2, v2)
for l2, d2, t2, v2 in self.values(value, depth + 1, seen)]
for l2, d2, t2, v2 in self.values(value, depth + 1, seen, steps)]
else:
out.append((lang, dens, dtype, value))
return out[:MAX_VALUES]
@@ -184,10 +187,16 @@ def _icons(attr, res):
def _read(z, name, limit):
"""A member's bytes, inflating at most limit + 1 of them whatever its header claims
(ZipFile.read inflates everything first, then trims to the declared size)."""
size = z.getinfo(name).file_size
if size > limit:
raise ApkError(f'{name} in the APK is {size / 1024**2:.0f} MB, more than a real one ({limit // 1024**2} MB)')
return z.read(name)
with z.open(name) as f:
data = f.read(limit + 1)
if len(data) > limit:
raise ApkError(f'{name} in the APK is larger than a real one ({limit // 1024**2} MB)')
return data
def apk_info(path):
+15 -7
View File
@@ -321,6 +321,13 @@ def _stage_folder(src, dest):
return dest
def _below(top, root):
"""The folders _unwrap stepped through from top to root, as 'a/b', or ''. Taken
before staging moves root elsewhere (possibly another drive on Windows)."""
rel = os.path.relpath(root, os.path.realpath(top)).replace(os.sep, '/')
return '' if rel == '.' else rel
def _unwrap(root):
"""Step into a single top-level folder, the usual shape of a zipped build.
@@ -424,8 +431,8 @@ def inspect(path, name=None):
work = None
try:
if os.path.isdir(path):
base = path
root = _unwrap(path)
unwrapped = _below(path, root)
if _has_links(root):
# scp -r follows links, so a link out of the folder could upload
# anything; copy the folder with its links made safe first.
@@ -434,24 +441,22 @@ def inspect(path, name=None):
elif path.lower().endswith('.zip'):
work = tempfile.mkdtemp(prefix=f'{TMP_PREFIX}{os.getpid()}-')
extract_zip(path, work)
base = work
root = _unwrap(work)
unwrapped = _below(work, root)
elif classify(path):
# A single executable is uploaded on its own; don't copy a whole Downloads folder.
work = tempfile.mkdtemp(prefix=f'{TMP_PREFIX}{os.getpid()}-')
shutil.copy2(path, os.path.join(work, os.path.basename(path)))
base = root = work
root, unwrapped = work, ''
else:
raise FrameError(f'{os.path.basename(path)} is not a .zip, a folder or a program')
title = name or display_name(os.path.basename(path.rstrip('/\\')))
found = candidates(root, title)
if not found:
raise FrameError(f'no Linux or Windows program found in {os.path.basename(path)}')
# An exe path may be given as it is in the archive, above the folder _unwrap stepped into.
unwrapped = os.path.relpath(root, os.path.realpath(base)).replace(os.sep, '/')
plan = {'source': os.path.basename(path.rstrip('/\\')), 'name': title, 'id': title_id(title),
'root': root, 'work': work, 'candidates': found,
'unwrapped': '' if unwrapped == '.' else unwrapped,
'unwrapped': unwrapped,
'size': _tree_size(root), 'warnings': []}
_choose(plan, found[0]['path'])
return plan
@@ -474,8 +479,11 @@ def _tree_size(root):
def _choose(plan, rel, runtime=None):
"""Set plan's launch target (a path relative to root) and its runtime."""
rel = rel.replace('\\', '/')
# A manifest may name the program as it is in the archive, above the folder
# _unwrap stepped into. A path that works as it is always wins.
prefix = plan.get('unwrapped') and plan['unwrapped'] + '/'
if prefix and rel.startswith(prefix):
if (prefix and rel.startswith(prefix) and not any(c['path'] == rel for c in plan['candidates'])
and not os.path.isfile(os.path.join(plan['root'], *rel.split('/')))):
rel = rel[len(prefix):]
target = next((c for c in plan['candidates'] if c['path'] == rel), None)
if target is None:
+4 -2
View File
@@ -1200,6 +1200,7 @@ async function sideloadOne(file, path, isDir = false) {
await act(`Copy ${file.name} to ~/Downloads`, () => upload(file, "push"));
return;
}
await loadTitles(); // fresh, so the dialog can say whether this replaces an installed title
const choice = await confirmTitle(r.plan, canPush);
if (choice === "push") {
api("/api/titles", { action: "discard", token: r.token }).catch(() => {});
@@ -1217,6 +1218,7 @@ function confirmTitle(plan, canPush) {
$("titleName").value = plan.name;
const idNote = () => { const id = titleId($("titleName").value);
$("titleIdNote").textContent = !id ? "Needs some letters or digits"
: !installedTitles ? `Shows in Steam as ${id}. Couldn't check whether it's installed already; if it is, this replaces it`
: installedTitles.has(id.toLowerCase()) ? `Replaces the installed ${id}, and everything in its folder on the Frame`
: `Shows in Steam as ${id}`; };
$("titleName").oninput = idNote; idNote();
@@ -1274,11 +1276,11 @@ async function installTitle(token, choice) {
}
} finally { $("prog").style.display = "none"; loadTitles(); }
}
let installedTitles = new Set(); // lower-case ids, so the install dialog can warn before replacing one
let installedTitles = null; // lower-case ids, so the install dialog can warn before replacing one; null: unknown
async function loadTitles() {
let list;
try { list = (await api("/api/titles")).titles; }
catch (e) { $("titleList").innerHTML = `<div class="sub">${esc(e.message)}</div>`; return; }
catch (e) { installedTitles = null; $("titleList").innerHTML = `<div class="sub">${esc(e.message)}</div>`; return; }
installedTitles = new Set(list.map(t => String(t.id).toLowerCase()));
$("titleCount").textContent = list.length ? `${list.length}` : "";
$("titleList").innerHTML = list.length ? list.map(t => `