Fix episode position logging/resume: NaN savePos, clobber-on-boot, unvalidated history writes
- app.js: pause/pagehide listeners passed the Event into savePos(floor), making pos NaN -> serialized as null -> every pause/unload clobbered the saved position. Wrap in arrows; pagehide now posts keepalive so the final save survives page unload - app.js: play() re-fetches history on a miss (boot fetch swallows errors) and only bumps an existing row instead of posting pos:0 at start-of-play - app.py: POST /api/history requires slug_id (was stored as "None"), validates/clamps pos; upsert keeps the old payload when the incoming pos is null so stale clients can't corrupt a row - tests: +3 API tests (slug required, pos clamping, null-pos keeps saved), +1 end-to-end browser test (resume ~400s with a failed boot fetch); lockout test now uses a throwaway username so it can't lock out users the browser tests log in with; hardened the flaky autoplay test
This commit is contained in:
1 parent
7444e29592
commit
d90b1af8d6
4 files changed
+156
-23
No files matched your search
+23
-2
@@ -10,6 +10,7 @@ import os
|
|||||||
import subprocess
|
import subprocess
|
||||||
import threading
|
import threading
|
||||||
import time
|
import time
|
||||||
|
from math import isfinite
|
||||||
from collections import Counter, OrderedDict
|
from collections import Counter, OrderedDict
|
||||||
from datetime import datetime
|
from datetime import datetime
|
||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
@@ -271,6 +272,25 @@ def api_history_post(entry: dict = Body(...), request: Request = None):
|
|||||||
entry["ep_id"] = int(entry["ep_id"])
|
entry["ep_id"] = int(entry["ep_id"])
|
||||||
except (TypeError, ValueError):
|
except (TypeError, ValueError):
|
||||||
raise HTTPException(422, "ep_id must be an integer")
|
raise HTTPException(422, "ep_id must be an integer")
|
||||||
|
slug = str(entry.get("slug_id") or "").strip()
|
||||||
|
if not slug:
|
||||||
|
raise HTTPException(422, "slug_id required") # a missing slug used to store a "None" row
|
||||||
|
entry["slug_id"] = slug
|
||||||
|
if entry.get("pos") is not None:
|
||||||
|
try:
|
||||||
|
pos = max(0.0, float(entry["pos"]))
|
||||||
|
except (TypeError, ValueError):
|
||||||
|
raise HTTPException(422, "pos must be a number")
|
||||||
|
if not isfinite(pos):
|
||||||
|
raise HTTPException(422, "pos not finite")
|
||||||
|
if entry.get("dur"): # clamp above-duration junk out of the progress UI
|
||||||
|
try:
|
||||||
|
pos = min(pos, float(entry["dur"]))
|
||||||
|
except (TypeError, ValueError):
|
||||||
|
pass
|
||||||
|
entry["pos"] = pos
|
||||||
|
# a missing/null pos stays null: the upsert keeps the existing row's
|
||||||
|
# payload, so a stale client can never clobber the saved position
|
||||||
entry["t"] = time.time()
|
entry["t"] = time.time()
|
||||||
u = request.state.user
|
u = request.state.user
|
||||||
c = db.connect()
|
c = db.connect()
|
||||||
@@ -278,8 +298,9 @@ def api_history_post(entry: dict = Body(...), request: Request = None):
|
|||||||
# read-modify-write (and no lost-update race between the read and write)
|
# read-modify-write (and no lost-update race between the read and write)
|
||||||
c.execute("INSERT INTO history (user, slug_id, ep_id, t, payload) VALUES (?,?,?,?,?) "
|
c.execute("INSERT INTO history (user, slug_id, ep_id, t, payload) VALUES (?,?,?,?,?) "
|
||||||
"ON CONFLICT(user, slug_id, ep_id) DO UPDATE SET "
|
"ON CONFLICT(user, slug_id, ep_id) DO UPDATE SET "
|
||||||
"t=excluded.t, payload=excluded.payload",
|
"t=excluded.t, payload=CASE WHEN json_extract(excluded.payload, '$.pos') IS NULL "
|
||||||
(u, str(entry.get("slug_id")), entry["ep_id"], entry["t"], json.dumps(entry)))
|
"THEN history.payload ELSE excluded.payload END",
|
||||||
|
(u, entry["slug_id"], entry["ep_id"], entry["t"], json.dumps(entry)))
|
||||||
# cap: newest 50 per user
|
# cap: newest 50 per user
|
||||||
c.execute("DELETE FROM history WHERE user=? AND rowid NOT IN "
|
c.execute("DELETE FROM history WHERE user=? AND rowid NOT IN "
|
||||||
"(SELECT rowid FROM history WHERE user=? ORDER BY t DESC LIMIT 50)", (u, u))
|
"(SELECT rowid FROM history WHERE user=? ORDER BY t DESC LIMIT 50)", (u, u))
|
||||||
|
|||||||
+17
-9
@@ -102,9 +102,9 @@ async function refreshHistory() {
|
|||||||
// ?all=1: resume positions live beyond the top-5 cap
|
// ?all=1: resume positions live beyond the top-5 cap
|
||||||
try { history = await api("/api/history?all=1"); } catch { /* home just shows empty */ }
|
try { history = await api("/api/history?all=1"); } catch { /* home just shows empty */ }
|
||||||
}
|
}
|
||||||
function postHistory(entry) {
|
function postHistory(entry, keepalive = false) {
|
||||||
fetch(BASE + "api/history", {method: "POST", headers: {"Content-Type": "application/json", "X-Aniweb": "1"},
|
fetch(BASE + "api/history", {method: "POST", headers: {"Content-Type": "application/json", "X-Aniweb": "1"},
|
||||||
body: JSON.stringify(entry)}).catch(() => {});
|
body: JSON.stringify(entry), keepalive}).catch(() => {});
|
||||||
}
|
}
|
||||||
function postEvent(ev) {
|
function postEvent(ev) {
|
||||||
fetch(BASE + "api/events", {method: "POST", headers: {"Content-Type": "application/json", "X-Aniweb": "1"},
|
fetch(BASE + "api/events", {method: "POST", headers: {"Content-Type": "application/json", "X-Aniweb": "1"},
|
||||||
@@ -1023,7 +1023,13 @@ async function play(ep_id, ep_no) {
|
|||||||
lastSavedPos = 0;
|
lastSavedPos = 0;
|
||||||
lastSeenPos = 0;
|
lastSeenPos = 0;
|
||||||
if (nextCta) { nextCta.remove(); nextCta = null; }
|
if (nextCta) { nextCta.remove(); nextCta = null; }
|
||||||
const saved = history.find(x => x.slug_id === detail.slug_id && x.ep_id === ep_id);
|
let saved = history.find(x => x.slug_id === detail.slug_id && x.ep_id === ep_id);
|
||||||
|
if (!saved) {
|
||||||
|
// the global may be empty (boot fetch failed — it swallows errors):
|
||||||
|
// re-fetch so the write below can't clobber a real saved position
|
||||||
|
await refreshHistory();
|
||||||
|
saved = history.find(x => x.slug_id === detail.slug_id && x.ep_id === ep_id);
|
||||||
|
}
|
||||||
if (saved?.pos > 10) {
|
if (saved?.pos > 10) {
|
||||||
v.addEventListener("loadedmetadata", () => {
|
v.addEventListener("loadedmetadata", () => {
|
||||||
if (saved.pos < v.duration * 0.95) v.currentTime = saved.pos;
|
if (saved.pos < v.duration * 0.95) v.currentTime = saved.pos;
|
||||||
@@ -1062,8 +1068,10 @@ async function play(ep_id, ep_no) {
|
|||||||
setTimeout(() => { if (v.paused && v.buffered.length) doPlay(); }, 3000);
|
setTimeout(() => { if (v.paused && v.buffered.length) doPlay(); }, 3000);
|
||||||
watchPos(v);
|
watchPos(v);
|
||||||
setupIntro(v, ep_no);
|
setupIntro(v, ep_no);
|
||||||
postHistory({...meta(v), title: detail.name, slug_id: detail.slug_id, season_id: seasonId,
|
// only bump the existing row to the top; with nothing saved a pos-0
|
||||||
ep_id, lang, ep: ep_no, pos: saved?.pos || 0});
|
// write would clobber the server's position if the re-fetch also failed
|
||||||
|
if (saved) postHistory({...meta(v), title: detail.name, slug_id: detail.slug_id, season_id: seasonId,
|
||||||
|
ep_id, lang, ep: ep_no, pos: saved.pos});
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
showStreamError(e.message, () => play(ep_id, ep_no));
|
showStreamError(e.message, () => play(ep_id, ep_no));
|
||||||
}
|
}
|
||||||
@@ -1104,7 +1112,7 @@ const meta = v => ({...(Number.isFinite(v.duration) && v.duration > 0 ? {dur: v.
|
|||||||
|
|
||||||
// one savePos covers all video elements: it reads currentEp, so stale
|
// one savePos covers all video elements: it reads currentEp, so stale
|
||||||
// closures from earlier plays can't write the wrong episode's position
|
// closures from earlier plays can't write the wrong episode's position
|
||||||
function savePos(floor = 0) {
|
function savePos(floor = 0, keep = false) {
|
||||||
if (!currentEp) return;
|
if (!currentEp) return;
|
||||||
// a fatal frag error can reset video.currentTime to 0 before this runs
|
// a fatal frag error can reset video.currentTime to 0 before this runs
|
||||||
// (hls.js recovery); playerFail passes the furthest point reached as floor
|
// (hls.js recovery); playerFail passes the furthest point reached as floor
|
||||||
@@ -1113,7 +1121,7 @@ function savePos(floor = 0) {
|
|||||||
lastSavedPos = pos;
|
lastSavedPos = pos;
|
||||||
postHistory({...meta(currentEp.v), title: detail.name, slug_id: detail.slug_id, season_id: seasonId,
|
postHistory({...meta(currentEp.v), title: detail.name, slug_id: detail.slug_id, season_id: seasonId,
|
||||||
ep_id: currentEp.ep_id, lang, ep: currentEp.ep_no, pos,
|
ep_id: currentEp.ep_id, lang, ep: currentEp.ep_no, pos,
|
||||||
...(epsList.length ? {total: epsList.length} : {})});
|
...(epsList.length ? {total: epsList.length} : {})}, keep);
|
||||||
}
|
}
|
||||||
|
|
||||||
function watchPos(v) {
|
function watchPos(v) {
|
||||||
@@ -1122,12 +1130,12 @@ function watchPos(v) {
|
|||||||
const seen = () => { lastSeenPos = Math.max(lastSeenPos, v.currentTime); };
|
const seen = () => { lastSeenPos = Math.max(lastSeenPos, v.currentTime); };
|
||||||
v.addEventListener("timeupdate", () => { seen(); if (v.currentTime - lastSavedPos >= 10) savePos(); });
|
v.addEventListener("timeupdate", () => { seen(); if (v.currentTime - lastSavedPos >= 10) savePos(); });
|
||||||
v.addEventListener("seeking", seen);
|
v.addEventListener("seeking", seen);
|
||||||
v.addEventListener("pause", savePos);
|
v.addEventListener("pause", () => savePos()); // not savePos: the event would become `floor` -> NaN pos
|
||||||
// pagehide is bound once at boot (per-play listeners accumulated and fired
|
// pagehide is bound once at boot (per-play listeners accumulated and fired
|
||||||
// with stale video elements)
|
// with stale video elements)
|
||||||
}
|
}
|
||||||
|
|
||||||
window.addEventListener("pagehide", savePos);
|
window.addEventListener("pagehide", () => savePos(0, true)); // keepalive: a plain fetch dies with the page
|
||||||
|
|
||||||
// ---------- boot ----------
|
// ---------- boot ----------
|
||||||
(async () => {
|
(async () => {
|
||||||
|
|||||||
+42
-3
@@ -104,6 +104,45 @@ def test_history_requires_ep_id(base=BASE):
|
|||||||
assert req("POST", "/api/history", {"slug_id": "x"}, jar=jar, base=base) == 422
|
assert req("POST", "/api/history", {"slug_id": "x"}, jar=jar, base=base) == 422
|
||||||
|
|
||||||
|
|
||||||
|
def test_history_requires_slug(base=BASE):
|
||||||
|
jar = login("admin", "adminpw12345", base)
|
||||||
|
# a missing slug_id used to be stored as a "None" ghost row
|
||||||
|
assert req("POST", "/api/history", {"ep_id": 991}, jar=jar, base=base) == 422
|
||||||
|
|
||||||
|
|
||||||
|
def test_history_clamps_pos(base=BASE):
|
||||||
|
jar = login("admin", "adminpw12345", base)
|
||||||
|
e = {"ep_id": 992, "slug_id": "junk-a", "title": "Junk", "season_id": 1,
|
||||||
|
"ep": "1", "lang": "eng", "pos": -50, "dur": 100}
|
||||||
|
assert req("POST", "/api/history", e, jar=jar, base=base) == 200
|
||||||
|
got = [x for x in _json("GET", "/api/history?all=1", jar=jar, base=base)
|
||||||
|
if x["slug_id"] == "junk-a"]
|
||||||
|
assert got and got[0]["pos"] == 0.0, got
|
||||||
|
e["pos"] = 999999
|
||||||
|
assert req("POST", "/api/history", e, jar=jar, base=base) == 200
|
||||||
|
got = [x for x in _json("GET", "/api/history?all=1", jar=jar, base=base)
|
||||||
|
if x["slug_id"] == "junk-a"]
|
||||||
|
assert got and got[0]["pos"] == 100.0, got
|
||||||
|
# non-numeric pos is rejected, not stored as junk
|
||||||
|
e["pos"] = "somenonsense"
|
||||||
|
assert req("POST", "/api/history", e, jar=jar, base=base) == 422
|
||||||
|
|
||||||
|
|
||||||
|
def test_history_null_pos_keeps_saved(base=BASE):
|
||||||
|
jar = login("dave", "davepw12345", base)
|
||||||
|
good = {"ep_id": 993, "slug_id": "keep-a", "title": "Keep", "season_id": 1,
|
||||||
|
"ep": "1", "lang": "eng", "pos": 400, "dur": 1500}
|
||||||
|
assert req("POST", "/api/history", good, jar=jar, base=base) == 200
|
||||||
|
# a stale client (old NaN-pos bug) re-POSTs pos null: the upsert must
|
||||||
|
# keep the row's saved position instead of clobbering it to 0
|
||||||
|
bad = dict(good)
|
||||||
|
bad["pos"] = None
|
||||||
|
assert req("POST", "/api/history", bad, jar=jar, base=base) == 200
|
||||||
|
got = [x for x in _json("GET", "/api/history?all=1", jar=jar, base=base)
|
||||||
|
if x["slug_id"] == "keep-a"]
|
||||||
|
assert got and got[0]["pos"] == 400, got
|
||||||
|
|
||||||
|
|
||||||
def test_history_per_user(base=BASE):
|
def test_history_per_user(base=BASE):
|
||||||
a = login("admin", "adminpw12345", base)
|
a = login("admin", "adminpw12345", base)
|
||||||
b = login("alice", "alicepw12345", base)
|
b = login("alice", "alicepw12345", base)
|
||||||
@@ -258,9 +297,9 @@ def test_stream_token_host_allowlist(base=BASE):
|
|||||||
|
|
||||||
|
|
||||||
def test_lockout_per_user(base=BASE):
|
def test_lockout_per_user(base=BASE):
|
||||||
# must run on a fresh server (in-memory lockout): 5 fails -> 429 for bob,
|
# a throwaway name: the lockout is per-username and in-memory, and bob is
|
||||||
# admin unaffected
|
# also used by the browser tests (a locked-out bob breaks their logins)
|
||||||
codes = [req("POST", "/login", {"username": "bob", "password": "bad"}, base=base)
|
codes = [req("POST", "/login", {"username": "lockout-ghost", "password": "bad"}, base=base)
|
||||||
for _ in range(6)]
|
for _ in range(6)]
|
||||||
assert codes[:5] == [401] * 5 and codes[5] == 429, codes
|
assert codes[:5] == [401] * 5 and codes[5] == 429, codes
|
||||||
assert req("POST", "/login", {"username": "admin", "password": "adminpw12345"}, base=base) == 200
|
assert req("POST", "/login", {"username": "admin", "password": "adminpw12345"}, base=base) == 200
|
||||||
|
|||||||
+74
-9
@@ -31,7 +31,7 @@ async def _login(page, user, pw):
|
|||||||
# "element stable" actionability check hangs non-deterministically;
|
# "element stable" actionability check hangs non-deterministically;
|
||||||
# fill() leaves focus on the field, so a bare keypress submits.
|
# fill() leaves focus on the field, so a bare keypress submits.
|
||||||
await page.keyboard.press("Enter")
|
await page.keyboard.press("Enter")
|
||||||
await page.wait_for_selector("#q", timeout=15000)
|
await page.wait_for_selector("#q", timeout=30000) # 30s: full-suite load slows the home render
|
||||||
await page.wait_for_function(
|
await page.wait_for_function(
|
||||||
f"document.getElementById('me').textContent.includes('{user}')",
|
f"document.getElementById('me').textContent.includes('{user}')",
|
||||||
timeout=5000, polling=POLL)
|
timeout=5000, polling=POLL)
|
||||||
@@ -73,7 +73,7 @@ async def _login_ui():
|
|||||||
|
|
||||||
|
|
||||||
async def _login_settled(page):
|
async def _login_settled(page):
|
||||||
await page.wait_for_selector("#q", timeout=15000)
|
await page.wait_for_selector("#q", timeout=30000) # 30s: full-suite load slows the home render
|
||||||
await page.wait_for_function(
|
await page.wait_for_function(
|
||||||
"document.getElementById('me').textContent.length > 0", timeout=5000, polling=POLL)
|
"document.getElementById('me').textContent.length > 0", timeout=5000, polling=POLL)
|
||||||
|
|
||||||
@@ -333,6 +333,62 @@ async def _episode_watch_state():
|
|||||||
|
|
||||||
|
|
||||||
|
|
||||||
|
def test_resume_saved_position():
|
||||||
|
asyncio.run(_resume_saved_position())
|
||||||
|
|
||||||
|
|
||||||
|
async def _resume_saved_position():
|
||||||
|
# end-to-end resume: seed a saved position, abort the first
|
||||||
|
# /api/history fetch (a failed boot fetch leaves history = []), and the
|
||||||
|
# page must resume near the saved time — not restart at 0. POSTs are
|
||||||
|
# aborted too, so the dying page's pagehide save can't clobber the seed.
|
||||||
|
async with async_playwright() as p:
|
||||||
|
browser = await p.chromium.launch(args=LAUNCH_ARGS)
|
||||||
|
page = await (await browser.new_context()).new_page()
|
||||||
|
errors = []
|
||||||
|
page.on("pageerror", lambda e: errors.append(str(e)))
|
||||||
|
|
||||||
|
await _login(page, "bob", "bobpw12345")
|
||||||
|
await page.goto(BASE + "/?anime=chainsaw-man-922")
|
||||||
|
await page.wait_for_selector("#eps .ep", timeout=30000)
|
||||||
|
e1_id = await page.evaluate("document.querySelector('#eps .ep').dataset.ep")
|
||||||
|
# seed exactly the way savePos writes (same session, same fields)
|
||||||
|
status = await page.evaluate(
|
||||||
|
"(id) => fetch('api/history', {method: 'POST'," \
|
||||||
|
" headers: {'Content-Type': 'application/json', 'X-Aniweb': '1'}," \
|
||||||
|
" body: JSON.stringify({title: detail.name, slug_id: detail.slug_id,"
|
||||||
|
" season_id: seasonId, ep_id: +id, lang: 'jpn', ep: 1,"
|
||||||
|
" pos: 400, dur: 1500})}).then(r => r.status)",
|
||||||
|
e1_id)
|
||||||
|
assert status == 200
|
||||||
|
|
||||||
|
state = {"aborted": 0}
|
||||||
|
async def hist_route(route):
|
||||||
|
if route.request.method == "POST":
|
||||||
|
await route.abort()
|
||||||
|
return
|
||||||
|
if state["aborted"] == 0: # the reloaded page's boot fetch
|
||||||
|
state["aborted"] += 1
|
||||||
|
await route.abort()
|
||||||
|
return
|
||||||
|
await route.continue_()
|
||||||
|
await page.route("**/api/history*", hist_route)
|
||||||
|
|
||||||
|
await page.reload()
|
||||||
|
# resumes near 400 (keyframe-aligned), not at 0
|
||||||
|
await page.wait_for_function(
|
||||||
|
"(document.querySelector('video')?.currentTime || 0) > 350",
|
||||||
|
timeout=120000, polling=250)
|
||||||
|
got = await page.evaluate("fetch('api/history?all=1').then(r => r.json())")
|
||||||
|
row = [x for x in got if x["slug_id"] == "chainsaw-man-922"
|
||||||
|
and x["ep_id"] == int(e1_id)]
|
||||||
|
# the row still holds the seeded position (a boot-write of
|
||||||
|
# pos: saved?.pos || 0 used to clobber it when the fetch failed)
|
||||||
|
assert row and row[0]["pos"] == 400, row
|
||||||
|
assert not errors, errors
|
||||||
|
await browser.close()
|
||||||
|
|
||||||
|
|
||||||
def test_autoplay_blocked_shows_tap_to_play():
|
def test_autoplay_blocked_shows_tap_to_play():
|
||||||
asyncio.run(_autoplay_blocked())
|
asyncio.run(_autoplay_blocked())
|
||||||
|
|
||||||
@@ -364,15 +420,24 @@ async def _autoplay_blocked():
|
|||||||
await _login(page, "admin", "adminpw12345")
|
await _login(page, "admin", "adminpw12345")
|
||||||
await page.goto(BASE + "/?anime=chainsaw-man-922")
|
await page.goto(BASE + "/?anime=chainsaw-man-922")
|
||||||
await page.wait_for_selector("#eps .ep", timeout=30000)
|
await page.wait_for_selector("#eps .ep", timeout=30000)
|
||||||
# blocked autoplay: the dead "Loading…" must become a tappable overlay
|
# blocked autoplay: the dead "Loading…" must become a tappable overlay.
|
||||||
|
# The fake block only rejects the FIRST play(); the app's 3s safety
|
||||||
|
# net then retriggers doPlay unblocked, so "already playing" is also a
|
||||||
|
# valid end state. 60s: under full-suite load the CDN can be slow.
|
||||||
await page.wait_for_function(
|
await page.wait_for_function(
|
||||||
"document.querySelector('.pl-overlay') && "
|
"(document.querySelector('video')?.currentTime || 0) > 0.5 || "
|
||||||
"document.querySelector('.pl-overlay').textContent.includes('Tap to play')",
|
"!!(document.querySelector('.pl-overlay .btn') && "
|
||||||
timeout=15000, polling=POLL)
|
" document.querySelector('.pl-overlay').textContent.includes('Tap to play'))",
|
||||||
# tapping (the gesture) starts real playback
|
timeout=60000, polling=POLL)
|
||||||
await page.click(".pl-overlay .btn", force=True)
|
# tapping (the gesture) starts real playback; if buffering swapped the
|
||||||
|
# overlay away in the meantime the video is already playing
|
||||||
|
if await page.query_selector(".pl-overlay .btn"):
|
||||||
|
try:
|
||||||
|
await page.click(".pl-overlay .btn", force=True, timeout=10000)
|
||||||
|
except Exception:
|
||||||
|
pass # a lost race fails loudly at the final wait below
|
||||||
await page.wait_for_function(
|
await page.wait_for_function(
|
||||||
"document.querySelector('video').currentTime > 0.5",
|
"(document.querySelector('video')?.currentTime || 0) > 0.5",
|
||||||
timeout=120000, polling=250)
|
timeout=120000, polling=250)
|
||||||
assert not errors, errors
|
assert not errors, errors
|
||||||
await browser.close()
|
await browser.close()
|
||||||
Reference in new issue
Block a user