diff --git a/package.json b/package.json index d42a325..0c147ae 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "netfelix-audio-fix", - "version": "2026.04.14.16", + "version": "2026.04.14.17", "scripts": { "dev:server": "NODE_ENV=development bun --hot server/index.tsx", "dev:client": "vite", diff --git a/server/api/__tests__/scan.test.ts b/server/api/__tests__/scan.test.ts new file mode 100644 index 0000000..4f3129b --- /dev/null +++ b/server/api/__tests__/scan.test.ts @@ -0,0 +1,31 @@ +import { describe, expect, test } from "bun:test"; +import { parseScanLimit } from "../scan"; + +describe("parseScanLimit", () => { + test("accepts positive integers and nullish/empty as no-limit", () => { + expect(parseScanLimit(5)).toEqual({ ok: true, value: 5 }); + expect(parseScanLimit(1)).toEqual({ ok: true, value: 1 }); + expect(parseScanLimit(10_000)).toEqual({ ok: true, value: 10_000 }); + expect(parseScanLimit(null)).toEqual({ ok: true, value: null }); + expect(parseScanLimit(undefined)).toEqual({ ok: true, value: null }); + expect(parseScanLimit("")).toEqual({ ok: true, value: null }); + }); + + test("coerces numeric strings (env var path) but rejects garbage", () => { + expect(parseScanLimit("7")).toEqual({ ok: true, value: 7 }); + expect(parseScanLimit("abc")).toEqual({ ok: false }); + expect(parseScanLimit("12abc")).toEqual({ ok: false }); + }); + + test("rejects the footguns that would silently disable the cap", () => { + // NaN: processed >= NaN never trips → cap never fires. + expect(parseScanLimit(Number.NaN)).toEqual({ ok: false }); + // Negative: off-by-one bugs in Math.min(limit, total). + expect(parseScanLimit(-1)).toEqual({ ok: false }); + expect(parseScanLimit(0)).toEqual({ ok: false }); + // Float: Math.min is fine but percentage math breaks on non-integers. + expect(parseScanLimit(1.5)).toEqual({ ok: false }); + // Infinity is technically a number but has no business as a cap. + expect(parseScanLimit(Number.POSITIVE_INFINITY)).toEqual({ ok: false }); + }); +}); diff --git a/server/api/review.ts b/server/api/review.ts index 3904f9a..c0f0e4e 100644 --- a/server/api/review.ts +++ b/server/api/review.ts @@ -130,11 +130,16 @@ function loadItemDetail(db: ReturnType, itemId: number) { * composite of (type, language, stream_index, title) so user overrides * survive stream-id changes when Jellyfin re-probes metadata. */ -function titleKey(s: { type: string; language: string | null; stream_index: number; title: string | null }): string { +export function titleKey(s: { + type: string; + language: string | null; + stream_index: number; + title: string | null; +}): string { return `${s.type}|${s.language ?? ""}|${s.stream_index}|${s.title ?? ""}`; } -function reanalyze(db: ReturnType, itemId: number, preservedTitles?: Map): void { +export function reanalyze(db: ReturnType, itemId: number, preservedTitles?: Map): void { const item = db.prepare("SELECT * FROM media_items WHERE id = ?").get(itemId) as MediaItem; if (!item) return; diff --git a/server/api/scan.ts b/server/api/scan.ts index 1c6e9e4..b5f5bc6 100644 --- a/server/api/scan.ts +++ b/server/api/scan.ts @@ -10,6 +10,19 @@ import { loadLibrary as loadSonarrLibrary, isUsable as sonarrUsable } from "../s const app = new Hono(); +/** + * Validate a scan `limit` input. Must be a positive integer or absent — + * NaN/negatives/non-numerics would disable the progress cap + * (`processed >= NaN` never trips) or produce bogus totals via + * `Math.min(NaN, …)`. Exported for unit tests. + */ +export function parseScanLimit(raw: unknown): { ok: true; value: number | null } | { ok: false } { + if (raw == null || raw === "") return { ok: true, value: null }; + const n = typeof raw === "number" ? raw : Number(raw); + if (!Number.isInteger(n) || n <= 0) return { ok: false }; + return { ok: true, value: n }; +} + // ─── State ──────────────────────────────────────────────────────────────────── let scanAbort: AbortController | null = null; @@ -63,10 +76,14 @@ app.post("/start", async (c) => { return c.json({ ok: false, error: "Scan already running" }, 409); } - const body = await c.req.json<{ limit?: number }>().catch(() => ({ limit: undefined })); - const formLimit = body.limit ?? null; - const envLimit = process.env.SCAN_LIMIT ? Number(process.env.SCAN_LIMIT) : null; - const limit = formLimit ?? envLimit ?? null; + const body = await c.req.json<{ limit?: unknown }>().catch(() => ({ limit: undefined })); + const formLimit = parseScanLimit(body.limit); + const envLimit = parseScanLimit(process.env.SCAN_LIMIT); + if (!formLimit.ok || !envLimit.ok) { + db.prepare("UPDATE config SET value = '0' WHERE key = 'scan_running'").run(); + return c.json({ ok: false, error: "limit must be a positive integer" }, 400); + } + const limit: number | null = formLimit.value ?? envLimit.value ?? null; setConfig("scan_limit", limit != null ? String(limit) : ""); runScan(limit).catch((err) => { @@ -239,8 +256,11 @@ async function runScan(limit: number | null = null): Promise { db .prepare("UPDATE media_items SET scan_status = 'error', scan_error = ? WHERE jellyfin_id = ?") .run(String(err), jellyfinItem.Id); - } catch { - /* ignore */ + } catch (dbErr) { + // Failed to persist the error status — log it so the incident + // doesn't disappear silently. We can't do much more; the outer + // loop continues so the scan still finishes. + logError(`Failed to record scan error for ${jellyfinItem.Id}:`, dbErr); } emitSse("log", { name: jellyfinItem.Name, type: jellyfinItem.Type, status: "error", file: jellyfinItem.Path }); } diff --git a/server/api/settings.ts b/server/api/settings.ts index 1f69a91..5424ca4 100644 --- a/server/api/settings.ts +++ b/server/api/settings.ts @@ -26,12 +26,15 @@ app.post("/jellyfin", async (c) => { // { ok, saved, testError } shape to decide what message to show. setConfig("jellyfin_url", url); setConfig("jellyfin_api_key", apiKey); - setConfig("setup_complete", "1"); const result = await testJellyfin({ url, apiKey }); - // Best-effort admin discovery only when the connection works; ignore failures. + // Only mark setup complete when the connection actually works. Setting + // setup_complete=1 on a failing test would let the user click past the + // wizard into an app that then dies on the first Jellyfin call. if (result.ok) { + setConfig("setup_complete", "1"); + // Best-effort admin discovery only when the connection works; ignore failures. try { const users = await getUsers({ url, apiKey }); const admin = users.find((u) => u.Name === "admin") ?? users[0]; diff --git a/server/api/subtitles.ts b/server/api/subtitles.ts index 04b6a86..2e3da5a 100644 --- a/server/api/subtitles.ts +++ b/server/api/subtitles.ts @@ -6,6 +6,7 @@ import { error as logError } from "../lib/log"; import { parseId } from "../lib/validate"; import { getItem, mapStream, normalizeLanguage, refreshItem } from "../services/jellyfin"; import type { MediaItem, MediaStream, ReviewPlan, StreamDecision, SubtitleFile } from "../types"; +import { reanalyze, titleKey } from "./review"; const app = new Hono(); @@ -405,6 +406,30 @@ app.post("/:id/rescan", async (c) => { await refreshItem(jfCfg, item.jellyfin_id); + // Snapshot custom_titles before the DELETE cascades stream_decisions away, + // so reanalyze() can re-attach them to the corresponding new stream rows. + // Without this rescanning subtitles also wipes per-audio-stream title + // overrides the user made in the review UI. + const preservedTitles = new Map(); + const oldTitleRows = db + .prepare(` + SELECT ms.type, ms.language, ms.stream_index, ms.title, sd.custom_title + FROM stream_decisions sd + JOIN media_streams ms ON ms.id = sd.stream_id + JOIN review_plans rp ON rp.id = sd.plan_id + WHERE rp.item_id = ? AND sd.custom_title IS NOT NULL + `) + .all(id) as { + type: string; + language: string | null; + stream_index: number; + title: string | null; + custom_title: string; + }[]; + for (const r of oldTitleRows) { + preservedTitles.set(titleKey(r), r.custom_title); + } + const fresh = await getItem(jfCfg, item.jellyfin_id); if (fresh) { const insertStream = db.prepare(` @@ -412,6 +437,10 @@ app.post("/:id/rescan", async (c) => { title, is_default, is_forced, is_hearing_impaired, channels, channel_layout, bit_rate, sample_rate) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) `); + // DELETE cascades to stream_decisions via FK. reanalyze() below + // rebuilds them from the fresh streams; without it the plan would + // keep status='done'/'approved' but reference zero decisions, and + // ffmpeg would emit a no-op command. db.prepare("DELETE FROM media_streams WHERE item_id = ?").run(id); for (const jStream of fresh.MediaStreams ?? []) { if (jStream.IsExternal) continue; @@ -435,6 +464,8 @@ app.post("/:id/rescan", async (c) => { } } + reanalyze(db, id, preservedTitles); + const detail = loadDetail(db, id); if (!detail) return c.notFound(); return c.json(detail); diff --git a/server/lib/__tests__/validate.test.ts b/server/lib/__tests__/validate.test.ts index 5a29794..b814e06 100644 --- a/server/lib/__tests__/validate.test.ts +++ b/server/lib/__tests__/validate.test.ts @@ -15,8 +15,12 @@ describe("parseId", () => { expect(parseId(undefined)).toBe(null); }); - test("parses leading integer from mixed strings (parseInt semantics)", () => { - expect(parseId("42abc")).toBe(42); + test("rejects mixed alphanumeric strings (strict — route params must be wholly numeric)", () => { + expect(parseId("42abc")).toBe(null); + expect(parseId("abc42")).toBe(null); + expect(parseId("42 ")).toBe(null); + expect(parseId("+42")).toBe(null); + expect(parseId("42.0")).toBe(null); }); }); diff --git a/server/lib/validate.ts b/server/lib/validate.ts index 3018b6f..9f1d2a3 100644 --- a/server/lib/validate.ts +++ b/server/lib/validate.ts @@ -1,8 +1,12 @@ import type { Context } from "hono"; -/** Parse a route param as a positive integer id. Returns null if invalid. */ +/** + * Parse a route param as a positive integer id. Returns null if invalid. + * Strict: rejects mixed strings like "42abc" that Number.parseInt would + * accept — route params must be wholly numeric or the request is bad. + */ export function parseId(raw: string | undefined): number | null { - if (!raw) return null; + if (!raw || !/^\d+$/.test(raw)) return null; const n = Number.parseInt(raw, 10); return Number.isFinite(n) && n > 0 ? n : null; } diff --git a/server/services/__tests__/ffmpeg.test.ts b/server/services/__tests__/ffmpeg.test.ts index 757e29f..309ce73 100644 --- a/server/services/__tests__/ffmpeg.test.ts +++ b/server/services/__tests__/ffmpeg.test.ts @@ -185,7 +185,15 @@ describe("buildCommand", () => { test("writes canonical 'ENG - CODEC · CHANNELS' title on every kept audio stream", () => { const streams = [ stream({ id: 1, type: "Video", stream_index: 0 }), - stream({ id: 2, type: "Audio", stream_index: 1, codec: "ac3", channels: 6, language: "eng", title: "Audio Description" }), + stream({ + id: 2, + type: "Audio", + stream_index: 1, + codec: "ac3", + channels: 6, + language: "eng", + title: "Audio Description", + }), stream({ id: 3, type: "Audio", stream_index: 2, codec: "dts", channels: 1, language: "deu" }), stream({ id: 4, type: "Audio", stream_index: 3, codec: "aac", channels: 2, language: null }), ];