got-feedBack/feedBack
1
0
Fork 0
mirror of https://github.com/got-feedBack/feedBack.git synced 2026-07-26 23:01:22 +00:00
Code Issues Actions 1 Packages Projects Releases Wiki Activity

library: cover-art layer — CAA auto-fetch + user overrides, GIF local-only (P9) (#715)

Browse Source 
* library: cover-art layer — CAA auto-fetch + user overrides, GIF local-only (R3/P9)

Third slice of the enrichment series (stacked on the matcher): covers.

- Serve chain for GET /api/song/{fn}/art: USER OVERRIDE -> PACK ART ->
  COVER ART ARCHIVE cache -> 404. Behaviour change, deliberate: a
  user-uploaded cover now OVERRIDES pack art (previously the upload only
  filled the no-art gap, which made custom art look broken on any song
  that already shipped a cover).
- GIF is allowed as an override and kept VERBATIM (animation intact) —
  a local-only bonus. Everything else normalizes to RGB PNG as before.
  One override per song (saving either kind removes the other), and
  nothing ever writes art INTO a pack file — test-pinned: the pack's
  cover.jpg is byte-identical after a GIF upload.
- Art by URL: POST /api/song/{fn}/art/url fetches server-side (http(s)
  only, 10 MB cap enforced while streaming) into the same override slot.
  DELETE /api/art/{fn}/override drops it — under /api/art because the
  greedy DELETE /api/song/{path} catch-all shadows anything beneath it
  (the same dodge the chart split/unsplit routes use).
- Cover Art Archive fetch as phase 3 of the enrichment pass: matched
  songs that LACK pack art get their release's front cover, throttled +
  identified + offline-guarded exactly like the MusicBrainz client
  (pytest can never reach the network; a transport error pauses the
  pass without burning the row). The cache is keyed by RELEASE MBID —
  ten charts of one album cost one fetch — and every outcome writes an
  art_state (pack/user/caa/none/error) so a row is evaluated once.
- LRU cap (200 MB) on the CAA side of the cache only; user overrides
  are never evicted, and evicted rows reset so a later pass may
  re-fetch. Deleting a song removes its override files (CAA files stay
  — they may be shared by other charts of the release).

No frontend changes: the grid, the review modal, and the player pick
the new art up through the same route they already use. The
upload/paste-a-link surfaces in the Details drawer land with the
context-menu slice once the drawer PR merges.

13 new tests (tests/test_art_layer.py) + demo-mode routes; full-suite
failure set byte-identical with the change stashed vs applied.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Nm7tHs1Yvjjtnnu4nzJgdN

* library: harden cover-art layer — SSRF guard, demo/size caps, override-delete state reset

Follow-up hardening on the R3 cover-art layer:

- remove_song_art_override: reset the enrichment row (set_enrichment_art(fn,
  None, None)) when an override is deleted, so a row previously settled as
  'user' re-queues and the CAA fallback resumes. Previously a removed override
  stranded the row (enrichment_art_pending only re-queues art_state IS NULL),
  leaving the song with no art at all.
- Base64 art upload: block it in demo mode (was open — a write/disk-fill vector,
  worse now that GIFs are stored verbatim), validate the filename resolves to a
  real song (mirrors the url route), and cap the decoded payload at 10 MB.
- Art-by-URL: reject hosts that resolve to loopback/private/link-local/reserved/
  multicast/unspecified addresses (SSRF, e.g. cloud metadata) and stop following
  redirects (allow_redirects=False) so a redirect can't smuggle the request to an
  internal target. Fails closed on unresolvable/unparseable hosts.
- _caa_http_get: stream with a per-file 10 MB cap (bounds any one response
  independently of the aggregate LRU); guard release_id against a conservative
  token before interpolating it into a cache-file path (no separators/dots).

Tests: delete-override→CAA-fallback, upload unknown-song/oversize rejection,
SSRF internal-host guard, and a demo-mode block assertion for art/upload.

Note: art_state='error' rows are intentionally not auto-retried — there is no
per-row attempt counter on the art side, so an unbounded retry could storm CAA
for permanently-bad rows; a bounded retry would need extra state, left out here.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: byrongamatos <xasiklas@gmail.com>
This commit is contained in:
ChrisBeWithYou 2026-07-02 13:55:43 -05:00 committed by GitHub
parent 8e953e8bc4
commit 7ca736d525
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 736 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

452
server.py
View File

@ -246,6 +246,12 @@ _DEMO_BLOCKED: list[tuple[str, re.Pattern]] = [
("GET", re.compile(r"^/api/chart/.+/fileinfo$")),
# Gap-fill (R4a) rewrites pack files on disk — never for demo visitors.
("POST", re.compile(r"^/api/song/.+/gap-fill$")),
# Art layer (R3): all three mutate server state / touch the network on a
# visitor's behalf — the base64 upload writes files, the URL fetch makes the
# server request arbitrary images, and the override delete removes files.
("POST", re.compile(r"^/api/song/.+/art/upload$")),
("POST", re.compile(r"^/api/song/.+/art/url$")),
("DELETE", re.compile(r"^/api/art/.+/override$")),
]
@ -2902,6 +2908,41 @@ class MetadataDB:
"candidates": candidates, "attempts": attempts or 0})
return out
def enrichment_art_pending(self, limit: int = 500) -> list[dict]:
"""Matched songs whose cover-art situation hasn't been evaluated yet
(art_state NULL). The art worker resolves each to 'pack' (song has its
own art), 'user' (an override exists), 'caa' (fetched), 'none' (the
release has no cover) or 'error' — any of which settles the row, so
this never re-offers a song each pass."""
rows = self.conn.execute(
"SELECT e.filename, e.mb_release_id "
"FROM song_enrichment e JOIN songs s ON s.filename = e.filename "
"WHERE e.match_state IN ('matched', 'manual') "
"AND e.mb_release_id IS NOT NULL AND e.art_state IS NULL "
"ORDER BY e.filename LIMIT ?", (max(1, int(limit)),)).fetchall()
return [{"filename": r[0], "mb_release_id": r[1]} for r in rows]
def set_enrichment_art(self, filename: str, path: str | None, state: str | None) -> None:
"""Stamp a row's art-cache outcome. Targeted UPDATE (not the match
writer) so it can never disturb the match lifecycle fields."""
with self._lock:
self.conn.execute(
"UPDATE song_enrichment SET art_cache_path = ?, art_state = ? "
"WHERE filename = ?", (path, state, filename))
self.conn.commit()
def clear_enrichment_art_paths(self, paths: list[str]) -> None:
"""Reset rows whose cached art file was evicted (LRU prune) back to
unevaluated, so a later pass may re-fetch if the song still qualifies."""
if not paths:
return
with self._lock:
ph = ",".join("?" * len(paths))
self.conn.execute(
f"UPDATE song_enrichment SET art_cache_path = NULL, art_state = NULL "
f"WHERE art_cache_path IN ({ph})", paths)
self.conn.commit()
def _estd_set(self) -> set[str]:
"""Get set of filenames that have a retuned variant (_EStd_ or _DropD_) in the DB."""
rows = self.conn.execute(
@ -5742,6 +5783,154 @@ def _enrich_backoff_elapsed(attempts, last_attempt_at, now: float) -> bool:
# handful is noise the user has to scroll past.
_ENRICH_MAX_CANDIDATES = 5
# ── Cover art (R3/P9) ─────────────────────────────────────────────────────────
# The art cache dir (CONFIG_DIR/art_cache) holds two kinds of file:
# {safe_name}.png / .gif — USER OVERRIDES (upload or URL-fetch; never
# evicted, removed only with the song or by the
# explicit remove-override route)
# caa_{release_mbid}.jpg — COVER ART ARCHIVE fetches, keyed by release so
# every chart of the same release shares one file;
# size-capped LRU (evictions reset the enrichment
# rows so a later pass may re-fetch)
_CAA_CACHE_CAP_BYTES = 200 * 1024 * 1024
# Per-cover cap on a single CAA fetch. The 500px thumbnail is normally tens of
# KB; this bounds any one response independently of the aggregate LRU cap so a
# single oversized (or misbehaving) release can't blow up memory/disk.
_CAA_MAX_BYTES = 10 * 1024 * 1024
# A release MBID is a UUID; before interpolating it into a cache-file path we
# require a conservative token (alphanumerics, hyphen, underscore only) so no
# separator or '.' can ever appear — blocks path traversal. Defence in depth:
# cheap even though the DB only ever holds MusicBrainz UUIDs. (Distinct name
# from the strict recording-MBID _MBID_RE above — this only gates a filename.)
_CAA_ID_RE = re.compile(r"^[A-Za-z0-9_-]{1,128}$")
def _caa_http_get(release_id: str) -> bytes | None:
"""Fetch a release's front cover from the Cover Art Archive — the one
network seam of the art layer (tests fake exactly this). Same etiquette
as the MusicBrainz client: throttled, identified, offline-guarded.
Returns the image bytes, None when the release has no cover (404), and
raises EnrichTransportError for anything network-shaped."""
if not _enrich_network_enabled():
raise EnrichTransportError("enrichment network disabled")
import requests
_enrich_throttle()
try:
with requests.get(
f"https://coverartarchive.org/release/{release_id}/front-500",
headers={"User-Agent": _enrich_user_agent()},
timeout=15, allow_redirects=True, stream=True,
) as resp:
if resp.status_code == 404:
return None
if resp.status_code != 200:
raise EnrichTransportError(f"cover art archive HTTP {resp.status_code}")
# Stream with a per-file cap so a huge response never fully downloads.
data = b""
for chunk in resp.iter_content(65536):
data += chunk
if len(data) > _CAA_MAX_BYTES:
# Not network-shaped: settle just this row as 'error' (the
# art loop's generic handler) rather than pausing the pass.
raise ValueError("cover art exceeds size cap")
return data
except requests.RequestException as e:
raise EnrichTransportError(str(e)) from e
def _art_safe_name(filename: str) -> str:
"""The flattened cache-file stem the art routes key user overrides on
(matches the legacy /art/upload naming, so old uploads keep working)."""
return filename.replace("/", "_").replace(" ", "_")
def _art_override_paths(filename: str) -> list[Path]:
"""Existing user-override art files for a song, GIF first (it wins —
the animated local-only bonus outranks a stale PNG)."""
stem = _art_safe_name(filename)
return [p for p in (ART_CACHE_DIR / f"{stem}.gif", ART_CACHE_DIR / f"{stem}.png")
if p.is_file()]
def _song_pack_art_exists(filename: str) -> bool:
"""Whether the song carries its own art (sloppak cover / loose-folder
image). Pack art always outranks a CAA fetch, so the art worker marks
these and never spends a request on them."""
try:
dlc = _get_dlc_dir()
if not dlc:
return False
p = _resolve_dlc_path(dlc, filename)
if p is None or not p.exists():
return False
if sloppak_mod.is_sloppak(p):
return sloppak_mod.read_cover_bytes(p) is not None
if loosefolder_mod.is_loose_song(p):
return loosefolder_mod.find_art(p) is not None
except Exception:
pass
return False
def _prune_caa_cache() -> None:
"""Keep the CAA side of the art cache under its size cap: evict the
oldest caa_* files (mtime LRU) and reset the enrichment rows that pointed
at them. User-override files are never touched."""
try:
files = sorted(ART_CACHE_DIR.glob("caa_*.jpg"), key=lambda p: p.stat().st_mtime)
total = sum(p.stat().st_size for p in files)
evicted: list[str] = []
while files and total > _CAA_CACHE_CAP_BYTES:
victim = files.pop(0)
try:
total -= victim.stat().st_size
victim.unlink()
evicted.append(str(victim))
except OSError:
break
if evicted:
meta_db.clear_enrichment_art_paths(evicted)
log.info("art cache: evicted %d cover(s) to stay under the cap", len(evicted))
except Exception:
log.exception("art cache prune failed")
def _enrich_art_one(row: dict) -> bool:
"""Resolve one matched song's cover-art situation (art worker, phase 3).
Returns True when a cover was actually fetched. Every outcome writes an
art_state so the row never re-queues:
'pack' — the song ships its own art (it wins; nothing to do)
'user' — an override exists (it wins; nothing to do)
'caa' — front cover cached (possibly deduped from an earlier fetch
of the same release — no network on that path)
'none' — the Cover Art Archive has no cover for this release
Network errors raise EnrichTransportError → the pass pauses and the row
stays unevaluated for the next kick."""
fn, release_id = row["filename"], row["mb_release_id"]
if not release_id or not _CAA_ID_RE.match(str(release_id)):
# Malformed release id — never build a cache path from it. Settle the
# row as 'error' so it isn't re-queued every pass.
meta_db.set_enrichment_art(fn, None, "error")
return False
if _song_pack_art_exists(fn):
meta_db.set_enrichment_art(fn, None, "pack")
return False
if _art_override_paths(fn):
meta_db.set_enrichment_art(fn, None, "user")
return False
cache_file = _enrichment_art_dir() / f"caa_{release_id}.jpg"
if cache_file.is_file():
meta_db.set_enrichment_art(fn, str(cache_file), "caa")
return False
data = _caa_http_get(release_id)
if data is None:
meta_db.set_enrichment_art(fn, None, "none")
return False
cache_file.write_bytes(data)
meta_db.set_enrichment_art(fn, str(cache_file), "caa")
_prune_caa_cache()
return True
def _enrich_one(row: dict, auto_min: float | None = None) -> None:
"""The matcher (P8; replaces P7's no-op). Precedence per design §5:
@ -5879,6 +6068,32 @@ def _background_enrich():
if pending or retriable:
log.info("Enrichment pass: %d rows stamped, %d matched", len(pending), matched)
# Phase 3 — cover art (R3/P9). For freshly-matched songs, resolve the art
# situation once: songs with their own pack art (or a user override) are
# marked and skipped; the rest fetch the release's front cover from the
# Cover Art Archive into the size-capped cache. Same pause-on-transport-
# error rule as matching — a dead network never burns a row's evaluation.
try:
art_rows = meta_db.enrichment_art_pending(limit=100000)
except Exception:
log.exception("enrichment: art-pending query failed")
return
fetched = 0
for row in art_rows:
try:
fetched += 1 if _enrich_art_one(row) else 0
except EnrichTransportError as e:
log.info("enrichment: network unavailable, art pass paused (%s)", e)
break
except Exception as e:
log.warning("enrichment art failed for %s: %s", row.get("filename"), e)
try:
meta_db.set_enrichment_art(row["filename"], None, "error")
except Exception:
pass
if art_rows:
log.info("Enrichment art pass: %d evaluated, %d covers fetched", len(art_rows), fetched)
def _kick_enrich() -> bool:
"""Request an enrichment pass, single-flight + coalescing (the _kick_scan
@ -7013,6 +7228,14 @@ def delete_song(filename: str):
meta_db.conn.execute("DELETE FROM song_enrichment WHERE filename = ?", (cache_key,))
meta_db.conn.commit()
# User art overrides go with the song (CAA cache files are keyed by
# RELEASE and may be shared with other charts — the LRU owns those).
for _p in _art_override_paths(cache_key):
try:
_p.unlink()
except OSError:
pass
_invalidate_song_caches(cache_key)
log.info("Deleted song %s", cache_key)
@ -10078,14 +10301,17 @@ def _file_art_response(path: Path, media_type: str, request: Request | None):
@app.get("/api/song/{filename:path}/art")
async def get_song_art(filename: str, request: Request = None):
"""Serve album art for a song.
"""Serve album art for a song, walking the R3 override chain:
Dispatches by format and returns the appropriate media type:
- Sloppak: serves `cover.jpg` (or manifest-declared cover) read directly
from the package (the single cover member for zip-form sloppaks — no
full unpack) as JPEG/PNG/WebP.
- Loose folder: serves the discovered art file directly as
JPEG/PNG/WebP.
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.
"""
dlc = _get_dlc_dir()
if not dlc:
@ -10097,7 +10323,12 @@ async def get_song_art(filename: str, request: Request = None):
if not song_path.exists():
return JSONResponse({"error": "not found"}, 404)
# Sloppak path: read the cover (manifest-declared or default) straight from
# 1. User override — GIF first (it wins over a stale PNG override).
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.
@ -10112,18 +10343,17 @@ async def get_song_art(filename: str, request: Request = None):
art = await asyncio.to_thread(sloppak_mod.read_cover_bytes, song_path)
except Exception:
art = None
if art is None:
return JSONResponse({"error": "no art"}, 404)
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)
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)
# Loose folder path: serve art file directly.
# 2b. Loose folder: serve the discovered art file directly.
# song_path is already validated against DLC_DIR by _resolve_dlc_path.
if loosefolder_mod.is_loose_song(song_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
@ -10140,15 +10370,13 @@ async def get_song_art(filename: str, request: Request = None):
".png": "image/png", ".webp": "image/webp",
}.get(art_resolved.suffix.lower(), "image/jpeg")
return _file_art_response(art_resolved, mt, request)
return JSONResponse({"error": "no art"}, 404)
# Custom art uploaded via /art/upload is cached as PNG under ART_CACHE_DIR;
# serve it if present (works for any song the user pinned art to).
art_cache = ART_CACHE_DIR
safe_name = filename.replace("/", "_").replace(" ", "_")
cached = art_cache / f"{safe_name}.png"
if cached.exists():
return _file_art_response(cached, "image/png", request)
# 3. Cover Art Archive cache (the enrichment art worker's fetch).
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)
@ -10390,10 +10618,50 @@ 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 custom album art as base64 PNG/JPG."""
"""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"}
@ -10404,22 +10672,128 @@ async def upload_song_art_b64(filename: str, data: dict):
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_cache = ART_CACHE_DIR
art_cache.mkdir(parents=True, exist_ok=True)
safe_name = filename.replace("/", "_").replace(" ", "_")
cached = art_cache / f"{safe_name}.png"
# Convert to PNG if needed
# 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:
from PIL import Image
import io
img = Image.open(io.BytesIO(img_data)).convert("RGB")
img.save(str(cached), "PNG")
except Exception as e:
return {"error": f"Invalid image: {e}"}
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
return {"ok": True}
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 NOT followed (a
redirect can't smuggle the request to an internal target), and the size
cap is enforced while streaming so a huge response never fully downloads.
Residual, accepted: the 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 _enrich_network_enabled():
raise EnrichTransportError("art fetch disabled (offline)")
if _url_host_is_internal(url):
raise ValueError("url host is not allowed")
import requests
try:
with requests.get(url, timeout=15, stream=True, allow_redirects=False,
headers={"User-Agent": _enrich_user_agent()}) as resp:
if resp.status_code != 200:
raise 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 EnrichTransportError(str(e)) from e
@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 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}")

318
tests/test_art_layer.py Normal file
View File

@ -0,0 +1,318 @@
"""Tests for the R3 art layer: the user>pack>CAA serve chain, user overrides
(upload / URL / delete, with GIF kept animated and LOCAL-ONLY), and the
enrichment art worker's Cover Art Archive fetch + LRU cache.
Both network seams (`_caa_http_get`, `_fetch_art_url`) are faked — nothing
here opens a socket, and the offline default is itself asserted.
"""
import importlib
import io as _io
import sys
import pytest
from fastapi.testclient import TestClient
from PIL import Image
@pytest.fixture()
def server(tmp_path, monkeypatch, isolate_logging):
monkeypatch.setenv("CONFIG_DIR", str(tmp_path / "config"))
dlc = tmp_path / "dlc"
dlc.mkdir()
monkeypatch.setenv("DLC_DIR", str(dlc))
monkeypatch.setenv("FEEDBACK_SKIP_STARTUP_TASKS", "1")
sys.modules.pop("server", None)
srv = importlib.import_module("server")
try:
yield srv
finally:
conn = getattr(getattr(srv, "meta_db", None), "conn", None)
if conn is not None:
conn.close()
sys.modules.pop("server", None)
@pytest.fixture()
def client(server):
return TestClient(server.app)
def png_bytes(color=(200, 30, 30)):
buf = _io.BytesIO()
Image.new("RGB", (4, 4), color).save(buf, "PNG")
return buf.getvalue()
def gif_bytes():
"""A tiny 2-frame animated GIF."""
buf = _io.BytesIO()
frames = [Image.new("P", (4, 4), i) for i in (0, 255)]
frames[0].save(buf, "GIF", save_all=True, append_images=frames[1:], duration=100)
return buf.getvalue()
def make_sloppak(server, name, with_cover=False, title="Song", artist="Artist"):
d = server.DLC_DIR / name
d.mkdir(parents=True)
(d / "manifest.yaml").write_text(
f"title: {title}\nartist: {artist}\nduration: 100\n"
"arrangements: []\nstems: []\n", encoding="utf-8")
if with_cover:
(d / "cover.jpg").write_bytes(png_bytes((10, 200, 10)))
server.meta_db.put(name, 0, 0, {
"title": title, "artist": artist, "album": "", "year": "",
"duration": 100, "arrangements": [{"name": "Lead", "index": 0}],
})
return d
def b64(data):
import base64
return base64.b64encode(data).decode()
# ── the serve chain: user > pack > CAA ────────────────────────────────────────
def test_user_override_beats_pack_art(server, client):
make_sloppak(server, "a.sloppak", with_cover=True)
r = client.get("/api/song/a.sloppak/art")
assert r.status_code == 200 # pack art serves
assert client.post("/api/song/a.sloppak/art/upload",
json={"image": b64(png_bytes((1, 2, 3)))}).json()["ok"]
r = client.get("/api/song/a.sloppak/art")
assert r.status_code == 200
assert r.headers["content-type"] == "image/png" # the override, not the pack jpeg
# Removing the override falls back to pack art. (The route lives under
# /api/art — the DELETE /api/song/{path} catch-all would shadow it.)
assert client.delete("/api/art/a.sloppak/override").json()["removed"]
r = client.get("/api/song/a.sloppak/art")
assert r.status_code == 200
assert r.headers["content-type"] == "image/jpeg"
def test_gif_override_kept_animated_and_local_only(server, client):
make_sloppak(server, "a.sloppak", with_cover=True)
raw = gif_bytes()
body = client.post("/api/song/a.sloppak/art/upload", json={"image": b64(raw)}).json()
assert body == {"ok": True, "kind": "gif"}
r = client.get("/api/song/a.sloppak/art")
assert r.headers["content-type"] == "image/gif"
assert r.content == raw # VERBATIM — animation intact
# …and the pack file was never touched (GIF never reaches the feedpak).
assert (server.DLC_DIR / "a.sloppak" / "cover.jpg").read_bytes() == png_bytes((10, 200, 10))
assert not (server.DLC_DIR / "a.sloppak" / "cover.gif").exists()
# A later PNG upload replaces the GIF (one override per song).
client.post("/api/song/a.sloppak/art/upload", json={"image": b64(png_bytes())})
assert client.get("/api/song/a.sloppak/art").headers["content-type"] == "image/png"
assert len(server._art_override_paths("a.sloppak")) == 1
def test_bad_upload_rejected(server, client):
make_sloppak(server, "a.sloppak")
assert "error" in client.post("/api/song/a.sloppak/art/upload",
json={"image": b64(b"GIF89a not really a gif")}).json()
assert "error" in client.post("/api/song/a.sloppak/art/upload",
json={"image": b64(b"plain text")}).json()
# ── art by URL ────────────────────────────────────────────────────────────────
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)))
body = client.post("/api/song/a.sloppak/art/url",
json={"url": "https://example.com/cover.png"}).json()
assert body == {"ok": True, "kind": "png"}
assert client.get("/api/song/a.sloppak/art").headers["content-type"] == "image/png"
def test_art_url_validation(server, client, monkeypatch):
make_sloppak(server, "a.sloppak")
assert client.post("/api/song/a.sloppak/art/url",
json={"url": "ftp://example.com/x.png"}).status_code == 400
assert client.post("/api/song/a.sloppak/art/url", json={}).status_code == 400
assert client.post("/api/song/ghost.sloppak/art/url",
json={"url": "https://example.com/x.png"}).status_code == 404
# 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)
assert client.post("/api/song/a.sloppak/art/url",
json={"url": "https://example.com/x.png"}).status_code == 400
def test_art_url_offline_by_default(server, client):
"""The real fetch seam refuses under the test env — pytest can never
reach the network even when a test forgets to fake it."""
make_sloppak(server, "a.sloppak")
r = client.post("/api/song/a.sloppak/art/url",
json={"url": "https://example.com/x.png"})
assert r.status_code == 502
# ── the enrichment art worker (Cover Art Archive) ─────────────────────────────
def _match_row(server, fn, release_id="rel-1"):
"""Seed a matched enrichment row with a release id (as the P8 matcher
would have written) so the art worker picks it up."""
song = server.meta_db.enrichment_song_row(fn)
h = server.meta_db.enrichment_content_hash(
song["artist"], song["title"], song["album"], song["duration"])
server.meta_db.apply_enrichment_match(
fn, h, "matched", source="text", score=1.0,
cand={"recording_id": "rec-1", "release_id": release_id,
"title": song["title"], "artist": song["artist"]})
@pytest.fixture()
def caa(server, monkeypatch):
"""Fake CAA transport + network flag on (mirrors the P8 test fixture)."""
calls = []
art = {"rel-1": png_bytes((60, 60, 200))}
def fake(release_id):
calls.append(release_id)
return art.get(release_id)
fake.calls, fake.art = calls, art
monkeypatch.setattr(server, "_caa_http_get", fake)
monkeypatch.setattr(server, "_enrich_network_enabled", lambda: True)
return fake
def test_caa_fetch_fills_missing_art(server, client, caa):
make_sloppak(server, "a.sloppak") # no pack art
_match_row(server, "a.sloppak")
server._background_enrich()
row = server.meta_db.get_enrichment("a.sloppak")
assert row["art_state"] == "caa"
assert row["art_cache_path"] and row["art_cache_path"].endswith("caa_rel-1.jpg")
r = client.get("/api/song/a.sloppak/art")
assert r.status_code == 200
assert r.headers["content-type"] == "image/jpeg"
# Settled: the next pass never re-fetches.
n = len(caa.calls)
server._background_enrich()
assert len(caa.calls) == n
def test_caa_skips_pack_art_and_dedupes_by_release(server, caa):
make_sloppak(server, "haspack.sloppak", with_cover=True, title="One")
make_sloppak(server, "b.sloppak", title="Two")
make_sloppak(server, "c.sloppak", title="Three")
_match_row(server, "haspack.sloppak")
_match_row(server, "b.sloppak") # same release as c
_match_row(server, "c.sloppak")
server._background_enrich()
assert server.meta_db.get_enrichment("haspack.sloppak")["art_state"] == "pack"
assert server.meta_db.get_enrichment("b.sloppak")["art_state"] == "caa"
assert server.meta_db.get_enrichment("c.sloppak")["art_state"] == "caa"
assert len(caa.calls) == 1 # one release → ONE fetch
def test_caa_404_marks_none(server, caa):
make_sloppak(server, "a.sloppak")
_match_row(server, "a.sloppak", release_id="rel-missing")
server._background_enrich()
assert server.meta_db.get_enrichment("a.sloppak")["art_state"] == "none"
n = len(caa.calls)
server._background_enrich()
assert len(caa.calls) == n # never re-hammered
def test_caa_transport_error_leaves_row_unevaluated(server, caa, monkeypatch):
make_sloppak(server, "a.sloppak")
_match_row(server, "a.sloppak")
def _down(release_id):
raise server.EnrichTransportError("down")
monkeypatch.setattr(server, "_caa_http_get", _down)
server._background_enrich()
assert server.meta_db.get_enrichment("a.sloppak")["art_state"] is None
# Network back → next pass completes it.
monkeypatch.setattr(server, "_caa_http_get", caa)
server._background_enrich()
assert server.meta_db.get_enrichment("a.sloppak")["art_state"] == "caa"
def test_offline_default_skips_art_worker(server, monkeypatch):
"""Under the plain test env the whole art phase is skipped with the rest
of the network work."""
calls = []
monkeypatch.setattr(server, "_caa_http_get", lambda rid: calls.append(rid))
make_sloppak(server, "a.sloppak")
_match_row(server, "a.sloppak")
server._background_enrich()
assert calls == []
assert server.meta_db.get_enrichment("a.sloppak")["art_state"] is None
def test_lru_prune_evicts_oldest_and_resets_rows(server, caa, monkeypatch):
monkeypatch.setattr(server, "_CAA_CACHE_CAP_BYTES", 1) # everything over cap
make_sloppak(server, "a.sloppak", title="One")
make_sloppak(server, "b.sloppak", title="Two")
caa.art["rel-2"] = png_bytes((1, 1, 1))
_match_row(server, "a.sloppak", release_id="rel-1")
_match_row(server, "b.sloppak", release_id="rel-2")
server._background_enrich()
# With a 1-byte cap every fetch immediately evicts — the rows that pointed
# at evicted files were reset to unevaluated.
caa_files = list(server.ART_CACHE_DIR.glob("caa_*.jpg"))
assert len(caa_files) <= 1
states = {fn: server.meta_db.get_enrichment(fn)["art_state"]
for fn in ("a.sloppak", "b.sloppak")}
assert None in states.values() or list(states.values()).count("caa") <= 1
def test_delete_song_removes_override(server, client):
make_sloppak(server, "a.sloppak")
client.post("/api/song/a.sloppak/art/upload", json={"image": b64(png_bytes())})
assert server._art_override_paths("a.sloppak")
r = client.delete("/api/song/a.sloppak")
assert r.status_code == 200
assert server._art_override_paths("a.sloppak") == []
def test_delete_override_restores_caa_fallback(server, client, caa):
"""Removing a user override that had settled the row as 'user' must reset
the enrichment state so the CAA fallback is fetched and served again —
otherwise the song is stranded with no art at all."""
make_sloppak(server, "a.sloppak") # no pack art
_match_row(server, "a.sloppak")
# Pin an override BEFORE the art worker runs → the pass stamps art_state='user'.
client.post("/api/song/a.sloppak/art/upload", json={"image": b64(png_bytes())})
server._background_enrich()
assert server.meta_db.get_enrichment("a.sloppak")["art_state"] == "user"
# Remove it → the row resets to unevaluated…
assert client.delete("/api/art/a.sloppak/override").json()["removed"]
assert server.meta_db.get_enrichment("a.sloppak")["art_state"] is None
# …and the next pass fetches + serves the release's front cover.
server._background_enrich()
assert server.meta_db.get_enrichment("a.sloppak")["art_state"] == "caa"
r = client.get("/api/song/a.sloppak/art")
assert r.status_code == 200
assert r.headers["content-type"] == "image/jpeg"
def test_upload_rejects_unknown_song_and_oversize(server, client):
# Unknown filename → 404 (no stray override file written).
assert client.post("/api/song/ghost.sloppak/art/upload",
json={"image": b64(png_bytes())}).status_code == 404
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))
assert client.post("/api/song/a.sloppak/art/upload",
json={"image": huge}).status_code == 400
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

5
tests/test_demo_mode.py
View File

@ -101,6 +101,11 @@ def test_demo_off_settings_post_not_blocked(tmp_path, monkeypatch):
# Context menus (R2): per-song re-match + the path-exposing Get info.
("POST", "/api/enrichment/refresh/some-file"),
("GET", "/api/chart/some-file/fileinfo"),
# Art layer (R3): the base64 upload writes files, the server-side URL fetch
# touches the network, and the override delete removes files — all mutations.
("POST", "/api/song/some-file/art/upload"),
("POST", "/api/song/some-file/art/url"),
("DELETE", "/api/art/some-file/override"),
])
def test_demo_on_blocked_routes_return_403(tmp_path, monkeypatch, method, path):
server, client = _make_client(tmp_path, monkeypatch, demo=True)