From b975164fc26ab876e827ff566f9dc34e2940cd46 Mon Sep 17 00:00:00 2001 From: Senko-san Date: Mon, 8 Jun 2026 18:23:30 +0300 Subject: [PATCH] feat(subsonic): response envelope, id scheme, and error mapping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - envelope: one serializer emitting the wrapper in XML (default) and JSON (f=json), carrying status/version/type/serverVersion - ids: stable, reversible type-prefixed ids (tr-/al-/ar-/pl-) ↔ UUIDs - errors: /rest requests render the Subsonic error envelope (always HTTP 200) with standard codes (10 missing param, 40 wrong creds, 50, 70 not found) Co-Authored-By: Claude Opus 4.8 --- app/api/errors.py | 37 ++++++++++-- app/api/rest/envelope.py | 102 ++++++++++++++++++++++++++++++++ app/api/rest/ids.py | 75 +++++++++++++++++++++++ tests/test_subsonic_envelope.py | 72 ++++++++++++++++++++++ tests/test_subsonic_security.py | 70 ++++++++++++++++++++++ 5 files changed, 352 insertions(+), 4 deletions(-) create mode 100644 app/api/rest/envelope.py create mode 100644 app/api/rest/ids.py create mode 100644 tests/test_subsonic_envelope.py create mode 100644 tests/test_subsonic_security.py diff --git a/app/api/errors.py b/app/api/errors.py index 5114e22..8cbdf1a 100644 --- a/app/api/errors.py +++ b/app/api/errors.py @@ -1,8 +1,15 @@ -"""Maps domain exceptions to HTTP responses. The only place that knows both.""" +"""Maps domain exceptions to HTTP responses. The only place that knows both. -from fastapi import FastAPI, Request, status +Two surfaces share this mapping: the native ``/api/v1`` API answers with a JSON +error body and an HTTP status code, while the Subsonic ``/rest`` layer answers +with its own envelope and **always HTTP 200** (the status lives in the body). A +request is routed to the Subsonic renderer by path prefix. +""" + +from fastapi import FastAPI, Request, Response, status from fastapi.responses import JSONResponse +from app.api.rest.envelope import subsonic_error from app.core.logging import get_logger from app.domain.errors import ( AlreadyExistsError, @@ -30,6 +37,21 @@ _STATUS_BY_ERROR: dict[type[DomainError], int] = { StorageError: status.HTTP_500_INTERNAL_SERVER_ERROR, } +# Subsonic error codes (subsonic.org/restapi): 10 missing param, 40 wrong +# credentials, 50 not authorized, 70 not found, 0 generic. +_SUBSONIC_CODE_BY_ERROR: dict[type[DomainError], int] = { + ValidationError: 10, + AuthenticationError: 40, + PermissionDeniedError: 50, + NotFoundError: 70, +} + +_SUBSONIC_PREFIX = "/rest" + + +def _is_subsonic(request: Request) -> bool: + return request.url.path.startswith(_SUBSONIC_PREFIX) + def _error_body(code: str, message: str) -> dict[str, dict[str, str]]: return {"error": {"code": code, "message": message}} @@ -45,7 +67,10 @@ def register_exception_handlers(app: FastAPI) -> None: ) @app.exception_handler(DomainError) - async def _handle_domain_error(_request: Request, exc: DomainError) -> JSONResponse: + async def _handle_domain_error(request: Request, exc: DomainError) -> Response: + if _is_subsonic(request): + code = _SUBSONIC_CODE_BY_ERROR.get(type(exc), 0) + return subsonic_error(code, exc.message, fmt=request.query_params.get("f")) http_status = _STATUS_BY_ERROR.get(type(exc), status.HTTP_400_BAD_REQUEST) return JSONResponse( status_code=http_status, @@ -53,8 +78,12 @@ def register_exception_handlers(app: FastAPI) -> None: ) @app.exception_handler(Exception) - async def _handle_unexpected(_request: Request, exc: Exception) -> JSONResponse: + async def _handle_unexpected(request: Request, exc: Exception) -> Response: log.error("unhandled_exception", exc_info=exc) + if _is_subsonic(request): + return subsonic_error( + 0, "An unexpected error occurred.", fmt=request.query_params.get("f") + ) return JSONResponse( status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, content=_error_body("internal_error", "An unexpected error occurred."), diff --git a/app/api/rest/envelope.py b/app/api/rest/envelope.py new file mode 100644 index 0000000..8fbd0e5 --- /dev/null +++ b/app/api/rest/envelope.py @@ -0,0 +1,102 @@ +"""The Subsonic response envelope — one serializer, two wire formats. + +Every Subsonic endpoint answers with a ```` wrapper carrying +``status`` / ``version`` / ``type`` / ``serverVersion``, in XML (default) or JSON +(``f=json``). All handlers return through :func:`subsonic_response`; errors go +through the rest-aware exception handler (see ``app.api.errors``). + +Payload data model (shared by both formats): + +* a scalar value → an XML attribute / a JSON field +* a nested dict → a single child element / nested object +* a list of dicts → repeated child elements / a JSON array +* the key ``"value"`` → element text content (used by e.g. lyrics) + +``None`` values are dropped. Subsonic always replies with **HTTP 200**, even for +errors — the status lives inside the envelope — so clients parse the body. +""" + +import json +from collections.abc import Mapping +from typing import Any +from xml.etree import ElementTree as ET + +from fastapi import Response + +SUBSONIC_API_VERSION = "1.16.1" +SERVER_TYPE = "mcma" +SERVER_VERSION = "0.1.0" + +_XML_NS = "http://subsonic.org/restapi" +_XML_MEDIA_TYPE = "application/xml; charset=utf-8" +_JSON_MEDIA_TYPE = "application/json; charset=utf-8" + + +def _is_json(fmt: str | None) -> bool: + return fmt in ("json", "jsonp") + + +def _scalar(value: object) -> str: + if isinstance(value, bool): + return "true" if value else "false" + return str(value) + + +def _build_xml(parent: ET.Element, data: Mapping[str, Any]) -> None: + for key, value in data.items(): + if value is None: + continue + if key == "value": + parent.text = _scalar(value) + elif isinstance(value, Mapping): + _build_xml(ET.SubElement(parent, key), value) + elif isinstance(value, list): + for item in value: + _build_xml(ET.SubElement(parent, key), item) + else: + parent.set(key, _scalar(value)) + + +def _strip_none(value: Any) -> Any: + """Recursively drop ``None`` values so JSON output matches XML (no empty attrs).""" + if isinstance(value, Mapping): + return {k: _strip_none(v) for k, v in value.items() if v is not None} + if isinstance(value, list): + return [_strip_none(v) for v in value] + return value + + +def _render(body: Mapping[str, Any], fmt: str | None) -> Response: + envelope: dict[str, Any] = { + "status": body["status"], + "version": SUBSONIC_API_VERSION, + "type": SERVER_TYPE, + "serverVersion": SERVER_VERSION, + "openSubsonic": True, + **{k: v for k, v in body.items() if k != "status"}, + } + + if _is_json(fmt): + payload = json.dumps({"subsonic-response": _strip_none(envelope)}) + return Response(content=payload, media_type=_JSON_MEDIA_TYPE) + + root = ET.Element("subsonic-response", {"xmlns": _XML_NS}) + _build_xml(root, envelope) + xml = b'\n' + ET.tostring(root, encoding="utf-8") + return Response(content=xml, media_type=_XML_MEDIA_TYPE) + + +def subsonic_response( + payload: Mapping[str, Any] | None = None, *, fmt: str | None = None +) -> Response: + """A successful ``status="ok"`` envelope wrapping ``payload``.""" + body: dict[str, Any] = {"status": "ok"} + if payload: + body.update(payload) + return _render(body, fmt) + + +def subsonic_error(code: int, message: str, *, fmt: str | None = None) -> Response: + """A ``status="failed"`` envelope carrying a Subsonic ````.""" + body = {"status": "failed", "error": {"code": code, "message": message}} + return _render(body, fmt) diff --git a/app/api/rest/ids.py b/app/api/rest/ids.py new file mode 100644 index 0000000..7097db8 --- /dev/null +++ b/app/api/rest/ids.py @@ -0,0 +1,75 @@ +"""Stable, reversible mapping between Subsonic opaque string ids and our UUIDs. + +Subsonic ids are opaque strings; ours are UUIDs. We use a type-prefixed, +human-debuggable convention (``tr-`` track, ``al-`` album, +``ar-`` artist, ``pl-`` playlist). Cover-art ids reuse the entity's +own id (an album cover is ``al-``, a track cover ``tr-``). Centralize +encode/decode here so the convention lives in exactly one place. +""" + +import uuid +from enum import StrEnum + +from app.domain.errors import NotFoundError + + +class IdKind(StrEnum): + TRACK = "tr" + ALBUM = "al" + ARTIST = "ar" + PLAYLIST = "pl" + + +def encode(kind: IdKind, value: uuid.UUID) -> str: + return f"{kind.value}-{value}" + + +def encode_track(value: uuid.UUID) -> str: + return encode(IdKind.TRACK, value) + + +def encode_album(value: uuid.UUID) -> str: + return encode(IdKind.ALBUM, value) + + +def encode_artist(value: uuid.UUID) -> str: + return encode(IdKind.ARTIST, value) + + +def encode_playlist(value: uuid.UUID) -> str: + return encode(IdKind.PLAYLIST, value) + + +def parse(raw: str) -> tuple[IdKind, uuid.UUID]: + """Decode any prefixed id into its kind + UUID. Raises ``NotFoundError`` on a + malformed id (an unknown id is, from the client's view, simply not found).""" + prefix, _, rest = raw.partition("-") + try: + kind = IdKind(prefix) + value = uuid.UUID(rest) + except ValueError as exc: + raise NotFoundError(f"Unknown id {raw!r}.") from exc + return kind, value + + +def _decode_as(raw: str, expected: IdKind) -> uuid.UUID: + kind, value = parse(raw) + if kind is not expected: + raise NotFoundError(f"Expected a {expected.name.lower()} id, got {raw!r}.") + return value + + +def decode_track(raw: str) -> uuid.UUID: + return _decode_as(raw, IdKind.TRACK) + + +def decode_album(raw: str) -> uuid.UUID: + return _decode_as(raw, IdKind.ALBUM) + + +def decode_artist(raw: str) -> uuid.UUID: + return _decode_as(raw, IdKind.ARTIST) + + +def decode_playlist(raw: str) -> uuid.UUID: + return _decode_as(raw, IdKind.PLAYLIST) diff --git a/tests/test_subsonic_envelope.py b/tests/test_subsonic_envelope.py new file mode 100644 index 0000000..76d1a37 --- /dev/null +++ b/tests/test_subsonic_envelope.py @@ -0,0 +1,72 @@ +"""Unit tests for the Subsonic response envelope (XML + JSON shapes).""" + +import json +from xml.etree import ElementTree as ET + +from app.api.rest.envelope import ( + SUBSONIC_API_VERSION, + subsonic_error, + subsonic_response, +) + + +def _xml_root(body: bytes) -> ET.Element: + return ET.fromstring(body) + + +def _local(tag: str) -> str: + return tag.rsplit("}", 1)[-1] # strip namespace + + +def test_ok_xml_shape() -> None: + resp = subsonic_response({"license": {"valid": True}}, fmt="xml") + assert resp.media_type.startswith("application/xml") + root = _xml_root(resp.body) + assert _local(root.tag) == "subsonic-response" + assert root.attrib["status"] == "ok" + assert root.attrib["version"] == SUBSONIC_API_VERSION + assert root.attrib["type"] == "mcma" + child = root[0] + assert _local(child.tag) == "license" + assert child.attrib["valid"] == "true" + + +def test_ok_json_shape() -> None: + resp = subsonic_response({"license": {"valid": True}}, fmt="json") + assert resp.media_type.startswith("application/json") + payload = json.loads(resp.body)["subsonic-response"] + assert payload["status"] == "ok" + assert payload["version"] == SUBSONIC_API_VERSION + assert payload["type"] == "mcma" + assert payload["license"] == {"valid": True} + + +def test_error_xml_shape() -> None: + resp = subsonic_error(40, "Wrong username or password.", fmt="xml") + root = _xml_root(resp.body) + assert root.attrib["status"] == "failed" + error = root[0] + assert _local(error.tag) == "error" + assert error.attrib["code"] == "40" + assert error.attrib["message"] == "Wrong username or password." + + +def test_error_json_shape() -> None: + resp = subsonic_error(70, "Not found.", fmt="json") + payload = json.loads(resp.body)["subsonic-response"] + assert payload["status"] == "failed" + assert payload["error"] == {"code": 70, "message": "Not found."} + + +def test_default_format_is_xml() -> None: + resp = subsonic_response(fmt=None) + assert resp.media_type.startswith("application/xml") + assert _xml_root(resp.body).attrib["status"] == "ok" + + +def test_list_renders_repeated_elements() -> None: + payload = {"genres": {"genre": [{"value": "Rock"}, {"value": "Jazz"}]}} + root = _xml_root(subsonic_response(payload, fmt="xml").body) + genres = root[0] + values = [g.text for g in genres] + assert values == ["Rock", "Jazz"] diff --git a/tests/test_subsonic_security.py b/tests/test_subsonic_security.py new file mode 100644 index 0000000..c304991 --- /dev/null +++ b/tests/test_subsonic_security.py @@ -0,0 +1,70 @@ +"""Unit tests for Subsonic crypto + id helpers (no DB, no network).""" + +import hashlib +import uuid + +import pytest +from app.api.rest import ids +from app.api.rest.ids import IdKind +from app.core.security import SubsonicPasswordCipher, generate_subsonic_password +from app.domain.errors import AuthenticationError, NotFoundError + + +def test_generate_subsonic_password_is_long_and_unique() -> None: + a = generate_subsonic_password() + b = generate_subsonic_password() + assert a != b + assert len(a) >= 20 + + +def test_cipher_roundtrip() -> None: + cipher = SubsonicPasswordCipher("a-secret-key") + plaintext = generate_subsonic_password() + token = cipher.encrypt(plaintext) + assert token != plaintext + assert cipher.decrypt(token) == plaintext + + +def test_cipher_token_then_md5_matches() -> None: + """The decrypted app-password must reproduce a client's t=md5(password+salt).""" + cipher = SubsonicPasswordCipher("a-secret-key") + password = generate_subsonic_password() + enc = cipher.encrypt(password) + + salt = "c19b2d" + decrypted = cipher.decrypt(enc) + expected = hashlib.md5((decrypted + salt).encode(), usedforsecurity=False).hexdigest() + client_token = hashlib.md5((password + salt).encode(), usedforsecurity=False).hexdigest() + assert expected == client_token + + +def test_cipher_wrong_key_fails() -> None: + token = SubsonicPasswordCipher("key-one").encrypt("hunter2") + with pytest.raises(AuthenticationError): + SubsonicPasswordCipher("key-two").decrypt(token) + + +def test_id_encode_decode_roundtrip() -> None: + value = uuid.uuid4() + assert ids.decode_track(ids.encode_track(value)) == value + assert ids.decode_album(ids.encode_album(value)) == value + assert ids.decode_artist(ids.encode_artist(value)) == value + assert ids.decode_playlist(ids.encode_playlist(value)) == value + + +def test_id_parse_returns_kind() -> None: + value = uuid.uuid4() + kind, parsed = ids.parse(ids.encode_album(value)) + assert kind is IdKind.ALBUM + assert parsed == value + + +def test_id_wrong_prefix_rejected() -> None: + track = ids.encode_track(uuid.uuid4()) + with pytest.raises(NotFoundError): + ids.decode_album(track) + + +def test_id_malformed_rejected() -> None: + with pytest.raises(NotFoundError): + ids.parse("not-a-real-id")