diff --git a/CHANGELOG.md b/CHANGELOG.md index cc3329a..8880854 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,7 +27,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - **Perf harness now measures 2D-highway frame time (R3c gate).** `scripts/perf-baseline.mjs` gains a `--song` mode that reports per-frame draw-cost p50/p95/p99 (draw-tagged via `highway.addDrawHook`), the metric that gates the `highway.js` split. Maintainer/CI-only; baseline recorded in `docs/perf-baseline.md`. -- **`routers/` — extracting `server.py`'s route layer, cheapest-first (R3).** Each PR moves a cohesive route group into a `fastapi.APIRouter` under `lib/routers/`, mounted with `app.include_router(...)` at its original site (FastAPI matches in registration order; the full route table stays byte-identical). Bodies are verbatim — only the decorator receiver (`@app` → `@router`) and singleton reads (`meta_db` → `appstate.meta_db`, resolved at call time) change. So far: `audio_effects` (5), `artist_aliases` (5), `loops` (3), `playlists` (12 + covers), `ws_highway` (the 902-line highway chart WebSocket), `chart` (split/unsplit/work/fileinfo — unblocked by the DLC-path substrate), `library_extras`, `wanted`, `shop`, `progression`, `profile`, `stats` (the `/api/stats/{path}` catch-all stays registered last so it can't shadow `/recent` `/best` `/top`), `version` (`/api/version`; VERSION-file lookup adjusted for the router subdir depth), and `diagnostics` (`/api/diagnostics/export|preview|hardware`; the plugins-root lookup adjusted for the router subdir depth, `_running_version` reached through the `appstate` seam, pure payload-cap helpers re-exported for the `server._diag_*` tests), and `tunings` (`/api/tunings`; the pure `config.json` reader moved to `lib/appconfig.py`, the tuning-provider registry read through the `appstate` seam so plugin-contributed tunings still merge). The DLC library-path resolution (`_get_dlc_dir`, pure `_resolve_dlc_path`) moved to `lib/dlc_paths.py`, reading paths through the seam; `config_dir`/`dlc_dir`/`dlc_dir_env` now ride the `appstate` seam (env-derived, so the pop-and-reimport fixtures reconfigure it for free), and the shared request-field sanitizer `_clean_str` moved to `lib/reqfields.py`. The next cut is picked by a dependency-closure scan that ranks groups by how many `monkeypatch.setattr(server, …)` targets they'd drag along. +- **`routers/` — extracting `server.py`'s route layer, cheapest-first (R3).** Each PR moves a cohesive route group into a `fastapi.APIRouter` under `lib/routers/`, mounted with `app.include_router(...)` at its original site (FastAPI matches in registration order; the full route table stays byte-identical). Bodies are verbatim — only the decorator receiver (`@app` → `@router`) and singleton reads (`meta_db` → `appstate.meta_db`, resolved at call time) change. So far: `audio_effects` (5), `artist_aliases` (5), `loops` (3), `playlists` (12 + covers), `ws_highway` (the 902-line highway chart WebSocket), `chart` (split/unsplit/work/fileinfo — unblocked by the DLC-path substrate), `library_extras`, `wanted`, `shop`, `progression`, `profile`, `stats` (the `/api/stats/{path}` catch-all stays registered last so it can't shadow `/recent` `/best` `/top`), `version` (`/api/version`; VERSION-file lookup adjusted for the router subdir depth), `art` (the `/api/song/{f}/art*` serve/cover-search/candidates/upload/url + `/api/art/{f}/override` routes; the shared `_song_pack_art_exists`/`_art_override_paths`/`_art_safe_name` helpers stay in `server.py` for the song/delete routes and are reached through the `appstate` seam, the CAA/release transport as `enrichment.X`), and `diagnostics` (`/api/diagnostics/export|preview|hardware`; the plugins-root lookup adjusted for the router subdir depth, `_running_version` reached through the `appstate` seam, pure payload-cap helpers re-exported for the `server._diag_*` tests), and `tunings` (`/api/tunings`; the pure `config.json` reader moved to `lib/appconfig.py`, the tuning-provider registry read through the `appstate` seam so plugin-contributed tunings still merge). The DLC library-path resolution (`_get_dlc_dir`, pure `_resolve_dlc_path`) moved to `lib/dlc_paths.py`, reading paths through the seam; `config_dir`/`dlc_dir`/`dlc_dir_env` now ride the `appstate` seam (env-derived, so the pop-and-reimport fixtures reconfigure it for free), and the shared request-field sanitizer `_clean_str` moved to `lib/reqfields.py`. The next cut is picked by a dependency-closure scan that ranks groups by how many `monkeypatch.setattr(server, …)` targets they'd drag along. - **`routers/` — the first extracted route module (R3).** The five audio-effects mapping endpoints move out of `server.py` into `lib/routers/audio_effects.py` as a `fastapi.APIRouter`, mounted with `app.include_router(...)` **at the point in the file diff --git a/docs/size-exemptions.md b/docs/size-exemptions.md index 9a52c36..341c9b2 100644 --- a/docs/size-exemptions.md +++ b/docs/size-exemptions.md @@ -55,8 +55,8 @@ without a *signed* exemption" is unenforceable. ## Planned, NOT exempt (owned by split plans — listed so nothing falls between states) core `static/app.js` (11,852) · `static/highway.js` (4,168, whole file) · `server.py` -(5,988 — was 14,037; ratcheted by the R3 `MetadataDB` + `AudioEffectsMappingDB` -extractions and fifteen `routers/` modules; the ~930-line metadata-enrichment subsystem — MB/CAA/AcoustID transport, matcher, background worker — now lives in `lib/enrichment.py`) · +(5,540 — was 14,037; ratcheted by the R3 `MetadataDB` + `AudioEffectsMappingDB` +extractions and sixteen `routers/` modules (the album-art routes are now `lib/routers/art.py`); the ~930-line metadata-enrichment subsystem — MB/CAA/AcoustID transport, matcher, background worker — now lives in `lib/enrichment.py`) · `lib/metadata_db.py` (4,373 — new in R3; the `MetadataDB` class alone is 4,018 lines and is a monolith in its own right, to be split per-table once the router train lands) · `static/v3/songs.js` (4,134) · `static/capabilities/audio-session.js` diff --git a/lib/appstate.py b/lib/appstate.py index 7181dd3..791bb0e 100644 --- a/lib/appstate.py +++ b/lib/appstate.py @@ -98,6 +98,7 @@ running_version = None art_cache_dir = None song_pack_art_exists = None art_override_paths = None +art_safe_name = None _SLOTS = frozenset({ "meta_db", "audio_effect_mappings", "tuning_providers", @@ -105,7 +106,7 @@ _SLOTS = frozenset({ "static_dir", "sloppak_cache_dir", "audio_cache_dir", "get_progression_content", "builtin_diagnostic_filename", "running_version", - "art_cache_dir", "song_pack_art_exists", "art_override_paths", + "art_cache_dir", "song_pack_art_exists", "art_override_paths", "art_safe_name", }) diff --git a/lib/routers/art.py b/lib/routers/art.py new file mode 100644 index 0000000..6fce6ba --- /dev/null +++ b/lib/routers/art.py @@ -0,0 +1,513 @@ +"""Album-art routes: serve / cover-search / candidates / upload / url / remove +(/api/song/{filename}/art*, /api/art/{filename}/override). + +Extracted verbatim from server.py (R3). Only the decorators (@app -> @router) and +the seam reads change: meta_db -> appstate.meta_db, ART_CACHE_DIR -> +appstate.art_cache_dir, and the three shared art helpers that stay in server.py +(they are also used by the song/delete routes) -> appstate. +(_song_pack_art_exists, _art_override_paths, _art_safe_name). The CAA / release +search transport lives in lib/enrichment.py and is reached as enrichment.X. +""" + +import asyncio +import hashlib +import ipaddress +from pathlib import Path + +from fastapi import APIRouter, HTTPException, Request +from fastapi.responses import FileResponse, JSONResponse, Response + +import appstate +import enrichment +import loosefolder as loosefolder_mod +import sloppak as sloppak_mod +from dlc_paths import _get_dlc_dir, _resolve_dlc_path + +import logging +log = logging.getLogger("feedBack.server") +router = APIRouter() + +def _if_none_match_hits(header: str | None, etag: str) -> bool: + """True if an If-None-Match header matches `etag` (weak comparison). + + Handles the `*` wildcard and comma-separated lists, and ignores a weak + `W/` prefix on either side — the standard semantics for a conditional GET. + """ + if not header: + return False + bare = etag.removeprefix("W/") + for tok in header.split(","): + t = tok.strip() + if t == "*" or t.removeprefix("W/") == bare: + return True + return False + + +# Album art is served with a strong validator (an ETag on the sloppak byte +# path; FileResponse's own ETag/Last-Modified on the file paths) and revalidated +# with `no-cache`. That keeps re-scroll cheap — a conditional GET returns a +# bodyless 304 — without ever serving a stale cover. A long `immutable` max-age +# was rejected: the frontend's `?v=` buster is only second-resolution, so +# a same-second cover rewrite would keep the URL and pin the old bytes for the +# cache lifetime. Validation cost is negligible for a localhost backend. +_ART_CACHE_HEADERS = {"Cache-Control": "no-cache"} + + +def _art_etag(path: Path) -> str | None: + """Strong validator for an art file: nanosecond mtime + size (so a + same-second rewrite still changes it). None if the file can't be stat'd.""" + try: + st = path.stat() + return f'"{st.st_mtime_ns}-{st.st_size}"' + except OSError: + return None + + +def _art_conditional(etag: str | None, request: Request | None): + """Return (headers, not_modified) for an art response. `not_modified` is + True when the client's If-None-Match already matches `etag` → caller should + return a bodyless 304. Starlette's FileResponse emits an ETag but does NOT + itself evaluate If-None-Match, so every art path routes through here to get + real conditional handling.""" + headers = dict(_ART_CACHE_HEADERS) + if etag: + headers["ETag"] = etag + inm = request.headers.get("if-none-match") if request is not None else None + return headers, bool(etag) and _if_none_match_hits(inm, etag) + + +def _file_art_response(path: Path, media_type: str, request: Request | None): + """FileResponse for an on-disk art file, with no-cache + ETag and a bodyless + 304 when the client's validator still matches.""" + headers, not_modified = _art_conditional(_art_etag(path), request) + if not_modified: + return Response(status_code=304, headers=headers) + return FileResponse(str(path), media_type=media_type, headers=headers) + + +@router.get("/api/song/{filename:path}/art") +async def get_song_art(filename: str, request: Request = None, source: str = ""): + """Serve album art for a song, walking the R3 override chain: + + 1. USER OVERRIDE (upload / URL-fetch, {safe_name}.gif|.png in the art + cache) — art the user explicitly pinned outranks everything, pack + art included. GIF is allowed HERE only: an animated cover is a + local-only bonus; packs stay jpg/png/webp and nothing ever writes + art into a pack file. + 2. PACK ART — sloppak cover (single member read, no full unpack) or + the loose folder's discovered image. + 3. COVER ART ARCHIVE cache — fetched by the enrichment art worker for + matched songs that lack pack art, keyed by release MBID. + + `?source=pack` narrows the chain to step 2 only (no override, no CAA): + the cover picker's "Pack original" tile must show the pack's own art + even while a user override is what the plain route serves. 404 when the + song ships no art of its own. + """ + dlc = _get_dlc_dir() + if not dlc: + return JSONResponse({"error": "not configured"}, 404) + + song_path = _resolve_dlc_path(dlc, filename) + if song_path is None: + return JSONResponse({"error": "forbidden"}, 403) + if not song_path.exists(): + return JSONResponse({"error": "not found"}, 404) + + pack_only = source == "pack" + + # 1. User override — GIF first (it wins over a stale PNG override). + if not pack_only: + for cached in appstate.art_override_paths(filename): + mt = "image/gif" if cached.suffix == ".gif" else "image/png" + return _file_art_response(cached, mt, request) + + # 2a. Sloppak: read the cover (manifest-declared or default) straight from + # the package. For a zip-form sloppak this opens just the cover member — + # NOT the whole archive — so the library grid never triggers a full unpack + # of stems just to paint a thumbnail. + if sloppak_mod.is_sloppak(song_path): + # Read the cover (cheap — single member, no full unpack) and validate by + # its CONTENT. A stat-based ETag would be wrong for directory-form + # sloppaks: editing cover.jpg in place changes the file's mtime, not the + # directory's, so a dir-stat ETag could emit a stale 304. Content hashing + # is correct for both dir- and zip-form. Raw byte Response lacks + # FileResponse's validators, so we attach the ETag + honor If-None-Match. + try: + art = await asyncio.to_thread(sloppak_mod.read_cover_bytes, song_path) + except Exception: + art = None + if art is not None: + data, mt = art + etag = f'"{hashlib.sha1(data).hexdigest()}"' + headers, not_modified = _art_conditional(etag, request) + if not_modified: + return Response(status_code=304, headers=headers) + return Response(content=data, media_type=mt, headers=headers) + + # 2b. Loose folder: serve the discovered art file directly. + # song_path is already validated against DLC_DIR by _resolve_dlc_path. + elif loosefolder_mod.is_loose_song(song_path): + art_path = loosefolder_mod.find_art(song_path) + if art_path: + # Re-resolve in case the matched file is a symlink — a crafted + # custom song could put `album_art.jpg` as a symlink to anywhere on + # disk. Insist the final target stays inside the song folder. + art_resolved = art_path.resolve() + try: + art_resolved.relative_to(song_path) + except ValueError: + return JSONResponse({"error": "forbidden"}, 403) + if art_resolved.is_file(): + mt = { + ".jpg": "image/jpeg", ".jpeg": "image/jpeg", + ".png": "image/png", ".webp": "image/webp", + }.get(art_resolved.suffix.lower(), "image/jpeg") + return _file_art_response(art_resolved, mt, request) + + # 3. Cover Art Archive cache (the enrichment art worker's fetch). + if not pack_only: + row = appstate.meta_db.get_enrichment(filename) + if row and row.get("art_state") == "caa" and row.get("art_cache_path"): + caa = Path(row["art_cache_path"]) + if caa.is_file(): + return _file_art_response(caa, "image/jpeg", request) + + return JSONResponse({"error": "no art"}, 404) + + +# ── Cover picker (PR-C): candidate assembly ─────────────────────────────────── +# Enumerated ON OPEN, never at scan time (charrette §8), and NO image bytes +# are fetched here — Cover Art Archive release INDEX jsons only (1-3 throttled +# calls on a cache miss); the tiles' thumbnails load straight from the archive +# in the client. Applying a pick never grows a new write path: the client +# POSTs the chosen thumb URL to the EXISTING …/art/url route (the override +# lane — never evicted, survives a re-match), "Pack original" DELETEs the +# override, uploads keep the existing upload route. +_ART_PICKER_MAX_CAA = 12 + + +@router.get("/api/song/{filename:path}/art/cover-search") +def api_art_cover_search(filename: str, q: str = ""): + """Search Cover Art Archive (via MusicBrainz release-groups) for album covers + — powers the Change-cover picker's search box, so a cover can be found even + for a song with no metadata match (the unmatched city-pop pile, where + /art/candidates is empty). `q` defaults to the song's own artist + album/ + title (romaji fallback applied). Read-only; the picker renders the thumbs and + applies a pick through the existing /art/url route.""" + query = (q or "").strip() + if not query: + pack = appstate.meta_db.pack_fields(appstate.meta_db._canonical_song_filename(filename)) + query = " ".join(x for x in (pack.get("artist"), pack.get("album") or pack.get("title")) if x).strip() + if not query: + return {"query": "", "covers": []} + try: + return {"query": query, "covers": enrichment._mb_search_release_groups(query, limit=8)} + except enrichment.EnrichTransportError: + return {"query": query, "covers": [], "error": "unavailable"} + + +@router.get("/api/song/{filename:path}/art/candidates") +def get_song_art_candidates(filename: str): + """Everything the cover picker can offer for one song, without fetching a + single image: the current cover (with its provenance), the pack original + when the song ships art, and CAA candidates for the matched/manual + release plus any distinct releases among the stored review candidates. + Sync route on purpose (the CAA index fetch sleeps in the shared + throttle — FastAPI runs `def` routes in the threadpool). One response, + `pending` always False — the client shows a spinner for the request's own + latency; offline / CAA-down just means an empty caa tail (the instant + tiles keep working), never an error.""" + from urllib.parse import quote + dlc = _get_dlc_dir() + song_path = _resolve_dlc_path(dlc, filename) if dlc else None + if song_path is None or not song_path.exists(): + raise HTTPException(status_code=404, detail="unknown song") + + row = appstate.meta_db.get_enrichment(filename) or {} + has_pack = appstate.song_pack_art_exists(filename) + art_url = f"/api/song/{quote(filename)}/art" + + # What the plain art route would serve right now — the serve chain's + # order (override > pack > CAA cache) restated as provenance. + if appstate.art_override_paths(filename): + provenance = "yours" + elif has_pack: + provenance = "pack" + elif row.get("art_state") == "caa" and row.get("art_cache_path"): + provenance = "matched" + else: + provenance = "none" + + candidates: list[dict] = [{ + "id": "current", "kind": "current", "label": "Current", + "thumb_url": art_url, "provenance": provenance, + }] + if has_pack: + candidates.append({ + "id": "pack", "kind": "pack", "label": "Pack original", + "thumb_url": art_url + "?source=pack", "provenance": "pack", + }) + + # Releases worth asking the archive about: the matched/manual release + # first (it seeds the best candidates), then any distinct release among + # the stored review candidates (a review row has no mb_release_id of its + # own — its releases live in the candidates JSON). + # Only spend the shared CAA rate budget on rows whose match warrants it: + # a matched/manual release seeds the best candidates, and a review row's + # stored candidates are still live proposals. A failed/rejected (or + # unscanned) row has no accepted match — asking would burn the budget and + # surface releases already rejected as non-matches. The Current + Pack + # tiles above serve regardless, so those songs still get a picker. + rids: list[str] = [] + if row.get("match_state") in ("matched", "manual", "review"): + if row.get("match_state") in ("matched", "manual") and row.get("mb_release_id"): + rids.append(str(row["mb_release_id"])) + for cand in (row.get("candidates") or []): + rid = str(cand.get("release_id") or "") if isinstance(cand, dict) else "" + if rid and rid not in rids: + rids.append(rid) + + caa_entries: list[dict] = [] + for rid in rids: + if len(caa_entries) >= _ART_PICKER_MAX_CAA: + break + try: + imgs = enrichment._caa_index_cached(rid) + except enrichment.EnrichTransportError: + # Offline / archive down — stop asking (each further miss would + # only burn a timeout). The instant tiles still serve; a later + # picker-open retries naturally (failures are never cached). + break + # Front covers first, approved before pending, otherwise index order + # (the picker grammar is a RANKED list — §7/§9). + def _rank(img): + types = img.get("types") or [] + is_front = bool(img.get("front")) or "Front" in types + return (not is_front, not bool(img.get("approved"))) + for img in sorted((i for i in imgs if isinstance(i, dict)), key=_rank): + if len(caa_entries) >= _ART_PICKER_MAX_CAA: + break + thumbs = img.get("thumbnails") or {} + if not isinstance(thumbs, dict): + continue + thumb = (thumbs.get("500") or thumbs.get("large") + or thumbs.get("250") or thumbs.get("small")) + if not thumb: + continue + types = [str(t) for t in (img.get("types") or []) if isinstance(t, str)] + caa_entries.append({ + "id": f"caa-{rid}-{img.get('id', '')}", + "kind": "caa", + "label": ", ".join(types) or "Cover", + "thumb_url": str(thumb), + "provenance": "matched", + "types": types, + "approved": bool(img.get("approved")), + "release_id": rid, + }) + + return {"candidates": candidates + caa_entries, "pending": False} + + +def _save_art_override(filename: str, img_data: bytes) -> dict: + """Persist a user art override into the art cache (R3). One override per + song: GIF input is validated and kept VERBATIM as .gif (animation intact — + the local-only bonus; it is never written into the pack file), everything + else is normalized to RGB PNG via PIL. Saving either kind removes the + other so the serve chain has exactly one user file to find.""" + appstate.art_cache_dir.mkdir(parents=True, exist_ok=True) + stem = appstate.art_safe_name(filename) + png_path = appstate.art_cache_dir / f"{stem}.png" + gif_path = appstate.art_cache_dir / f"{stem}.gif" + from PIL import Image + import io as _io + if img_data[:6] in (b"GIF87a", b"GIF89a"): + try: + probe = Image.open(_io.BytesIO(img_data)) + probe.verify() # decodes headers/frames without keeping the image + if probe.format != "GIF": + raise ValueError("not a GIF") + except Exception as e: + return {"error": f"Invalid image: {e}"} + gif_path.write_bytes(img_data) + png_path.unlink(missing_ok=True) + return {"ok": True, "kind": "gif"} + try: + img = Image.open(_io.BytesIO(img_data)).convert("RGB") + img.save(str(png_path), "PNG") + except Exception as e: + return {"error": f"Invalid image: {e}"} + gif_path.unlink(missing_ok=True) + return {"ok": True, "kind": "png"} + + +@router.post("/api/song/{filename:path}/art/upload") +async def upload_song_art_b64(filename: str, data: dict): + """Upload a custom cover as base64 (PNG/JPG/WebP → normalized PNG; + GIF → kept animated, local-only). The override outranks pack art in the + serve chain; remove it via DELETE …/art/override.""" + import base64 + # Reject art for a filename that doesn't resolve to a real song (mirrors the + # url route's guard) — no writing stray override files for unknown keys. + dlc = _get_dlc_dir() + song_path = _resolve_dlc_path(dlc, filename) if dlc else None + if song_path is None or not song_path.exists(): + raise HTTPException(status_code=404, detail="unknown song") + b64 = data.get("image", "") + if not b64: + return {"error": "No image data"} + # Strip data URL prefix if present + if "," in b64: + b64 = b64.split(",", 1)[1] + try: + img_data = base64.b64decode(b64) + except Exception: + return {"error": "Invalid base64"} + if len(img_data) > _ART_URL_MAX_BYTES: + raise HTTPException(status_code=400, detail="image larger than 10 MB") + return _save_art_override(filename, img_data) + + +# Art-by-URL fetch cap — a cover, not a wallpaper pack. +_ART_URL_MAX_BYTES = 10 * 1024 * 1024 + + +def _url_host_is_internal(url: str) -> bool: + """True when a user-supplied URL's host resolves to a loopback, private, + link-local, reserved, multicast or unspecified address — an SSRF target we + refuse to fetch on the user's behalf (e.g. 169.254.169.254 metadata, LAN + services). Fails CLOSED: an unresolvable or unparseable host is treated as + internal. Every resolved address must be public for the URL to pass.""" + from urllib.parse import urlparse + import socket + host = urlparse(url).hostname + if not host: + return True + try: + infos = socket.getaddrinfo(host, None) + except OSError: + return True + if not infos: + return True + for info in infos: + raw = info[4][0].split("%", 1)[0] # strip any zone id + try: + ip = ipaddress.ip_address(raw) + except ValueError: + return True + if (ip.is_private or ip.is_loopback or ip.is_link_local + or ip.is_reserved or ip.is_multicast or ip.is_unspecified): + return True + return False + + +# Art-by-URL redirect budget. Cover hosts commonly answer with a redirect — +# the Cover Art Archive (whose thumbs the cover picker applies through this +# very route) 307s every image to archive.org — so redirects must work; 5 +# hops is generous for any real CDN chain while still bounding the walk. +_ART_URL_MAX_REDIRECTS = 5 + + +def _fetch_art_url(url: str) -> bytes: + """The one place art-by-URL touches the network (tests fake this seam). + User-initiated, so not throttled like the background workers — but the + same offline guard applies (pytest can never fetch), the host is checked + against internal/reserved ranges (SSRF), redirects are followed MANUALLY + with the scheme + internal-host guard re-applied to every hop (so a + redirect can't smuggle the request to an internal target — a blanket + no-redirect rule would break every Cover Art Archive pick, which always + redirects to archive.org), and the size cap is enforced while streaming + so a huge response never fully downloads. + + Residual, accepted: each hop's host is resolved here and again by + requests, so a rebinding DNS name is a theoretical TOCTOU. Not closed + with an IP-pinned connection because (a) this is a single-user, no-auth + app (constitution §I) and the route is demo-blocked, so there is no + untrusted submission path, and (b) no other in-tree client (MusicBrainz, + CAA) pins either — a bespoke pinned+SNI adapter here would be + inconsistent and disproportionate. The cheap guards above still stop the + realistic vectors (direct internal URL, redirect-to-internal).""" + if not enrichment._enrich_network_enabled(): + raise enrichment.EnrichTransportError("art fetch disabled (offline)") + import requests + from urllib.parse import urljoin, urlparse + for _hop in range(_ART_URL_MAX_REDIRECTS + 1): + # Re-validate EVERY hop, not just the user's original URL: the whole + # point of handling redirects ourselves is that each target gets the + # same scheme + SSRF gate before any request is made. + if urlparse(url).scheme not in ("http", "https"): + raise ValueError("url must be http(s)") + if _url_host_is_internal(url): + raise ValueError("url host is not allowed") + try: + with requests.get(url, timeout=15, stream=True, allow_redirects=False, + headers={"User-Agent": enrichment._enrich_user_agent()}) as resp: + if resp.status_code in (301, 302, 303, 307, 308): + loc = resp.headers.get("Location") or "" + if not loc: + raise enrichment.EnrichTransportError( + f"HTTP {resp.status_code} without a Location") + url = urljoin(url, loc) + continue + if resp.status_code != 200: + raise enrichment.EnrichTransportError(f"HTTP {resp.status_code}") + data = b"" + for chunk in resp.iter_content(65536): + data += chunk + if len(data) > _ART_URL_MAX_BYTES: + raise ValueError("image larger than 10 MB") + return data + except requests.RequestException as e: + raise enrichment.EnrichTransportError(str(e)) from e + raise enrichment.EnrichTransportError("too many redirects") + + +@router.post("/api/song/{filename:path}/art/url") +def set_song_art_from_url(filename: str, data: dict): + """Paste-a-link cover art (the media-server idiom): the server fetches the + image and stores it as this song's local override — identical result to an + upload, including the GIF-stays-local rule. http(s) only.""" + url = str((data or {}).get("url") or "").strip() + from urllib.parse import urlparse + parsed = urlparse(url) + if parsed.scheme not in ("http", "https") or not parsed.hostname: + raise HTTPException(status_code=400, detail="url must be http(s)") + dlc = _get_dlc_dir() + song_path = _resolve_dlc_path(dlc, filename) if dlc else None + if song_path is None or not song_path.exists(): + raise HTTPException(status_code=404, detail="unknown song") + try: + img_data = _fetch_art_url(url) + except enrichment.EnrichTransportError as e: + return JSONResponse({"error": "could not fetch image", "detail": str(e)}, + status_code=502) + except ValueError as e: + raise HTTPException(status_code=400, detail=str(e)) + return _save_art_override(filename, img_data) + + +@router.delete("/api/art/{filename:path}/override") +def remove_song_art_override(filename: str): + """Drop the user art override — the serve chain falls back to pack art, + then the Cover Art Archive cache. Lives under /api/art (NOT /api/song) so + the greedy DELETE /api/song/{path} catch-all can't shadow it — the same + dodge the chart split/unsplit routes use.""" + removed = False + for p in appstate.art_override_paths(filename): + try: + p.unlink() + removed = True + except OSError: + pass + if removed: + # The art worker may have settled this row as 'user' (override present, + # no pack art). Reset it so the next enrichment pass re-evaluates and the + # CAA fallback resumes — otherwise a removed override strands the row + # (enrichment_art_pending only re-queues art_state IS NULL) and the song + # is left with no art at all. + try: + appstate.meta_db.set_enrichment_art(filename, None, None) + except Exception: + log.exception("art override delete: failed to reset enrichment state") + return {"ok": True, "removed": removed} diff --git a/server.py b/server.py index 96253e6..e33a711 100644 --- a/server.py +++ b/server.py @@ -1,7 +1,6 @@ """FeedBack — FastAPI backend serving highway viewer + library.""" import asyncio -import hashlib import json import logging import math @@ -58,6 +57,7 @@ import appstate from routers import audio_effects, artist_aliases, loops, playlists, ws_highway, chart, wanted, library_extras, shop, progression, profile, stats, version, diagnostics from routers import tunings as tunings_router import enrichment +from routers import art as art_router import sloppak as sloppak_mod import loosefolder as loosefolder_mod # Pure text-matching engine for MusicBrainz enrichment (P8): denoise/score/ @@ -409,7 +409,7 @@ class LocalLibraryProvider: } async def get_art(self, song_id: str): - return await get_song_art(song_id) + return await art_router.get_song_art(song_id) class LibraryProviderRegistry: @@ -1841,6 +1841,7 @@ appstate.configure( art_cache_dir=ART_CACHE_DIR, song_pack_art_exists=_song_pack_art_exists, art_override_paths=_art_override_paths, + art_safe_name=_art_safe_name, ) @@ -5033,287 +5034,24 @@ app.include_router(diagnostics.router) -def _if_none_match_hits(header: str | None, etag: str) -> bool: - """True if an If-None-Match header matches `etag` (weak comparison). - - Handles the `*` wildcard and comma-separated lists, and ignores a weak - `W/` prefix on either side — the standard semantics for a conditional GET. - """ - if not header: - return False - bare = etag.removeprefix("W/") - for tok in header.split(","): - t = tok.strip() - if t == "*" or t.removeprefix("W/") == bare: - return True - return False +# ── Album-art routes → routers/art.py (R3) ────────────────────────────────── +app.include_router(art_router.router) -# Album art is served with a strong validator (an ETag on the sloppak byte -# path; FileResponse's own ETag/Last-Modified on the file paths) and revalidated -# with `no-cache`. That keeps re-scroll cheap — a conditional GET returns a -# bodyless 304 — without ever serving a stale cover. A long `immutable` max-age -# was rejected: the frontend's `?v=` buster is only second-resolution, so -# a same-second cover rewrite would keep the URL and pin the old bytes for the -# cache lifetime. Validation cost is negligible for a localhost backend. -_ART_CACHE_HEADERS = {"Cache-Control": "no-cache"} -def _art_etag(path: Path) -> str | None: - """Strong validator for an art file: nanosecond mtime + size (so a - same-second rewrite still changes it). None if the file can't be stat'd.""" - try: - st = path.stat() - return f'"{st.st_mtime_ns}-{st.st_size}"' - except OSError: - return None -def _art_conditional(etag: str | None, request: Request | None): - """Return (headers, not_modified) for an art response. `not_modified` is - True when the client's If-None-Match already matches `etag` → caller should - return a bodyless 304. Starlette's FileResponse emits an ETag but does NOT - itself evaluate If-None-Match, so every art path routes through here to get - real conditional handling.""" - headers = dict(_ART_CACHE_HEADERS) - if etag: - headers["ETag"] = etag - inm = request.headers.get("if-none-match") if request is not None else None - return headers, bool(etag) and _if_none_match_hits(inm, etag) -def _file_art_response(path: Path, media_type: str, request: Request | None): - """FileResponse for an on-disk art file, with no-cache + ETag and a bodyless - 304 when the client's validator still matches.""" - headers, not_modified = _art_conditional(_art_etag(path), request) - if not_modified: - return Response(status_code=304, headers=headers) - return FileResponse(str(path), media_type=media_type, headers=headers) -@app.get("/api/song/{filename:path}/art") -async def get_song_art(filename: str, request: Request = None, source: str = ""): - """Serve album art for a song, walking the R3 override chain: - - 1. USER OVERRIDE (upload / URL-fetch, {safe_name}.gif|.png in the art - cache) — art the user explicitly pinned outranks everything, pack - art included. GIF is allowed HERE only: an animated cover is a - local-only bonus; packs stay jpg/png/webp and nothing ever writes - art into a pack file. - 2. PACK ART — sloppak cover (single member read, no full unpack) or - the loose folder's discovered image. - 3. COVER ART ARCHIVE cache — fetched by the enrichment art worker for - matched songs that lack pack art, keyed by release MBID. - - `?source=pack` narrows the chain to step 2 only (no override, no CAA): - the cover picker's "Pack original" tile must show the pack's own art - even while a user override is what the plain route serves. 404 when the - song ships no art of its own. - """ - dlc = _get_dlc_dir() - if not dlc: - return JSONResponse({"error": "not configured"}, 404) - - song_path = _resolve_dlc_path(dlc, filename) - if song_path is None: - return JSONResponse({"error": "forbidden"}, 403) - if not song_path.exists(): - return JSONResponse({"error": "not found"}, 404) - - pack_only = source == "pack" - - # 1. User override — GIF first (it wins over a stale PNG override). - if not pack_only: - for cached in _art_override_paths(filename): - mt = "image/gif" if cached.suffix == ".gif" else "image/png" - return _file_art_response(cached, mt, request) - - # 2a. Sloppak: read the cover (manifest-declared or default) straight from - # the package. For a zip-form sloppak this opens just the cover member — - # NOT the whole archive — so the library grid never triggers a full unpack - # of stems just to paint a thumbnail. - if sloppak_mod.is_sloppak(song_path): - # Read the cover (cheap — single member, no full unpack) and validate by - # its CONTENT. A stat-based ETag would be wrong for directory-form - # sloppaks: editing cover.jpg in place changes the file's mtime, not the - # directory's, so a dir-stat ETag could emit a stale 304. Content hashing - # is correct for both dir- and zip-form. Raw byte Response lacks - # FileResponse's validators, so we attach the ETag + honor If-None-Match. - try: - art = await asyncio.to_thread(sloppak_mod.read_cover_bytes, song_path) - except Exception: - art = None - if art is not None: - data, mt = art - etag = f'"{hashlib.sha1(data).hexdigest()}"' - headers, not_modified = _art_conditional(etag, request) - if not_modified: - return Response(status_code=304, headers=headers) - return Response(content=data, media_type=mt, headers=headers) - - # 2b. Loose folder: serve the discovered art file directly. - # song_path is already validated against DLC_DIR by _resolve_dlc_path. - elif loosefolder_mod.is_loose_song(song_path): - art_path = loosefolder_mod.find_art(song_path) - if art_path: - # Re-resolve in case the matched file is a symlink — a crafted - # custom song could put `album_art.jpg` as a symlink to anywhere on - # disk. Insist the final target stays inside the song folder. - art_resolved = art_path.resolve() - try: - art_resolved.relative_to(song_path) - except ValueError: - return JSONResponse({"error": "forbidden"}, 403) - if art_resolved.is_file(): - mt = { - ".jpg": "image/jpeg", ".jpeg": "image/jpeg", - ".png": "image/png", ".webp": "image/webp", - }.get(art_resolved.suffix.lower(), "image/jpeg") - return _file_art_response(art_resolved, mt, request) - - # 3. Cover Art Archive cache (the enrichment art worker's fetch). - if not pack_only: - row = meta_db.get_enrichment(filename) - if row and row.get("art_state") == "caa" and row.get("art_cache_path"): - caa = Path(row["art_cache_path"]) - if caa.is_file(): - return _file_art_response(caa, "image/jpeg", request) - - return JSONResponse({"error": "no art"}, 404) -# ── Cover picker (PR-C): candidate assembly ─────────────────────────────────── -# Enumerated ON OPEN, never at scan time (charrette §8), and NO image bytes -# are fetched here — Cover Art Archive release INDEX jsons only (1-3 throttled -# calls on a cache miss); the tiles' thumbnails load straight from the archive -# in the client. Applying a pick never grows a new write path: the client -# POSTs the chosen thumb URL to the EXISTING …/art/url route (the override -# lane — never evicted, survives a re-match), "Pack original" DELETEs the -# override, uploads keep the existing upload route. -_ART_PICKER_MAX_CAA = 12 -@app.get("/api/song/{filename:path}/art/cover-search") -def api_art_cover_search(filename: str, q: str = ""): - """Search Cover Art Archive (via MusicBrainz release-groups) for album covers - — powers the Change-cover picker's search box, so a cover can be found even - for a song with no metadata match (the unmatched city-pop pile, where - /art/candidates is empty). `q` defaults to the song's own artist + album/ - title (romaji fallback applied). Read-only; the picker renders the thumbs and - applies a pick through the existing /art/url route.""" - query = (q or "").strip() - if not query: - pack = meta_db.pack_fields(meta_db._canonical_song_filename(filename)) - query = " ".join(x for x in (pack.get("artist"), pack.get("album") or pack.get("title")) if x).strip() - if not query: - return {"query": "", "covers": []} - try: - return {"query": query, "covers": enrichment._mb_search_release_groups(query, limit=8)} - except enrichment.EnrichTransportError: - return {"query": query, "covers": [], "error": "unavailable"} -@app.get("/api/song/{filename:path}/art/candidates") -def get_song_art_candidates(filename: str): - """Everything the cover picker can offer for one song, without fetching a - single image: the current cover (with its provenance), the pack original - when the song ships art, and CAA candidates for the matched/manual - release plus any distinct releases among the stored review candidates. - Sync route on purpose (the CAA index fetch sleeps in the shared - throttle — FastAPI runs `def` routes in the threadpool). One response, - `pending` always False — the client shows a spinner for the request's own - latency; offline / CAA-down just means an empty caa tail (the instant - tiles keep working), never an error.""" - from urllib.parse import quote - dlc = _get_dlc_dir() - song_path = _resolve_dlc_path(dlc, filename) if dlc else None - if song_path is None or not song_path.exists(): - raise HTTPException(status_code=404, detail="unknown song") - - row = meta_db.get_enrichment(filename) or {} - has_pack = _song_pack_art_exists(filename) - art_url = f"/api/song/{quote(filename)}/art" - - # What the plain art route would serve right now — the serve chain's - # order (override > pack > CAA cache) restated as provenance. - if _art_override_paths(filename): - provenance = "yours" - elif has_pack: - provenance = "pack" - elif row.get("art_state") == "caa" and row.get("art_cache_path"): - provenance = "matched" - else: - provenance = "none" - - candidates: list[dict] = [{ - "id": "current", "kind": "current", "label": "Current", - "thumb_url": art_url, "provenance": provenance, - }] - if has_pack: - candidates.append({ - "id": "pack", "kind": "pack", "label": "Pack original", - "thumb_url": art_url + "?source=pack", "provenance": "pack", - }) - - # Releases worth asking the archive about: the matched/manual release - # first (it seeds the best candidates), then any distinct release among - # the stored review candidates (a review row has no mb_release_id of its - # own — its releases live in the candidates JSON). - # Only spend the shared CAA rate budget on rows whose match warrants it: - # a matched/manual release seeds the best candidates, and a review row's - # stored candidates are still live proposals. A failed/rejected (or - # unscanned) row has no accepted match — asking would burn the budget and - # surface releases already rejected as non-matches. The Current + Pack - # tiles above serve regardless, so those songs still get a picker. - rids: list[str] = [] - if row.get("match_state") in ("matched", "manual", "review"): - if row.get("match_state") in ("matched", "manual") and row.get("mb_release_id"): - rids.append(str(row["mb_release_id"])) - for cand in (row.get("candidates") or []): - rid = str(cand.get("release_id") or "") if isinstance(cand, dict) else "" - if rid and rid not in rids: - rids.append(rid) - - caa_entries: list[dict] = [] - for rid in rids: - if len(caa_entries) >= _ART_PICKER_MAX_CAA: - break - try: - imgs = enrichment._caa_index_cached(rid) - except enrichment.EnrichTransportError: - # Offline / archive down — stop asking (each further miss would - # only burn a timeout). The instant tiles still serve; a later - # picker-open retries naturally (failures are never cached). - break - # Front covers first, approved before pending, otherwise index order - # (the picker grammar is a RANKED list — §7/§9). - def _rank(img): - types = img.get("types") or [] - is_front = bool(img.get("front")) or "Front" in types - return (not is_front, not bool(img.get("approved"))) - for img in sorted((i for i in imgs if isinstance(i, dict)), key=_rank): - if len(caa_entries) >= _ART_PICKER_MAX_CAA: - break - thumbs = img.get("thumbnails") or {} - if not isinstance(thumbs, dict): - continue - thumb = (thumbs.get("500") or thumbs.get("large") - or thumbs.get("250") or thumbs.get("small")) - if not thumb: - continue - types = [str(t) for t in (img.get("types") or []) if isinstance(t, str)] - caa_entries.append({ - "id": f"caa-{rid}-{img.get('id', '')}", - "kind": "caa", - "label": ", ".join(types) or "Cover", - "thumb_url": str(thumb), - "provenance": "matched", - "types": types, - "approved": bool(img.get("approved")), - "release_id": rid, - }) - - return {"candidates": candidates + caa_entries, "pending": False} @app.post("/api/song/{filename:path}/meta") @@ -5559,207 +5297,20 @@ def post_song_gap_fill(filename: str, data: dict): return {"ok": True, "written": additions, "skipped": skipped} -def _save_art_override(filename: str, img_data: bytes) -> dict: - """Persist a user art override into the art cache (R3). One override per - song: GIF input is validated and kept VERBATIM as .gif (animation intact — - the local-only bonus; it is never written into the pack file), everything - else is normalized to RGB PNG via PIL. Saving either kind removes the - other so the serve chain has exactly one user file to find.""" - ART_CACHE_DIR.mkdir(parents=True, exist_ok=True) - stem = _art_safe_name(filename) - png_path = ART_CACHE_DIR / f"{stem}.png" - gif_path = ART_CACHE_DIR / f"{stem}.gif" - from PIL import Image - import io as _io - if img_data[:6] in (b"GIF87a", b"GIF89a"): - try: - probe = Image.open(_io.BytesIO(img_data)) - probe.verify() # decodes headers/frames without keeping the image - if probe.format != "GIF": - raise ValueError("not a GIF") - except Exception as e: - return {"error": f"Invalid image: {e}"} - gif_path.write_bytes(img_data) - png_path.unlink(missing_ok=True) - return {"ok": True, "kind": "gif"} - try: - img = Image.open(_io.BytesIO(img_data)).convert("RGB") - img.save(str(png_path), "PNG") - except Exception as e: - return {"error": f"Invalid image: {e}"} - gif_path.unlink(missing_ok=True) - return {"ok": True, "kind": "png"} -@app.post("/api/song/{filename:path}/art/upload") -async def upload_song_art_b64(filename: str, data: dict): - """Upload a custom cover as base64 (PNG/JPG/WebP → normalized PNG; - GIF → kept animated, local-only). The override outranks pack art in the - serve chain; remove it via DELETE …/art/override.""" - import base64 - # Reject art for a filename that doesn't resolve to a real song (mirrors the - # url route's guard) — no writing stray override files for unknown keys. - dlc = _get_dlc_dir() - song_path = _resolve_dlc_path(dlc, filename) if dlc else None - if song_path is None or not song_path.exists(): - raise HTTPException(status_code=404, detail="unknown song") - b64 = data.get("image", "") - if not b64: - return {"error": "No image data"} - # Strip data URL prefix if present - if "," in b64: - b64 = b64.split(",", 1)[1] - try: - img_data = base64.b64decode(b64) - except Exception: - return {"error": "Invalid base64"} - if len(img_data) > _ART_URL_MAX_BYTES: - raise HTTPException(status_code=400, detail="image larger than 10 MB") - return _save_art_override(filename, img_data) -# Art-by-URL fetch cap — a cover, not a wallpaper pack. -_ART_URL_MAX_BYTES = 10 * 1024 * 1024 -def _url_host_is_internal(url: str) -> bool: - """True when a user-supplied URL's host resolves to a loopback, private, - link-local, reserved, multicast or unspecified address — an SSRF target we - refuse to fetch on the user's behalf (e.g. 169.254.169.254 metadata, LAN - services). Fails CLOSED: an unresolvable or unparseable host is treated as - internal. Every resolved address must be public for the URL to pass.""" - from urllib.parse import urlparse - import socket - host = urlparse(url).hostname - if not host: - return True - try: - infos = socket.getaddrinfo(host, None) - except OSError: - return True - if not infos: - return True - for info in infos: - raw = info[4][0].split("%", 1)[0] # strip any zone id - try: - ip = ipaddress.ip_address(raw) - except ValueError: - return True - if (ip.is_private or ip.is_loopback or ip.is_link_local - or ip.is_reserved or ip.is_multicast or ip.is_unspecified): - return True - return False -# Art-by-URL redirect budget. Cover hosts commonly answer with a redirect — -# the Cover Art Archive (whose thumbs the cover picker applies through this -# very route) 307s every image to archive.org — so redirects must work; 5 -# hops is generous for any real CDN chain while still bounding the walk. -_ART_URL_MAX_REDIRECTS = 5 -def _fetch_art_url(url: str) -> bytes: - """The one place art-by-URL touches the network (tests fake this seam). - User-initiated, so not throttled like the background workers — but the - same offline guard applies (pytest can never fetch), the host is checked - against internal/reserved ranges (SSRF), redirects are followed MANUALLY - with the scheme + internal-host guard re-applied to every hop (so a - redirect can't smuggle the request to an internal target — a blanket - no-redirect rule would break every Cover Art Archive pick, which always - redirects to archive.org), and the size cap is enforced while streaming - so a huge response never fully downloads. - - Residual, accepted: each hop's host is resolved here and again by - requests, so a rebinding DNS name is a theoretical TOCTOU. Not closed - with an IP-pinned connection because (a) this is a single-user, no-auth - app (constitution §I) and the route is demo-blocked, so there is no - untrusted submission path, and (b) no other in-tree client (MusicBrainz, - CAA) pins either — a bespoke pinned+SNI adapter here would be - inconsistent and disproportionate. The cheap guards above still stop the - realistic vectors (direct internal URL, redirect-to-internal).""" - if not enrichment._enrich_network_enabled(): - raise enrichment.EnrichTransportError("art fetch disabled (offline)") - import requests - from urllib.parse import urljoin, urlparse - for _hop in range(_ART_URL_MAX_REDIRECTS + 1): - # Re-validate EVERY hop, not just the user's original URL: the whole - # point of handling redirects ourselves is that each target gets the - # same scheme + SSRF gate before any request is made. - if urlparse(url).scheme not in ("http", "https"): - raise ValueError("url must be http(s)") - if _url_host_is_internal(url): - raise ValueError("url host is not allowed") - try: - with requests.get(url, timeout=15, stream=True, allow_redirects=False, - headers={"User-Agent": enrichment._enrich_user_agent()}) as resp: - if resp.status_code in (301, 302, 303, 307, 308): - loc = resp.headers.get("Location") or "" - if not loc: - raise enrichment.EnrichTransportError( - f"HTTP {resp.status_code} without a Location") - url = urljoin(url, loc) - continue - if resp.status_code != 200: - raise enrichment.EnrichTransportError(f"HTTP {resp.status_code}") - data = b"" - for chunk in resp.iter_content(65536): - data += chunk - if len(data) > _ART_URL_MAX_BYTES: - raise ValueError("image larger than 10 MB") - return data - except requests.RequestException as e: - raise enrichment.EnrichTransportError(str(e)) from e - raise enrichment.EnrichTransportError("too many redirects") -@app.post("/api/song/{filename:path}/art/url") -def set_song_art_from_url(filename: str, data: dict): - """Paste-a-link cover art (the media-server idiom): the server fetches the - image and stores it as this song's local override — identical result to an - upload, including the GIF-stays-local rule. http(s) only.""" - url = str((data or {}).get("url") or "").strip() - from urllib.parse import urlparse - parsed = urlparse(url) - if parsed.scheme not in ("http", "https") or not parsed.hostname: - raise HTTPException(status_code=400, detail="url must be http(s)") - dlc = _get_dlc_dir() - song_path = _resolve_dlc_path(dlc, filename) if dlc else None - if song_path is None or not song_path.exists(): - raise HTTPException(status_code=404, detail="unknown song") - try: - img_data = _fetch_art_url(url) - except enrichment.EnrichTransportError as e: - return JSONResponse({"error": "could not fetch image", "detail": str(e)}, - status_code=502) - except ValueError as e: - raise HTTPException(status_code=400, detail=str(e)) - return _save_art_override(filename, img_data) -@app.delete("/api/art/{filename:path}/override") -def remove_song_art_override(filename: str): - """Drop the user art override — the serve chain falls back to pack art, - then the Cover Art Archive cache. Lives under /api/art (NOT /api/song) so - the greedy DELETE /api/song/{path} catch-all can't shadow it — the same - dodge the chart split/unsplit routes use.""" - removed = False - for p in _art_override_paths(filename): - try: - p.unlink() - removed = True - except OSError: - pass - if removed: - # The art worker may have settled this row as 'user' (override present, - # no pack art). Reset it so the next enrichment pass re-evaluates and the - # CAA fallback resumes — otherwise a removed override strands the row - # (enrichment_art_pending only re-queues art_state IS NULL) and the song - # is left with no art at all. - try: - meta_db.set_enrichment_art(filename, None, None) - except Exception: - log.exception("art override delete: failed to reset enrichment state") - return {"ok": True, "removed": removed} @app.get("/api/song/{filename:path}") diff --git a/tests/test_art_candidates.py b/tests/test_art_candidates.py index 7f80ccf..7c73ae5 100644 --- a/tests/test_art_candidates.py +++ b/tests/test_art_candidates.py @@ -13,6 +13,7 @@ tests/test_art_layer.py. import importlib import enrichment +from routers import art import io as _io import sys @@ -252,7 +253,7 @@ def test_caa_candidates_capped_at_12(server, client, caa_index): caa_index.indexes["rel-big"] = { "images": [_img(300 + i, front=(i == 0)) for i in range(20)]} _match_row(server, "a.sloppak", release_id="rel-big") - assert len(_caa(_get(client))) == server._ART_PICKER_MAX_CAA == 12 + assert len(_caa(_get(client))) == art._ART_PICKER_MAX_CAA == 12 def test_demo_mode_blocks_candidates(server, client, monkeypatch): @@ -342,9 +343,9 @@ def test_fetch_art_url_follows_redirects_validating_each_hop(server, monkeypatch monkeypatch.setattr(requests, "get", fake_get) monkeypatch.setattr(enrichment, "_enrich_network_enabled", lambda: True) - monkeypatch.setattr(server, "_url_host_is_internal", + monkeypatch.setattr(art, "_url_host_is_internal", lambda u: (checked.append(u), False)[1]) - data = server._fetch_art_url("https://coverartarchive.example/release/x/front-500") + data = art._fetch_art_url("https://coverartarchive.example/release/x/front-500") assert data == b"IMGDATA" assert fetched == ["https://coverartarchive.example/release/x/front-500", "https://archive.example/img.png"] @@ -356,10 +357,10 @@ def test_fetch_art_url_blocks_redirect_to_internal(server, monkeypatch): monkeypatch.setattr(requests, "get", lambda url, **kw: _FakeResp( 302, {"Location": "http://internal.example/x.png"})) monkeypatch.setattr(enrichment, "_enrich_network_enabled", lambda: True) - monkeypatch.setattr(server, "_url_host_is_internal", + monkeypatch.setattr(art, "_url_host_is_internal", lambda u: "internal" in u) with pytest.raises(ValueError): - server._fetch_art_url("https://public.example/x.png") + art._fetch_art_url("https://public.example/x.png") def test_fetch_art_url_redirect_budget(server, monkeypatch): @@ -367,6 +368,6 @@ def test_fetch_art_url_redirect_budget(server, monkeypatch): monkeypatch.setattr(requests, "get", lambda url, **kw: _FakeResp( 307, {"Location": "https://public.example/next.png"})) monkeypatch.setattr(enrichment, "_enrich_network_enabled", lambda: True) - monkeypatch.setattr(server, "_url_host_is_internal", lambda u: False) + monkeypatch.setattr(art, "_url_host_is_internal", lambda u: False) with pytest.raises(enrichment.EnrichTransportError): - server._fetch_art_url("https://public.example/x.png") + art._fetch_art_url("https://public.example/x.png") diff --git a/tests/test_art_layer.py b/tests/test_art_layer.py index 0bb322b..8e506c7 100644 --- a/tests/test_art_layer.py +++ b/tests/test_art_layer.py @@ -8,6 +8,7 @@ here opens a socket, and the offline default is itself asserted. import importlib import enrichment +from routers import art import io as _io import sys @@ -122,7 +123,7 @@ def test_bad_upload_rejected(server, client): def test_art_url_fetches_and_overrides(server, client, monkeypatch): make_sloppak(server, "a.sloppak", with_cover=True) - monkeypatch.setattr(server, "_fetch_art_url", lambda url: png_bytes((9, 9, 9))) + monkeypatch.setattr(art, "_fetch_art_url", lambda url: png_bytes((9, 9, 9))) body = client.post("/api/song/a.sloppak/art/url", json={"url": "https://example.com/cover.png"}).json() assert body == {"ok": True, "kind": "png"} @@ -139,7 +140,7 @@ def test_art_url_validation(server, client, monkeypatch): # Oversize → 400 (the seam raises ValueError at the cap). def _huge(url): raise ValueError("image larger than 10 MB") - monkeypatch.setattr(server, "_fetch_art_url", _huge) + monkeypatch.setattr(art, "_fetch_art_url", _huge) assert client.post("/api/song/a.sloppak/art/url", json={"url": "https://example.com/x.png"}).status_code == 400 @@ -303,7 +304,7 @@ def test_upload_rejects_unknown_song_and_oversize(server, client): assert server._art_override_paths("ghost.sloppak") == [] # Oversize decoded payload → 400 (bounds the base64 upload path). make_sloppak(server, "a.sloppak") - huge = b64(b"\x00" * (server._ART_URL_MAX_BYTES + 1)) + huge = b64(b"\x00" * (art._ART_URL_MAX_BYTES + 1)) assert client.post("/api/song/a.sloppak/art/upload", json={"image": huge}).status_code == 400 @@ -311,10 +312,10 @@ def test_upload_rejects_unknown_song_and_oversize(server, client): def test_fetch_art_url_blocks_internal_hosts(server): """The SSRF guard refuses loopback / link-local / private targets before any request is made (the real seam, not the faked one).""" - assert server._url_host_is_internal("http://127.0.0.1/x.png") - assert server._url_host_is_internal("http://localhost/x.png") - assert server._url_host_is_internal("http://169.254.169.254/latest/meta-data") - assert server._url_host_is_internal("http://10.0.0.5/x.png") - assert server._url_host_is_internal("http://[::1]/x.png") - assert server._url_host_is_internal("http://nonexistent.invalid/x.png") # unresolvable → closed - assert not server._url_host_is_internal("http://93.184.216.34/x.png") # public literal + assert art._url_host_is_internal("http://127.0.0.1/x.png") + assert art._url_host_is_internal("http://localhost/x.png") + assert art._url_host_is_internal("http://169.254.169.254/latest/meta-data") + assert art._url_host_is_internal("http://10.0.0.5/x.png") + assert art._url_host_is_internal("http://[::1]/x.png") + assert art._url_host_is_internal("http://nonexistent.invalid/x.png") # unresolvable → closed + assert not art._url_host_is_internal("http://93.184.216.34/x.png") # public literal