diff --git a/docs/api-v2-guide.md b/docs/api-v2-guide.md index bd2545c4..c1471fa4 100644 --- a/docs/api-v2-guide.md +++ b/docs/api-v2-guide.md @@ -1040,7 +1040,20 @@ leaving `homepage_url` out clears it. That is what the Subsonic every call — and both surfaces are the same domain method, so an absent field cannot mean two different things depending on the door. -**The server never connects to a station.** The address is validated as +**`image_url` is the one exception, and for the same reason turned around.** +A station may carry an image address, validated as `homepage_url` is. The +protocol has no image parameter, so a Subsonic edit says nothing about it; for +renaming a station in a Subsonic client not to take its image away, an absent +`image_url` leaves the image as it is, on `PUT` too. `null` or a blank value +clears it. The server fetches the image **once**, when it is set or changed +(an address already stored, with its image held, is not fetched again), into +the artwork store, and answers `artwork_hash` — read it from +`/api/v2/artwork/{hash}` like any cover, which serves it to the station's own +account only. A fetch that fails keeps `image_url` with `artwork_hash: null`; +saving the station again with the address retries it. Subsonic's +`internetRadioStation` is unchanged and carries no `coverArt`. + +**The server never connects to a station's stream.** The address is validated as absolute `http` or `https`, stored as typed and handed back; the player is what reaches the station. That is the whole security answer for the feature: a station is a URL somebody supplies, and a server that fetched one would be a diff --git a/docs/operations.md b/docs/operations.md index ef00c4cf..f0041447 100644 --- a/docs/operations.md +++ b/docs/operations.md @@ -411,6 +411,41 @@ never a feed address — a private feed carries its token in its path or its query string. For the same reason only administrators are shown a feed's address; other accounts see at most its scheme and host. +## Podcast covers and station images + +**On by default.** The server fetches the cover a podcast feed names, and the +image an account gives one of its radio stations, into the artwork store +(`artwork/` in the data directory), so the web client and Subsonic clients +show them as they show album covers — the browser never loads them from +their own host. [RFC-025](rfcs/RFC-025-smart-playlists-and-podcasts.md) +carries the decisions; [privacy](privacy.md) says what is fetched and when. + +| Variable | Default | What it does | +| --- | --- | --- | +| `WAVEFLOW_REMOTE_ARTWORK` | on | `off` fetches neither: an address is still stored, no image is fetched, and the images already stored stay shown | + +**When.** A podcast cover only while `WAVEFLOW_PODCASTS` is on: when a channel +is subscribed, and on a refresh that finds the feed naming a cover it does not +hold — a new address, a failed last attempt, a file gone from the store, or a +copy older than a week. A station image once, when an account sets or changes +it, and only once the station is saved; saving the station again retries one +that failed. An account may have at most ten station images fetched an hour; +past that, a station is saved without its image until a later save. Never on a timer of its +own, never at boot, and never while anybody listens. + +**Under which rules.** Those of the podcast fetcher: public addresses only, +checked before connecting and pinned, at most five redirects each checked +before it is followed, 10 MiB, and the bytes must be a JPEG, PNG or WebP — +a page of HTML is refused whatever it is labelled. A podcast cover may be +plain http only while `WAVEFLOW_PODCAST_ALLOW_HTTP` is on; a station image may +always be, as its stream may. **What it logs**: a verdict such as +`image over the size ceiling`, never the address. + +**What it stores.** One file per image, `.`, beside the scanned +covers. The artwork sweep collects it once nothing names it: when the channel +or the station is deleted, or when the feed or the account names another +image. + ## Scanning A scan reads only what changed. A file whose size and modification time are diff --git a/docs/privacy.md b/docs/privacy.md index d476a34f..27dbce4a 100644 --- a/docs/privacy.md +++ b/docs/privacy.md @@ -3,22 +3,25 @@ Every host a WaveFlow Server can contact, what it sends there, when, and the setting that stops it. This was written from the code (RFC-019 decision 4), and the code is the authority if the two ever disagree: `src/outbound.rs`, -`src/enrichment.rs`, `src/portrait.rs`, `src/lrclib.rs`, `src/podcast.rs` and -`src/scrobblers/`. +`src/enrichment.rs`, `src/portrait.rs`, `src/lrclib.rs`, `src/podcast.rs`, +`src/services/remote_artwork.rs` and `src/scrobblers/`. -**A server left at its defaults sends nothing.** Booting makes no request, and -no background task does unless an operator configured it *and*, for -scrobbling, an account linked a destination — or, for podcasts, an +**A server left at its defaults sends nothing on its own.** Booting makes no +request, and no background task does unless an operator configured it *and*, +for scrobbling, an account linked a destination — or, for podcasts, an administrator subscribed to a feed. The clients are built at boot; -building one sends nothing. +building one sends nothing. The one request a default server makes is one an +account asks for by name: the image of a radio station, fetched once when the +account sets it (below; `WAVEFLOW_REMOTE_ARTWORK=off` stops it). ## Rules every request follows All outbound clients are built by one function, `src/outbound.rs`: - **No redirect is followed by the client.** A provider cannot bounce a - request to another host. The one exception is the podcast fetcher, below: - it follows at most five redirects by hand, and puts every hop through the + request to another host. The one exception is the podcast fetcher, below, + and the podcast covers and station images fetched under its rules: it + follows at most five redirects by hand, and puts every hop through the same scheme and public-address checks as the first address before contacting it. - **`HTTP_PROXY` and the other proxy variables are ignored.** @@ -35,9 +38,10 @@ All outbound clients are built by one function, `src/outbound.rs`: verdict, never with its URL or its body. The server never turns a request it receives into a request of its own, with -one exception an operator switches on: an administrator subscribing to a -podcast, or asking for an episode, below. Radio -station URLs are stored and handed to clients, never fetched; remote entries in +two exceptions: an administrator subscribing to a podcast, or asking for an +episode, which an operator switches on; and an account setting a radio +station's image, which an operator can switch off — both below. Radio +station stream URLs are stored and handed to clients, never fetched; remote entries in an imported playlist are dropped; FFmpeg and ffprobe are only ever given local files inside a library or the data directory, and are told so: every input is passed with `-protocol_whitelist file`, so neither can open a network or @@ -213,9 +217,9 @@ one interval after boot, not at boot — and when one asks for an episode. the server's disk, so the podcast host sees the server, once, and no listener. **What.** A plain `GET` with the server's User-Agent and nothing about who -asked. The channel's cover address is stored and handed to clients, and never -fetched by the server; a client that shows it fetches it from the feed's host -itself. +asked. The channel's cover is fetched by the server too — see the next +section — and handed to clients by hash, so no client needs to contact the +feed's host for it. **Kept.** The channel's title, description, links and cover address; each episode's title, description, dates, duration and enclosure address; the @@ -226,6 +230,52 @@ account is shown at most its scheme and host. Enclosure addresses are shown to nobody. No feed or enclosure address, and no part of a response, is written to the log. +## Podcast covers and station images + +**On by default; `WAVEFLOW_REMOTE_ARTWORK=off` stops both** +([RFC-025](rfcs/RFC-025-smart-playlists-and-podcasts.md), amended). Off, the +addresses are still stored and nothing is fetched; images already fetched stay +shown. + +**Which hosts.** + +- **A podcast cover**: the host the feed names for its image — the feed's own + host, or another one the feed chose, such as a CDN — and every redirect it + answers with, up to five. Only while `WAVEFLOW_PODCASTS` is on. +- **A station image**: the host in the address an account typed as its + station's image, and its redirects, up to five. Nothing is fetched for a + station without one. + +Every address is held to the podcast fetcher's rules, above: http or https +(plain http for a cover only while `WAVEFLOW_PODCAST_ALLOW_HTTP` is on; always +for a station image, as for its stream), no user name or password, the name +resolved first and every address public, the connection pinned to the address +checked, each redirect checked before it is followed. An answer is read up to +10 MiB within 20 seconds, and kept only if its bytes are a JPEG, PNG or WebP. + +**When.** A podcast cover when a channel is subscribed, and on a refresh that +finds the feed naming a cover the server does not hold — a new address, one +whose last fetch failed, a file gone from the store, or a copy older than a +week; an unchanged address is otherwise not fetched again. A station image +once, when an account sets or changes it, and again only when the account +saves the station after a fetch that failed — at most ten an hour per account. +In both cases only after the write was accepted: a station or a channel +refused (a full list, a station that is not the account's) contacts nobody. Never at boot, never on a timer of +its own, and **never while anybody listens**: the image is served from the +server's artwork store, so the image's host sees the server, once, and no +listener — which is why the browser does not load it itself. + +**What.** A plain `GET` with the server's User-Agent and nothing about who +asked or who listens. + +**Kept.** The image, under `artwork/` in the data directory, named by the hash +of its bytes. A cover's address is shown to administrators only, as the feed's +is — a private feed's cover can carry its subscriber's token — and every other +account is given the hash alone. A station's image address is shown to the +account the station belongs to, and its image is served to that account alone. +Deleting the channel or the station releases the image, and the artwork sweep +removes it. No image address, and no part of an answer, is written to the log. + ## What the browser contacts The embedded web client loads nothing from a third party: its fonts are @@ -235,7 +285,9 @@ to that (RFC-019 decision 2). Two exceptions, each deliberate: - **Internet radio.** The browser plays a station straight from the station's own server, at the URL a user entered. That host sees the listener's address. - Subsonic clients receive the same URLs and connect themselves. + Subsonic clients receive the same URLs and connect themselves. A station's + image is not part of this: the server fetched it, and the browser reads it + from the server. - **Linking Last.fm**, above: a navigation the user starts. **`/reference`**, the API reference page, was a third until RFC-023: it loaded diff --git a/docs/rfcs/RFC-025-smart-playlists-and-podcasts.md b/docs/rfcs/RFC-025-smart-playlists-and-podcasts.md index 0ab08dfb..55c2625f 100644 --- a/docs/rfcs/RFC-025-smart-playlists-and-podcasts.md +++ b/docs/rfcs/RFC-025-smart-playlists-and-podcasts.md @@ -2,7 +2,9 @@ - **Status**: Proposed - **Implemented by**: [#333](https://github.com/InstaZDLL/waveflow-server/pull/333) - (this RFC and its implementation, together). The *Status* field above never + (this RFC and its implementation, together), and + [#342](https://github.com/InstaZDLL/waveflow-server/pull/342) (the amendment + on artwork for podcasts and radio stations). The *Status* field above never flips in this project; this line is the one that says what is running. - **Date**: 2026-10-25 - **Authors**: the WaveFlow project @@ -381,7 +383,9 @@ lyrics: newest are kept per refresh. - **The cover is not fetched.** Its address is kept and handed to clients as `originalImageUrl`; channels carry no `coverArt`. Fetching artwork is a second - outbound surface this RFC does not open. + outbound surface this RFC does not open. *Superseded by the + [amendment](#amendment--artwork-for-podcasts-and-radio-stations) below: the + cover is fetched, and its address is shown to managers only.* ## Decision 11 — episodes are downloaded, never relayed @@ -484,6 +488,139 @@ enclosure that is HTML, an episode past its ceiling and one past the quota. and the production address policy's refusal of loopback is the portrait fetcher's own test. +## Amendment — artwork for podcasts and radio stations + +*Added 2026-10-26.* The play bar showed a station or an episode with a +generic symbol, and the podcasts page showed no cover at all. The covers +existed — a feed names one, and a station may have a logo — but the web client +cannot load them: RFC-019's Content-Security-Policy keeps `img-src` to this +origin, and an image loaded from the feed's host would hand that host the +address of every listener, which decision 11 exists to prevent for the audio. +So the server fetches the image once and serves it from the artwork store, +like any cover. + +**A1 — one fetcher, the podcast rules, the portrait checks.** A cover's address +comes out of a document, a station image's from a person, so neither can be +held to an allowlist as a provider's portrait is (RFC-011 decision 8). +`portrait::fetch_followed` reaches the host through decision 9's rules — +`podcast::open`: http or https, no user information, every address public, +checked and pinned, redirects followed by hand, at most five, each hop checked +before it is contacted — and reads the answer through the portrait fetcher's +own checks, now shared by both doors: 10 MiB, read with +`outbound::read_bounded_body`, and the bytes sniffed as JPEG, PNG or WebP. A +`Content-Type` naming another type is refused unread; one naming none, or +`application/octet-stream` — what object storage labels every cover with — is +read and judged by its bytes. A page of HTML is refused whatever it is +labelled. Nothing is logged but a verdict. + +**A2 — a channel's cover.** Fetched when the channel is subscribed — in the +background, once the channel is committed, so that the subscription answers +with its cover pending rather than holding the request for a feed fetch and a +cover fetch in a row (Subsonic's `createPodcastChannel` sits under no +`TimeoutLayer`, so nothing else would bound that sum) — and on a refresh, +before the writer gate, recorded in the refresh's transaction: +`podcast_channel.artwork_hash`, with the address it was fetched from +(`artwork_url`) and when (`artwork_fetched_at`). A refresh fetches it again +only when the feed names an address other than the one fetched from, when no +cover is held (the last fetch failed), when the file is gone from the store, +or when the copy is older than a week; otherwise the feed is read and the cover +is not. A failed fetch stores nothing and keeps whatever was held — the old +cover, when the feed moved to a new one that cannot be reached — and the next +refresh tries again, because the addresses still differ. A feed that stops +naming a cover has it forgotten. Plain http is allowed for a cover exactly +when `WAVEFLOW_PODCAST_ALLOW_HTTP` allows it for the feed. + +Two fetches of one channel's cover can be in flight at once — the +subscription's, in the background, and a refresh's — so the write that records +one names what its fetch assumed: the channel still names the address it came +from (or, to forget a cover, names none), and `artwork_fetched_at` is still +what it was when the fetch began. Otherwise the cover is dropped and the newer +state stands; its `artwork` row names nothing, and the sweep collects it. + +**A3 — episodes show their channel's cover.** An episode's own +`itunes:image` is not fetched: a refresh keeps up to five hundred episodes, and +fetching one image each would make the cover traffic proportional to the +episodes rather than to the channels, which is what decision 8 keeps it to. + +**A4 — what is handed out.** The hash, natively as `artwork_hash` on a channel +and on each episode, and through Subsonic as `coverArt` on `getPodcasts`, +`getNewestPodcasts`, `getPodcastEpisode` and an episode bookmark's entry, +served by `/api/v2/artwork/{hash}` and `getCoverArt` to every account, as every +account reads the channels. **The address is shown to managers only** — +natively `image_url`, through Subsonic `originalImageUrl` — as the feed's is +(decision 8): a private feed's cover can carry its subscriber's token. This +changes an answer: `originalImageUrl` was given to every account. + +**A5 — a station's image.** A station may carry an `image_url`, validated as +`homepage_url` is: absolute, http or https, a blank value meaning none. When it +is set or changed through the native API, the server fetches it once under A1 +— plain http allowed, as a station's stream so often is — and stores +`radio_station.artwork_hash`; an address already stored, with its image in the +store, is not fetched again. **A failed fetch keeps the address and shows no +image**, and the retry is the next save that carries the address; there is no +background pass, so a station image is never fetched without an account +asking. The image is served to the station's account alone, as the station is +(404 to everyone else). **A replacement does not clear the image by omission**: +`PUT` without `image_url` keeps it, and `null` clears it. The Subsonic writes +have no image parameter — `createInternetRadioStation` creates a station +without one and `updateInternetRadioStation` leaves it as it is — so renaming a +station in a Subsonic client cannot take its image away, and a native client +written before the field existed cannot either. `internetRadioStation` is +unchanged and carries no `coverArt`: the frozen contract defines none. + +**A5b — nothing is fetched for a write that is refused, and a station image +has a budget.** Every check that can refuse a write runs, and the row is +committed, before any image is fetched: the address rules, the per-account +station ceiling, the station being the account's own (404 otherwise), and for +a channel the instance ceiling and the duplicate check — the ceiling is now +also asked before the *feed* is fetched. The image is then fetched, written to +the store, and attached by a second write that names the row (and, for a +station, its account and the address it was fetched for), so an image that +arrives after the row was deleted or its address changed attaches to nothing +and is swept. An account at its ceiling, or naming a station that is not its +own, makes the server contact nobody and write nothing. Station images are the +one fetch any account can ask for, so they are also bounded per account: at +most ten an hour (in memory, refilled by a restart), past which a station is +saved without an image and a later save retries. With an unchanged address +never fetched again, that caps the worst case at 100 MiB an hour per account, +which the sweep collects. + +**A6 — one switch, on by default.** `WAVEFLOW_REMOTE_ARTWORK` (on) stops both +when it is off: the addresses are still stored, nothing is fetched, and images +already stored stay shown. On by default because nothing here happens on its +own: a cover only while `WAVEFLOW_PODCASTS` — off by default — is on, and a +station image only when an account sets one. That makes a station image the +one request a server at its defaults can be asked to make; `docs/privacy.md` +says so. + +**A7 — the sweep.** `sweep_artwork_store` counts `podcast_channel.artwork_hash` +and `radio_station.artwork_hash` as references, so a cover named by either is +never collected, however old its file. Deleting the channel or the station — +or the feed or the account naming another image — releases it, and the next +sweep collects the row and the file. The window the sweep's module describes +(a row committed an instant after the sweep read the table) can carry off a +fetched file; the next write that names it asks whether the file is still +there before it trusts an unchanged address, and fetches it again. + +**Tests.** `tests/podcasts.rs`, against its loopback host: a cover fetched, +served natively and through `getCoverArt`, and `coverArt` on channels and +episodes; an unchanged address not fetched again, a moved one fetched once +through a redirect and the old cover released; a cover over the ceiling, +declared and streamed; HTML labelled as an image and as HTML; redirects to a +private and to a mapped private address; an unlabelled JPEG accepted; a failed +fetch retried on the next refresh; a station image fetched, served to its +account only, kept by a native write without the field and by a Subsonic edit, +retried on the next save, cleared by `null`, and a foreign station's +replacement answered 404 with no request made; an account at its station +ceiling and an instance at its channel ceiling refused with no request to the +feed's or the image's host and nothing stored; and the sweep keeping both +images until the channel and the station are deleted. +`src/services/remote_artwork.rs` tests the hourly budget. A Subsonic +`createPodcastChannel` whose cover host never answers is answered within five +seconds, the channel created and its cover pending. The private-feed test's cover +now carries the subscriber's token, so it proves the address reaches no +reader. `src/services/podcasts.rs` tests when a cover is due. + ## What is still open - **Navidrome `.nsp` import** (decision 6). @@ -493,8 +630,7 @@ fetcher's own test. only, so that no existing answer changes in this pull request. - **A per-account `podcastRole`.** Only administrators manage podcasts; a role column on accounts would let an operator grant it. -- **Channel artwork**, fetched into the artwork store under the outbound rules, - so `coverArt` can be answered. +- **An episode's own artwork** (`itunes:image` on an item): see A3. - **Automatic downloads** of new episodes, per channel, and a per-channel refresh. - **OPML import and export** of subscriptions. diff --git a/docs/subsonic-compatibility.md b/docs/subsonic-compatibility.md index fa441ef8..492acdc6 100644 --- a/docs/subsonic-compatibility.md +++ b/docs/subsonic-compatibility.md @@ -364,14 +364,28 @@ lists episode bookmarks after the track ones, their `entry` the episode The schema requires the attribute, and a private feed carries its subscriber's token in its path as often as in its query. Episodes carry no enclosure address at all: they are played from the server. -- **No cover art**: a channel carries `originalImageUrl`, the feed's own - address, and no `coverArt` — the server does not fetch it. +- **Cover art, fetched by the server** (RFC-025, amended). A channel carries + `coverArt`, the hash of the copy of the feed's cover the server fetched into + the artwork store, and every episode carries its channel's — an episode's own + `itunes:image` is not fetched. `getCoverArt` serves it to every account, as + every account reads the channels. Both are absent until a fetch succeeds, + and when `WAVEFLOW_REMOTE_ARTWORK` is off. **Changed values**: there was no + `coverArt` on a channel or an episode before, and `originalImageUrl`, the + feed's own address for the cover, is now given only to an account holding + `podcastRole`, as `channel.url` is — a private feed's cover can carry its + subscriber's token. Both are optional in the schema. +- **Internet radio stations carry no `coverArt`**, though a station may now + have an image set through the native API: `InternetRadioStation` defines no + such field, and the frozen contract is left as it was. + `createInternetRadioStation` takes no image, and `updateInternetRadioStation` + leaves a station's image as it is rather than clearing it. - **A feed that cannot be read** answers `createPodcastChannel` with error 0 and a generic message; nothing is stored. To check in the next campaign: that DSub and Symfonium list channels and play a -downloaded episode through `streamId`, and that a client which never sees -`podcastRole="true"` keeps hiding its podcast screens. +downloaded episode through `streamId`, that a client which never sees +`podcastRole="true"` keeps hiding its podcast screens, and that both show a +channel's and an episode's `coverArt` through `getCoverArt`. ## The replay this alignment required diff --git a/migrations-v2/20261028000000_podcast_and_radio_artwork.sql b/migrations-v2/20261028000000_podcast_and_radio_artwork.sql new file mode 100644 index 00000000..06fda4f5 --- /dev/null +++ b/migrations-v2/20261028000000_podcast_and_radio_artwork.sql @@ -0,0 +1,40 @@ +-- Artwork for podcast channels and internet radio stations (RFC-025, amended). +-- +-- Both carried a cover address and nothing else, and the web client could not +-- show it: RFC-019's Content-Security-Policy keeps `img-src` to this origin, +-- and loading the image from the feed's host would hand that host the address +-- of every listener. So the server fetches the image once, under the rules of +-- `src/podcast.rs` (public addresses only, checked and pinned, redirects +-- followed by hand and checked before each hop, a size ceiling, the bytes +-- sniffed as JPEG, PNG or WebP), stores it in the artwork store, and a client +-- reads it as it reads any cover: `/api/v2/artwork/{hash}`. +-- +-- `artwork_hash` references the store as the catalogue's columns do. The +-- artwork sweep counts both columns as references, so a cover named here is +-- never collected, and deleting the channel or the station is what releases +-- it. + +-- `artwork_url` is the address the stored cover was fetched from, which is +-- not always the address the feed names now: when a feed changes its cover +-- and the new one cannot be fetched, the old cover stays shown and the next +-- refresh tries again, because the two addresses differ. `artwork_fetched_at` +-- is when it was last fetched, so an unchanged address is fetched again only +-- once its age passes the TTL. Neither is shown to anybody: a private feed's +-- cover can carry its subscriber's token as readily as the feed itself. +ALTER TABLE podcast_channel ADD COLUMN artwork_hash TEXT REFERENCES artwork(hash) ON DELETE SET NULL; +ALTER TABLE podcast_channel ADD COLUMN artwork_url TEXT; +ALTER TABLE podcast_channel ADD COLUMN artwork_fetched_at INTEGER; + +-- `image_url` is what the account typed, validated as `homepage_url` is: +-- absolute, http or https. It is kept when the image cannot be fetched, and +-- `artwork_hash` is then NULL: the station shows the default symbol until the +-- next save that carries an address tries again. +ALTER TABLE radio_station ADD COLUMN image_url TEXT; +ALTER TABLE radio_station ADD COLUMN artwork_hash TEXT REFERENCES artwork(hash) ON DELETE SET NULL; + +-- What the sweep and the artwork route ask: "does anything name this hash?" +-- Partial, because most rows name nothing. +CREATE INDEX podcast_channel_artwork_idx + ON podcast_channel (artwork_hash) WHERE artwork_hash IS NOT NULL; +CREATE INDEX radio_station_artwork_idx + ON radio_station (artwork_hash) WHERE artwork_hash IS NOT NULL; diff --git a/src/api/radio.rs b/src/api/radio.rs index d3fe33b9..eb530cdd 100644 --- a/src/api/radio.rs +++ b/src/api/radio.rs @@ -25,6 +25,25 @@ pub struct RadioStationRequest { pub stream_url: String, #[serde(default)] pub homepage_url: Option, + /// An image for the station, absolute `http` or `https`, which the server + /// fetches once into the artwork store and hands out as `artwork_hash`. + /// + /// The one field that is not replaced by omission: absent, the image stays + /// as it is; `null` or blank, it is cleared. A client written before the + /// field existed — or a Subsonic one, which has no such parameter — must + /// not take a station's image away by renaming it. + #[serde(default, deserialize_with = "crate::services::present")] + #[schema(value_type = Option, nullable)] + pub image_url: Option>, +} + +impl RadioStationRequest { + fn image(&self) -> crate::services::StationImage<'_> { + match &self.image_url { + None => crate::services::StationImage::Keep, + Some(value) => crate::services::StationImage::Set(value.as_deref()), + } + } } #[utoipa::path(get, path = "/api/v2/radio-stations", tag = "radio", responses((status = 200, body = [crate::services::RadioStationItem]), (status = 401, body = ErrorResponse)))] @@ -72,6 +91,7 @@ pub async fn create_radio_station( &request.name, &request.stream_url, request.homepage_url.as_deref(), + request.image_url.as_ref().and_then(Option::as_deref), ) .await .map_err(service_error)?; @@ -97,6 +117,7 @@ pub async fn replace_radio_station( &request.name, &request.stream_url, request.homepage_url.as_deref(), + request.image(), ) .await .map(Json) diff --git a/src/config.rs b/src/config.rs index 1087042c..07dd7387 100644 --- a/src/config.rs +++ b/src/config.rs @@ -94,6 +94,26 @@ pub struct PodcastSettings { pub addresses: crate::portrait::AddressPolicy, } +/// Remote artwork: the cover a podcast feed names, and the image an account +/// gives a radio station (RFC-025, amended). +/// +/// **On by default**, unlike podcasts, because nothing here happens on its own: +/// a station image is fetched once when an account sets or changes one, and a +/// podcast cover only while `WAVEFLOW_PODCASTS` is on and a channel is +/// subscribed or refreshed. Off, an address is still stored and nothing is +/// fetched; covers already stored stay shown. +#[derive(Debug, Clone, Copy)] +pub struct RemoteArtworkSettings { + /// `WAVEFLOW_REMOTE_ARTWORK`, on by default. + pub enabled: bool, + /// One image fetch, redirects included. Derived, not set: the figure the + /// feed fetch defaults to, for a body the size of a cover. + pub timeout: Duration, + /// Which addresses a fetch may reach, as for podcasts: public ones in + /// production, always, and loopback as well for the test suite's hosts. + pub addresses: crate::portrait::AddressPolicy, +} + /// What the server accepts when a member attaches a loop to a track. /// /// Apart from [`UploadLimits`] rather than folded into it: the two magazines @@ -557,6 +577,9 @@ pub struct Config { pub lyrics: LyricsSettings, /// Podcasts — RFC-025. See [`PodcastSettings`]. pub podcasts: PodcastSettings, + /// Podcast covers and station images — RFC-025, amended. See + /// [`RemoteArtworkSettings`]. + pub remote_artwork: RemoteArtworkSettings, pub allowed_origins: Vec, /// `WAVEFLOW_TRUSTED_PROXIES`: the reverse proxies whose /// `X-Forwarded-For` is believed when telling one client from another @@ -632,6 +655,7 @@ impl std::fmt::Debug for Config { .field("metadata", &self.metadata) .field("lyrics", &self.lyrics) .field("podcasts", &self.podcasts) + .field("remote_artwork", &self.remote_artwork) .field("allowed_origins", &self.allowed_origins) .field("trusted_proxies", &self.trusted_proxies) // Whether it is set, never what it is. @@ -868,6 +892,11 @@ impl Config { let metadata = parse_metadata_settings()?; let lyrics = parse_lyrics_settings()?; let podcasts = parse_podcast_settings(&data_dir)?; + let remote_artwork = RemoteArtworkSettings { + enabled: parse_bool_env("WAVEFLOW_REMOTE_ARTWORK", true)?, + timeout: Duration::from_secs(20), + addresses: crate::portrait::AddressPolicy::Public, + }; // Absent is empty; a value that is not UTF-8 refuses to boot, rather // than starting with no trusted proxy and counting every client behind // it under the proxy's address. @@ -971,6 +1000,7 @@ impl Config { metadata, lyrics, podcasts, + remote_artwork, allowed_origins, trusted_proxies, setup_token, @@ -1111,6 +1141,15 @@ impl Config { dir: podcast_dir_for_tests, addresses: crate::portrait::AddressPolicy::AllowLoopback, }, + // On, as production defaults, and admitting loopback for the same + // reason the podcast policy above does: the image hosts the suite + // serves are listeners of its own. No test reaches this unless it + // names an image, so nothing leaves the machine. + remote_artwork: RemoteArtworkSettings { + enabled: true, + timeout: Duration::from_secs(10), + addresses: crate::portrait::AddressPolicy::AllowLoopback, + }, allowed_origins: Vec::new(), trusted_proxies: Vec::new(), // Drawn at boot and written to the data directory, as production diff --git a/src/portrait.rs b/src/portrait.rs index 18a3604c..adeffff1 100644 --- a/src/portrait.rs +++ b/src/portrait.rs @@ -21,6 +21,13 @@ //! host lives on loopback, which the default refuses. [`AddressPolicy::default`] //! is production, and a test asserts it refuses loopback, so the suite cannot //! prove the fetcher only with its main protection switched off. +//! +//! A second door, [`fetch_followed`], serves the addresses no allowlist can +//! hold: a podcast feed's cover and a radio station's image (RFC-025, amended). +//! It reaches the host through the podcast fetcher's rules instead — the same +//! address policy, checked and pinned, and redirects followed by hand with +//! every hop checked — and reads the answer through the same ceiling and the +//! same sniffing as this one. use std::{ net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr}, @@ -144,6 +151,9 @@ pub enum Refusal { Unreached, /// The host answered something other than success. Status(u16), + /// More redirects than a followed fetch allows, or one without a usable + /// target. Only [`fetch_followed`] follows any. + Redirects, } impl Refusal { @@ -153,7 +163,7 @@ impl Refusal { pub fn is_answer(self) -> bool { matches!( self, - Self::Destination | Self::Address | Self::NotAnImage | Self::TooLarge + Self::Destination | Self::Address | Self::NotAnImage | Self::TooLarge | Self::Redirects ) } } @@ -167,6 +177,7 @@ impl std::fmt::Display for Refusal { Self::TooLarge => formatter.write_str("image over the size ceiling"), Self::Unreached => formatter.write_str("image host unreached"), Self::Status(status) => write!(formatter, "image host answered HTTP {status}"), + Self::Redirects => formatter.write_str("too many redirects for an image"), } } } @@ -238,14 +249,46 @@ async fn fetch_within( if !response.status().is_success() { return Err(Refusal::Status(response.status().as_u16())); } - let declared = response + checked_image(response, Declared::Image).await +} + +/// What the `Content-Type` of an answer must say before its body is read. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum Declared { + /// One of the three image types, and the same one the bytes turn out to + /// be. What a provider's API answers, and so what it is held to. + Image, + /// That, or a type that names no type: none at all, + /// `application/octet-stream` or `binary/octet-stream`. Podcast covers are + /// commonly served from object storage that labels everything so, and the + /// bytes decide either way. A type that names something else — `text/html` + /// above all — is still refused unread. + ImageOrUnlabelled, +} + +/// The body of a successful answer, read under [`MAX_IMAGE_BYTES`] and +/// recognised from its bytes. Shared by the provider fetcher above and the +/// followed one below, so the two cannot come to disagree about what an image +/// is. +async fn checked_image(response: reqwest::Response, rule: Declared) -> Result { + let mime = response .headers() .get(reqwest::header::CONTENT_TYPE) .and_then(|value| value.to_str().ok()) .and_then(|value| value.split(';').next()) - .map(|value| value.trim().to_ascii_lowercase()) - .and_then(|value| format_of_mime(&value)) - .ok_or(Refusal::NotAnImage)?; + .map(|value| value.trim().to_ascii_lowercase()); + let declared = match mime.as_deref().and_then(format_of_mime) { + Some(format) => Some(format), + None if rule == Declared::ImageOrUnlabelled + && matches!( + mime.as_deref(), + None | Some("" | "application/octet-stream" | "binary/octet-stream") + ) => + { + None + } + None => return Err(Refusal::NotAnImage), + }; // The shared bounded read (RFC-023 decision 1), under this fetcher's own // ceiling: an image is allowed to weigh more than an API answer. let bytes = crate::outbound::read_bounded_body(response, MAX_IMAGE_BYTES) @@ -255,7 +298,7 @@ async fn fetch_within( crate::outbound::BodyError::Unread => Refusal::Unreached, })?; let format = sniff(&bytes).ok_or(Refusal::NotAnImage)?; - if format != declared { + if declared.is_some_and(|declared| declared != format) { return Err(Refusal::NotAnImage); } Ok(Portrait { @@ -265,6 +308,48 @@ async fn fetch_within( }) } +/// Fetches an image whose address came from a person or out of a document — +/// a podcast feed's cover, a radio station's image — rather than from a +/// provider on an allowlist. +/// +/// There is no allowlist to hold such an address to, so it is held instead to +/// every rule the podcast fetcher applies to a feed address +/// ([`crate::podcast::open`]): http or https only (http where `policy` allows +/// it), no user information, every address the name resolves to admitted by +/// the address policy, checked before connecting and pinned, and redirects +/// followed by hand, at most [`crate::podcast::MAX_REDIRECTS`], each hop put +/// through the same checks before it is contacted. What comes back is then +/// held to the rules above: [`MAX_IMAGE_BYTES`], and JPEG, PNG or WebP by the +/// bytes. The whole of it, redirects included, within `policy.timeout`. +pub async fn fetch_followed( + url: &url::Url, + policy: crate::podcast::FetchPolicy, +) -> Result { + let within = async { + let response = crate::podcast::open(url, policy) + .await + .map_err(Refusal::from)?; + checked_image(response, Declared::ImageOrUnlabelled).await + }; + tokio::time::timeout(policy.timeout, within) + .await + .unwrap_or(Err(Refusal::Unreached)) +} + +impl From for Refusal { + fn from(error: crate::podcast::FetchError) -> Self { + use crate::podcast::FetchError; + match error { + FetchError::Destination => Self::Destination, + FetchError::Address => Self::Address, + FetchError::Redirects => Self::Redirects, + FetchError::TooLarge => Self::TooLarge, + FetchError::Unreached => Self::Unreached, + FetchError::Status(status) => Self::Status(status), + } + } +} + fn format_of_mime(mime: &str) -> Option<&'static str> { match mime { "image/jpeg" | "image/jpg" => Some("jpg"), diff --git a/src/services/artwork.rs b/src/services/artwork.rs index 608a4b2b..ce4566b7 100644 --- a/src/services/artwork.rs +++ b/src/services/artwork.rs @@ -46,6 +46,12 @@ //! next visit downloads it again, up to the hit TTL — and is the same width, //! since its row is committed in one transaction under this same gate. //! +//! A podcast cover and a station image (RFC-025, amended) are the same case: +//! their bytes came from another host. Both are repaired on the next write +//! that names them — a refresh of the channel, a save of the station — which +//! asks whether the file is still in the store before it trusts an unchanged +//! address, and fetches it again when it is not. +//! //! That is an argument for tolerating the window, not for pretending it is //! shut. It is narrow, it needs a scan and a sweep to interleave at one //! instant, and no test here forces that ordering. @@ -208,10 +214,13 @@ impl DomainServices { Ok(swept) } - /// Drops every `artwork` row no artist, album, track or hand-chosen - /// artist image (RFC-013) names, and says how many went. The choice - /// table's key has no `ON DELETE`: forgetting it here would fail the - /// sweep rather than strand a choice. + /// Drops every `artwork` row no artist, album, track, hand-chosen artist + /// image (RFC-013), podcast channel or radio station (RFC-025, amended) + /// names, and says how many went. The choice table's key has no + /// `ON DELETE`: forgetting it here would fail the sweep rather than strand + /// a choice. A channel's cover and a station's image are released by + /// deleting the channel or the station — or by the feed naming another + /// cover, or the account another image — and collected here. /// /// Three columns rather than the canvas store's one link table, and all /// three are `ON DELETE SET NULL` — which is the reason this is needed at @@ -234,7 +243,9 @@ impl DomainServices { AND NOT EXISTS (SELECT 1 FROM album WHERE album.artwork_hash = artwork.hash) \ AND NOT EXISTS (SELECT 1 FROM track WHERE track.artwork_hash = artwork.hash) \ AND NOT EXISTS (SELECT 1 FROM artist_image_choice c WHERE c.artwork_hash = artwork.hash) \ - AND NOT EXISTS (SELECT 1 FROM artist_image_candidate c WHERE c.artwork_hash = artwork.hash)", + AND NOT EXISTS (SELECT 1 FROM artist_image_candidate c WHERE c.artwork_hash = artwork.hash) \ + AND NOT EXISTS (SELECT 1 FROM podcast_channel p WHERE p.artwork_hash = artwork.hash) \ + AND NOT EXISTS (SELECT 1 FROM radio_station r WHERE r.artwork_hash = artwork.hash)", ) .execute(&mut *tx) .await? diff --git a/src/services/catalog.rs b/src/services/catalog.rs index a1b126ac..a7fd1860 100644 --- a/src/services/catalog.rs +++ b/src/services/catalog.rs @@ -259,6 +259,8 @@ impl DomainServices { UNION SELECT 1 FROM artist ar JOIN library_member m ON m.library_id=ar.library_id WHERE ar.fanart_hash=a.hash AND m.user_id=? \ UNION SELECT 1 FROM artist_image_choice c JOIN artist ar ON ar.id=c.artist_id JOIN library_member m ON m.library_id=ar.library_id WHERE c.artwork_hash=a.hash AND m.user_id=? \ UNION SELECT 1 FROM artist_image_candidate c JOIN artist_image_search s ON s.id=c.search_id JOIN artist ar ON ar.id=s.artist_id JOIN library_member m ON m.library_id=ar.library_id WHERE c.artwork_hash=a.hash AND m.user_id=? AND m.role IN ('owner', 'manager') \ + UNION SELECT 1 FROM podcast_channel pc WHERE pc.artwork_hash=a.hash \ + UNION SELECT 1 FROM radio_station rs WHERE rs.artwork_hash=a.hash AND rs.user_id=? \ ) UNION ALL SELECT a.hash, a.format FROM track t JOIN library_member m ON m.library_id=t.library_id JOIN artwork a ON a.hash=t.artwork_hash WHERE t.id=? AND m.user_id=? \ UNION ALL SELECT a.hash, a.format FROM album al JOIN library_member m ON m.library_id=al.library_id JOIN artwork a ON a.hash=al.artwork_hash WHERE al.id=? AND m.user_id=? \ UNION ALL SELECT a.hash, a.format FROM artist ar JOIN library_member m ON m.library_id=ar.library_id JOIN artwork a ON a.hash=COALESCE((SELECT c.artwork_hash FROM artist_image_choice c WHERE c.artist_id=ar.id AND c.kind='portrait'), ar.artwork_hash) WHERE ar.id=? AND m.user_id=? LIMIT 1", @@ -268,8 +270,11 @@ impl DomainServices { // portrait, the chosen one first. A Deezer preview is served to those // who may pick it: the owners and managers of the artist's library. An // automatic banner (RFC-014), to the members of a library whose artist - // names it. + // names it. A podcast cover (RFC-025, amended), to every account, as + // the channels are one list per instance that every account reads; a + // station's image, to the account the station belongs to. .bind(id).bind(user_id.to_string()).bind(user_id.to_string()).bind(user_id.to_string()).bind(user_id.to_string()).bind(user_id.to_string()).bind(user_id.to_string()) + .bind(user_id.to_string()) .bind(id).bind(user_id.to_string()).bind(id).bind(user_id.to_string()).bind(id).bind(user_id.to_string()) .fetch_optional(self.db.pool()).await?; row.map(|row| Ok::<_, sqlx::Error>((row.try_get("hash")?, row.try_get("format")?))) diff --git a/src/services/mod.rs b/src/services/mod.rs index 23c54578..545d3468 100644 --- a/src/services/mod.rs +++ b/src/services/mod.rs @@ -546,7 +546,7 @@ pub struct TrackMetadataPatch { /// supplies the outer `None` when it is absent — so wrapping whatever the field /// holds in `Some` is exactly the distinction: `Some(None)` for `null`, /// `Some(Some(value))` for a value. -fn present<'de, T, D>(deserializer: D) -> Result>, D::Error> +pub(crate) fn present<'de, T, D>(deserializer: D) -> Result>, D::Error> where T: Deserialize<'de>, D: serde::Deserializer<'de>, @@ -1293,7 +1293,8 @@ pub struct BookmarkItem { /// Not a catalogue entity: no library, no tags, nothing scanned. The server /// stores the address and hands it to the client, and never fetches it — see /// `src/services/radio.rs` for why that single decision is the feature's security -/// answer. +/// answer. The station's image is the one address it fetches, once, when it +/// is set. #[derive(Debug, Clone, Serialize, ToSchema)] pub struct RadioStationItem { pub id: Uuid, @@ -1303,6 +1304,13 @@ pub struct RadioStationItem { /// The station's own page, when it gave one. Omitting it on a write clears /// it, because a station is replaced rather than patched. pub homepage_url: Option, + /// The image address the account gave, kept whether or not it could be + /// fetched. Shown to the station's own account only, as the station is. + pub image_url: Option, + /// The image, fetched once into the artwork store + /// (`/api/v2/artwork/{hash}`). `None` when there is no image address or + /// its fetch failed; a client then shows its default symbol. + pub artwork_hash: Option, pub created_at: i64, pub updated_at: i64, } @@ -1494,6 +1502,11 @@ pub struct DomainServices { /// Podcasts (RFC-025): the settings, and the one download permit and one /// refresh lock the background work shares. podcasts: podcasts::PodcastRuntime, + /// Podcast covers and station images (RFC-025, amended): whether they are + /// fetched, how long one may take, and which addresses it may reach. + remote_artwork: crate::config::RemoteArtworkSettings, + /// How many station images each account has had fetched this hour. + station_images: remote_artwork::FetchBudget, /// Smart playlists (RFC-025): what each reader was last shown, the /// evaluation counter and the evaluation budget. smart: smart_playlists::SmartRuntime, @@ -1563,6 +1576,7 @@ mod playlists; mod podcasts; mod quality; mod radio; +mod remote_artwork; mod remote_lyrics; // RFC-024: what the listening history and the catalogue suggest. mod local_similar; @@ -1619,6 +1633,7 @@ pub use quality::{ AnalysisState, QualityAnalysis, QualitySummary, QualityTrack, QualityVerdict, QualityVerdictCount, Spectrogram, TrackQuality, TrackQualityDetail, }; +pub use radio::StationImage; pub use scrobbling::{ ScrobbleDestinationName, ScrobbleDrain, ScrobbleEnvelope, ScrobbleLinkState, ScrobbleProvider, ScrobbleTarget, ScrobbleUnavailable, ScrobbleVerdict, UncertainScrobble, @@ -1693,6 +1708,8 @@ impl DomainServices { crate::lrclib::LyricsLookup::disabled(), )), podcasts: podcasts::PodcastRuntime::new(config.podcasts.clone()), + remote_artwork: config.remote_artwork, + station_images: remote_artwork::FetchBudget::default(), smart: smart_playlists::SmartRuntime::new(config.db_max_connections), } } diff --git a/src/services/podcasts.rs b/src/services/podcasts.rs index af3c208c..101dc34b 100644 --- a/src/services/podcasts.rs +++ b/src/services/podcasts.rs @@ -16,6 +16,12 @@ //! //! **Network I/O never holds the writer gate.** A fetch runs first; the gate is //! taken for the rows it produced, after. +//! +//! **A channel's cover is fetched by the server** (RFC-025, amended), when the +//! channel is subscribed and on a refresh that finds the feed naming a cover +//! it does not hold — a new address, a failed last attempt, a file gone from +//! the store, or one older than [`COVER_TTL_MS`]. It is stored in the artwork +//! store and handed out by hash; the address is shown to managers only. use std::path::Path; @@ -48,8 +54,15 @@ pub struct PodcastChannelItem { pub title: String, pub description: Option, pub site_url: Option, - /// The cover the feed names. Never fetched by the server. + /// The cover address the feed names, to an account that may manage + /// podcasts, and `None` to everyone else: a private feed's cover can carry + /// its subscriber's token as readily as the feed. Readers are given + /// `artwork_hash` instead. pub image_url: Option, + /// The cover, fetched by the server into the artwork store + /// (`/api/v2/artwork/{hash}`, Subsonic `coverArt`). `None` until a fetch + /// succeeds, or when the feed names none. + pub artwork_hash: Option, /// `completed` when the last fetch succeeded, `error` when it did not. pub status: String, /// Why the last fetch failed, as a word from a fixed list. @@ -67,6 +80,9 @@ pub struct PodcastEpisodeItem { pub id: Uuid, pub channel_id: Uuid, pub channel_title: String, + /// The channel's cover: an episode shows its channel's, and an episode's + /// own `itunes:image` is not fetched. + pub artwork_hash: Option, pub title: String, pub description: Option, pub published_at: Option, @@ -135,6 +151,9 @@ pub(super) struct PodcastRuntime { /// One refresh pass at a time; a second asked for while one runs is /// answered by the one that runs. refreshing: Arc>, + /// Held for reading by every cover fetch a subscription starts in the + /// background, so that taking it for writing waits for all of them. + covers: Arc>, } impl PodcastRuntime { @@ -143,13 +162,15 @@ impl PodcastRuntime { settings, downloads: Arc::new(tokio::sync::Semaphore::new(1)), refreshing: Arc::new(tokio::sync::Mutex::new(())), + covers: Arc::new(tokio::sync::RwLock::new(())), } } } macro_rules! episode_columns { () => { - "SELECT e.id, e.channel_id, c.title AS channel_title, e.title, e.description, \ + "SELECT e.id, e.channel_id, c.title AS channel_title, c.artwork_hash, e.title, \ + e.description, \ e.published_at, e.duration_ms, e.status, e.error_code, e.file_size, e.suffix, \ e.created_at, b.position_ms \ FROM podcast_episode e JOIN podcast_channel c ON c.id=e.channel_id \ @@ -159,8 +180,8 @@ macro_rules! episode_columns { macro_rules! channel_columns { () => { - "SELECT c.id, c.feed_url, c.title, c.description, c.site_url, c.image_url, c.status, \ - c.error_code, c.created_at, c.refreshed_at, \ + "SELECT c.id, c.feed_url, c.title, c.description, c.site_url, c.image_url, \ + c.artwork_hash, c.status, c.error_code, c.created_at, c.refreshed_at, \ (SELECT COUNT(*) FROM podcast_episode e WHERE e.channel_id=c.id \ AND e.status<>'deleted') AS episode_count \ FROM podcast_channel c " @@ -327,6 +348,15 @@ impl DomainServices { if exists.is_some() { return Err(ServiceError::Conflict.into()); } + // Asked before the feed is fetched as well as under the gate below, so + // a full instance contacts nobody; the answer that counts is the one + // the insert is made under. + let count: i64 = sqlx::query_scalar("SELECT COUNT(*) FROM podcast_channel") + .fetch_one(self.db.pool()) + .await?; + if count >= MAX_PODCAST_CHANNELS { + return Err(ServiceError::Conflict.into()); + } let feed = self.fetch_feed(&url).await.map_err(PodcastFailure::Feed)?; let id = Uuid::new_v4(); let now = now_ms(); @@ -366,9 +396,63 @@ impl DomainServices { upsert_episodes(&mut tx, id, &feed.episodes, now).await?; tx.commit().await?; } + // Only once the channel is stored: a subscription refused for the + // ceiling or as a duplicate has fetched no cover and written nothing + // to the store. + // + // **And not in the request.** The feed has already spent up to its own + // deadline, and a cover host as slow again would hold the answer for + // the sum of both — past the native route's timeout, after the channel + // is committed, and without any bound at all on Subsonic's + // `createPodcastChannel`, which no `TimeoutLayer` covers. So the + // subscription answers with the cover pending, and the cover follows + // in the background, under its own deadline. A cover that cannot be + // fetched leaves the channel without one; the next refresh tries + // again. + if let Some(named) = feed.image_url { + // Taken before the task is spawned, so a caller of + // `podcast_covers_settled` that returns after this answer cannot + // miss it. + let pending = Arc::clone(&self.podcasts.covers).read_owned().await; + let this = self.clone(); + tokio::spawn(async move { + let _pending = pending; + this.fill_first_cover(id, &named).await; + }); + } Ok(self.podcast_channel(user_id, id).await?) } + /// The cover of a channel just subscribed, fetched and attached. + async fn fill_first_cover(&self, channel_id: Uuid, named: &str) { + let cover = self + .channel_cover(Some(named), &KnownCover::default()) + .await; + if matches!(cover, CoverWrite::Keep) { + return; + } + let recorded: Result<(), ServiceError> = async { + let _writer = self.db.writer_guard().await; + let mut tx = self.db.begin_write(&_writer).await?; + // Unsubscribed meanwhile: the update names no row, and the file + // is left to the sweep. + write_cover(&mut tx, &channel_id.to_string(), &cover, now_ms()).await?; + tx.commit().await?; + Ok(()) + } + .await; + if let Err(error) = recorded { + tracing::error!(error = %error, "a podcast cover could not be recorded"); + } + } + + /// Waits until every cover fetch a subscription started so far has + /// ended. Public for the tests, which otherwise could not tell a cover + /// refused from one still on its way. + pub async fn podcast_covers_settled(&self) { + let _all = self.podcasts.covers.write().await; + } + /// Unsubscribes: the channel, its episodes, the bookmarks on them, and the /// files on disk. pub async fn delete_podcast_channel( @@ -422,20 +506,28 @@ impl DomainServices { let Ok(_pass) = self.podcasts.refreshing.try_lock() else { return; }; - let channels: Vec<(String, String)> = - match sqlx::query_as("SELECT id, feed_url FROM podcast_channel ORDER BY id") - .fetch_all(self.db.pool()) - .await - { - Ok(channels) => channels, - Err(error) => { - tracing::error!(error = %error, "podcast refresh could not list channels"); - return; - } - }; + type ChannelRow = (String, String, Option, Option, Option); + let channels: Vec = match sqlx::query_as( + "SELECT id, feed_url, artwork_hash, artwork_url, artwork_fetched_at \ + FROM podcast_channel ORDER BY id", + ) + .fetch_all(self.db.pool()) + .await + { + Ok(channels) => channels, + Err(error) => { + tracing::error!(error = %error, "podcast refresh could not list channels"); + return; + } + }; let mut failed = 0usize; - for (id, feed_url) in &channels { - if let Err(error) = self.refresh_channel(id, feed_url).await { + for (id, feed_url, hash, url, fetched_at) in &channels { + let known = KnownCover { + hash: hash.clone(), + url: url.clone(), + fetched_at: *fetched_at, + }; + if let Err(error) = self.refresh_channel(id, feed_url, &known).await { failed += 1; tracing::warn!(error = %error, "a podcast channel could not be refreshed"); } @@ -448,11 +540,22 @@ impl DomainServices { ); } - async fn refresh_channel(&self, id: &str, feed_url: &str) -> Result<(), ServiceError> { + async fn refresh_channel( + &self, + id: &str, + feed_url: &str, + known: &KnownCover, + ) -> Result<(), ServiceError> { let fetched = match url::Url::parse(feed_url) { Ok(url) => self.fetch_feed(&url).await, Err(_) => Err("destination_refused".to_owned()), }; + // A feed that could not be read says nothing about its cover, which + // stays as it was. + let cover = match &fetched { + Ok(feed) => self.channel_cover(feed.image_url.as_deref(), known).await, + Err(_) => CoverWrite::Keep, + }; let now = now_ms(); let _writer = self.db.writer_guard().await; let mut tx = self.db.begin_write(&_writer).await?; @@ -476,6 +579,7 @@ impl DomainServices { if updated.rows_affected() > 0 { upsert_episodes(&mut tx, parse_uuid(id.to_owned())?, &feed.episodes, now) .await?; + write_cover(&mut tx, id, &cover, now).await?; } } Err(code) => { @@ -510,6 +614,38 @@ impl DomainServices { .map_err(podcast::FeedError::code) } + /// What a refresh should do about the cover the feed names: keep the one + /// held, forget it, or the one it has just fetched. + /// + /// A fetch that fails keeps what is held — the old cover, when the feed + /// moved to a new one that cannot be reached — and stores nothing; the + /// next refresh tries again, because the address fetched from still + /// differs from the one named, or because no cover is held at all. + async fn channel_cover(&self, named: Option<&str>, known: &KnownCover) -> CoverWrite { + let file_present = match &known.hash { + Some(hash) => self.artwork_file_present(hash).await, + None => false, + }; + match cover_plan(named, known, file_present, now_ms()) { + CoverPlan::Keep => CoverWrite::Keep, + CoverPlan::Clear => CoverWrite::Clear, + CoverPlan::Fetch(url) => { + let seen = known.fetched_at; + match self + .fetch_remote_artwork(url, self.podcasts.settings.allow_http, "podcast cover") + .await + { + Some(artwork) => CoverWrite::Stored { + artwork, + url: url.to_owned(), + seen, + }, + None => CoverWrite::Keep, + } + } + } + } + /// Queues an episode's download and answers at once. /// /// An episode already stored stays as it is; one that failed, or was @@ -932,6 +1068,125 @@ struct StoredEpisode { suffix: &'static str, } +/// How long a cover fetched from an unchanged address is trusted before it is +/// fetched again: a feed can replace the image behind the same address. A +/// week, against the default refresh every six hours, is one cover fetch per +/// channel per week rather than four a day. +const COVER_TTL_MS: i64 = 7 * 24 * 60 * 60 * 1000; + +/// What a channel holds of its cover. +#[derive(Debug, Default)] +struct KnownCover { + hash: Option, + /// The address `hash` was fetched from. + url: Option, + fetched_at: Option, +} + +/// What [`cover_plan`] decides, before anything is fetched. +#[derive(Debug, PartialEq, Eq)] +enum CoverPlan<'a> { + Keep, + Clear, + Fetch(&'a str), +} + +/// What a refresh writes about the cover. +enum CoverWrite { + Keep, + Clear, + Stored { + artwork: super::remote_artwork::FetchedArtwork, + /// The address it was fetched from. + url: String, + /// `artwork_fetched_at` as it was read before the fetch began. Any + /// other value at write time means a cover was recorded meanwhile, + /// and that one is the newer, whatever order the fetches end in. + seen: Option, + }, +} + +/// Whether the cover a feed names has to be fetched. +/// +/// Only when the feed names one and the channel does not already hold it: +/// fetched from that very address, its file still in the store, and fetched +/// less than [`COVER_TTL_MS`] ago. A feed that names none any more has its +/// cover forgotten, which releases it to the sweep. +fn cover_plan<'a>( + named: Option<&'a str>, + known: &KnownCover, + file_present: bool, + now: i64, +) -> CoverPlan<'a> { + let Some(named) = named else { + return if known.hash.is_some() || known.url.is_some() { + CoverPlan::Clear + } else { + CoverPlan::Keep + }; + }; + let held = known.hash.is_some() + && file_present + && known.url.as_deref() == Some(named) + && known + .fetched_at + .is_some_and(|at| now.saturating_sub(at) < COVER_TTL_MS); + if held { + CoverPlan::Keep + } else { + CoverPlan::Fetch(named) + } +} + +/// Writes a cover decision onto a channel, under the writer gate: in the +/// transaction of a refresh, or in its own after a subscription. +/// +/// **Only while the decision still holds.** A cover is fetched outside the +/// gate, and two fetches for one channel can be in flight at once — the +/// subscription's, in the background, and a refresh's. So the write names what +/// the fetch assumed: the channel still exists, it still names the address the +/// cover came from (or, to clear it, names none), and no cover was recorded +/// since this fetch began. Otherwise nothing is written, the newer state +/// stands, and an `artwork` row recorded for the dropped cover names nothing; +/// the sweep collects it with its file. +async fn write_cover( + tx: &mut sqlx::Transaction<'static, sqlx::Sqlite>, + channel_id: &str, + cover: &CoverWrite, + now: i64, +) -> Result<(), sqlx::Error> { + match cover { + CoverWrite::Keep => {} + CoverWrite::Clear => { + sqlx::query( + "UPDATE podcast_channel SET artwork_hash=NULL, artwork_url=NULL, \ + artwork_fetched_at=NULL WHERE id=? AND image_url IS NULL", + ) + .bind(channel_id) + .execute(&mut **tx) + .await?; + } + CoverWrite::Stored { artwork, url, seen } => { + super::remote_artwork::record_artwork(tx, artwork, "podcast", now).await?; + // `IS`, not `=`: a channel that held no cover reads `NULL`, and + // must still read `NULL` for this write to be the first. + sqlx::query( + "UPDATE podcast_channel SET artwork_hash=?, artwork_url=?, artwork_fetched_at=? \ + WHERE id=? AND image_url=? AND artwork_fetched_at IS ?", + ) + .bind(&artwork.hash) + .bind(url) + .bind(now) + .bind(channel_id) + .bind(url) + .bind(seen) + .execute(&mut **tx) + .await?; + } + } + Ok(()) +} + /// Streams the answer at `url` into `staging`, at most `ceiling` bytes, /// answering its size, its BLAKE3 and its first bytes. async fn write_bounded( @@ -1049,7 +1304,10 @@ fn channel_from_row( title: row.try_get("title")?, description: row.try_get("description")?, site_url: row.try_get("site_url")?, - image_url: row.try_get("image_url")?, + image_url: row + .try_get::, _>("image_url")? + .filter(|_| can_manage), + artwork_hash: row.try_get("artwork_hash")?, status: row.try_get("status")?, error: row.try_get("error_code")?, created_at: row.try_get("created_at")?, @@ -1065,6 +1323,7 @@ fn episode_from_row(row: sqlx::sqlite::SqliteRow) -> Result { + /// Leaves it as it is: what the Subsonic writes do, since the protocol has + /// no image parameter, and what a native write that does not mention the + /// field does. + Keep, + /// Sets it, or clears it with `None` or a blank value. + Set(Option<&'a str>), +} + +macro_rules! station_select { + () => { + "SELECT id, name, stream_url, homepage_url, image_url, artwork_hash, created_at, \ + updated_at FROM radio_station " + }; +} + /// How many stations one account may keep. /// /// Not a protocol limit — there is none — but a write surface a client drives @@ -42,10 +84,10 @@ impl DomainServices { user_id: Uuid, ) -> Result, ServiceError> { let mut connection = self.db.pool().acquire().await?; - let rows = sqlx::query( - "SELECT id, name, stream_url, homepage_url, created_at, updated_at \ - FROM radio_station WHERE user_id=? ORDER BY name COLLATE NOCASE, id", - ) + let rows = sqlx::query(concat!( + station_select!(), + "WHERE user_id=? ORDER BY name COLLATE NOCASE, id" + )) .bind(user_id.to_string()) .fetch_all(&mut *connection) .await?; @@ -60,15 +102,12 @@ impl DomainServices { station_id: Uuid, ) -> Result { let mut connection = self.db.pool().acquire().await?; - let row = sqlx::query( - "SELECT id, name, stream_url, homepage_url, created_at, updated_at \ - FROM radio_station WHERE user_id=? AND id=?", - ) - .bind(user_id.to_string()) - .bind(station_id.to_string()) - .fetch_optional(&mut *connection) - .await? - .ok_or(ServiceError::NotFound)?; + let row = sqlx::query(concat!(station_select!(), "WHERE user_id=? AND id=?")) + .bind(user_id.to_string()) + .bind(station_id.to_string()) + .fetch_optional(&mut *connection) + .await? + .ok_or(ServiceError::NotFound)?; radio_station_from_row(row) } @@ -77,67 +116,85 @@ impl DomainServices { /// Nothing is deduplicated. Two rows may carry the same address under two /// names, because `updateInternetRadioStation` has no conflict code to /// report with and a client told "failed" would have nothing to act on. + /// + /// An `image_url` is fetched once the station is stored, never before: a + /// write refused for the ceiling has contacted nobody. A station whose + /// image cannot be fetched is kept, with the address and without the + /// image. pub async fn create_radio_station( &self, user_id: Uuid, name: &str, stream_url: &str, homepage_url: Option<&str>, + image_url: Option<&str>, ) -> Result { let name = validated_name(name)?; let stream_url = validated_url(stream_url)?; let homepage_url = validated_homepage(homepage_url)?; + let image_url = validated_homepage(image_url)?; let id = Uuid::new_v4(); let now = now_ms(); - let _writer = self.db.writer_guard().await; - let mut tx = self.db.begin_write(&_writer).await?; - // **Two locks make the count safe**, and they cover different writers. - // The writer gate serialises this process's mutations, so two creations - // here cannot both read 499 and both insert. `begin_write` opens with - // `BEGIN IMMEDIATE`, so SQLite's write lock is held before the - // `SELECT COUNT(*)` runs: a writer that bypasses the gate — another - // process on the same file, say — cannot insert between the count and - // the insert either. - let count: i64 = sqlx::query_scalar("SELECT COUNT(*) FROM radio_station WHERE user_id=?") + { + let _writer = self.db.writer_guard().await; + let mut tx = self.db.begin_write(&_writer).await?; + // **Two locks make the count safe**, and they cover different + // writers. The writer gate serialises this process's mutations, so + // two creations here cannot both read 499 and both insert. + // `begin_write` opens with `BEGIN IMMEDIATE`, so SQLite's write + // lock is held before the `SELECT COUNT(*)` runs: a writer that + // bypasses the gate — another process on the same file, say — + // cannot insert between the count and the insert either. + let count: i64 = + sqlx::query_scalar("SELECT COUNT(*) FROM radio_station WHERE user_id=?") + .bind(user_id.to_string()) + .fetch_one(&mut *tx) + .await?; + if count >= MAX_RADIO_STATIONS { + tx.rollback().await?; + return Err(ServiceError::Conflict); + } + sqlx::query( + "INSERT INTO radio_station \ + (id, user_id, name, stream_url, homepage_url, image_url, artwork_hash, \ + created_at, updated_at) \ + VALUES (?, ?, ?, ?, ?, ?, NULL, ?, ?)", + ) + .bind(id.to_string()) .bind(user_id.to_string()) - .fetch_one(&mut *tx) + .bind(&name) + .bind(&stream_url) + .bind(homepage_url.as_deref()) + .bind(image_url.as_deref()) + .bind(now) + .bind(now) + .execute(&mut *tx) .await?; - if count >= MAX_RADIO_STATIONS { - tx.rollback().await?; - return Err(ServiceError::Conflict); + tx.commit().await?; } - sqlx::query( - "INSERT INTO radio_station \ - (id, user_id, name, stream_url, homepage_url, created_at, updated_at) \ - VALUES (?, ?, ?, ?, ?, ?, ?)", - ) - .bind(id.to_string()) - .bind(user_id.to_string()) - .bind(&name) - .bind(&stream_url) - .bind(homepage_url.as_deref()) - .bind(now) - .bind(now) - .execute(&mut *tx) - .await?; - tx.commit().await?; - Ok(RadioStationItem { - id, - name, - stream_url, - homepage_url, - created_at: now, - updated_at: now, - }) + if let Some(url) = &image_url { + self.attach_station_image(user_id, id, url).await?; + } + self.radio_station(user_id, id).await } - /// Replaces a station's three fields. + /// Replaces a station's three fields, and its image when told to. /// /// A replacement and not a patch, on both surfaces: the protocol's /// `updateInternetRadioStation` requires the name and the address every /// time, so omitting the homepage there clears it. A native `PATCH` that /// kept the old value instead would make the same call mean two different /// things depending on which door it came through. + /// + /// The image is the exception, and for the same reason turned around: the + /// protocol has no image parameter, so a Subsonic edit says nothing about + /// it and [`StationImage::Keep`] leaves it alone — otherwise renaming a + /// station in any Subsonic client would take its image away. An address + /// that is the one already stored, with its image in the store, is not + /// fetched again; any other is, once the replacement is committed — so a + /// station that is not this account's is 404 before anything is + /// contacted. A failure stores the address with no image, and saving the + /// station again is how a fetch is retried. pub async fn update_radio_station( &self, user_id: Uuid, @@ -145,44 +202,83 @@ impl DomainServices { name: &str, stream_url: &str, homepage_url: Option<&str>, + image: StationImage<'_>, ) -> Result { let name = validated_name(name)?; let stream_url = validated_url(stream_url)?; let homepage_url = validated_homepage(homepage_url)?; + let image = match image { + StationImage::Keep => None, + StationImage::Set(value) => Some(validated_homepage(value)?), + }; let now = now_ms(); - let _writer = self.db.writer_guard().await; - let mut connection = self.db.pool().acquire().await?; - let updated = sqlx::query( - "UPDATE radio_station SET name=?, stream_url=?, homepage_url=?, updated_at=? \ - WHERE user_id=? AND id=?", - ) - .bind(&name) - .bind(&stream_url) - .bind(homepage_url.as_deref()) - .bind(now) - .bind(user_id.to_string()) - .bind(station_id.to_string()) - .execute(&mut *connection) - .await?; - if updated.rows_affected() == 0 { - return Err(ServiceError::NotFound); + // The address to fetch once the replacement is in, and the hash kept + // from before when the address did not change. + let (due, kept) = { + let _writer = self.db.writer_guard().await; + let mut tx = self.db.begin_write(&_writer).await?; + let held: Option<(Option, Option)> = sqlx::query_as( + "SELECT image_url, artwork_hash FROM radio_station WHERE user_id=? AND id=?", + ) + .bind(user_id.to_string()) + .bind(station_id.to_string()) + .fetch_optional(&mut *tx) + .await?; + let Some((held_url, held_hash)) = held else { + tx.rollback().await?; + return Err(ServiceError::NotFound); + }; + let plan = match image { + None => { + sqlx::query( + "UPDATE radio_station SET name=?, stream_url=?, homepage_url=?, \ + updated_at=? WHERE user_id=? AND id=?", + ) + .bind(&name) + .bind(&stream_url) + .bind(homepage_url.as_deref()) + .bind(now) + .bind(user_id.to_string()) + .bind(station_id.to_string()) + .execute(&mut *tx) + .await?; + (None, None) + } + Some(image_url) => { + let kept = held_hash.filter(|_| image_url.is_some() && image_url == held_url); + sqlx::query( + "UPDATE radio_station SET name=?, stream_url=?, homepage_url=?, \ + image_url=?, artwork_hash=?, updated_at=? \ + WHERE user_id=? AND id=?", + ) + .bind(&name) + .bind(&stream_url) + .bind(homepage_url.as_deref()) + .bind(image_url.as_deref()) + .bind(kept.as_deref()) + .bind(now) + .bind(user_id.to_string()) + .bind(station_id.to_string()) + .execute(&mut *tx) + .await?; + (image_url, kept) + } + }; + tx.commit().await?; + plan + }; + if let Some(url) = due { + // Kept from before only while its file is still in the store; a + // file the sweep's window carried off is fetched again. + let held = match &kept { + Some(hash) => self.artwork_file_present(hash).await, + None => false, + }; + if !held { + self.attach_station_image(user_id, station_id, &url).await?; + } } - // `created_at` is the only field the caller did not supply, so it is the - // only reason to read the row back. - let created_at: i64 = - sqlx::query_scalar("SELECT created_at FROM radio_station WHERE user_id=? AND id=?") - .bind(user_id.to_string()) - .bind(station_id.to_string()) - .fetch_one(&mut *connection) - .await?; - Ok(RadioStationItem { - id: station_id, - name, - stream_url, - homepage_url, - created_at, - updated_at: now, - }) + self.radio_station(user_id, station_id).await } /// Removes a station. @@ -191,6 +287,9 @@ impl DomainServices { /// the track it sits on, so removing one that is not there is not the same /// request answered twice — it names something that does not exist, and the /// account cannot tell that from one belonging to somebody else. + /// + /// Its image goes with it: nothing names the hash any more, and the + /// artwork sweep collects it. pub async fn delete_radio_station( &self, user_id: Uuid, @@ -208,6 +307,55 @@ impl DomainServices { } Ok(()) } + + /// Fetches the image of a station already stored, and attaches it. + /// + /// Called only after the write that set the address has committed. The + /// attaching write names the station, its account and the address the + /// image was fetched for, so an image that arrives after the station was + /// deleted, or after its address changed again, attaches to nothing — its + /// file is then named by no row, and the sweep collects it. + /// + /// Within the account's hourly budget only: past it, nothing is fetched and + /// the station stays without an image until a later save. Plain http is + /// allowed: a station's own logo is as often served that way as its + /// stream is, and the address rules apply either way. + async fn attach_station_image( + &self, + user_id: Uuid, + station_id: Uuid, + url: &str, + ) -> Result<(), ServiceError> { + if !self.remote_artwork.enabled { + return Ok(()); + } + if !self.station_images.take(user_id, now_ms()) { + tracing::warn!( + subject = "station image", + reason = "hourly budget spent", + "remote artwork not fetched" + ); + return Ok(()); + } + let Some(fetched) = self.fetch_remote_artwork(url, true, "station image").await else { + return Ok(()); + }; + let now = now_ms(); + let _writer = self.db.writer_guard().await; + let mut tx = self.db.begin_write(&_writer).await?; + record_artwork(&mut tx, &fetched, "station", now).await?; + sqlx::query( + "UPDATE radio_station SET artwork_hash=? WHERE user_id=? AND id=? AND image_url=?", + ) + .bind(&fetched.hash) + .bind(user_id.to_string()) + .bind(station_id.to_string()) + .bind(url) + .execute(&mut *tx) + .await?; + tx.commit().await?; + Ok(()) + } } fn radio_station_from_row(row: sqlx::sqlite::SqliteRow) -> Result { @@ -216,6 +364,8 @@ fn radio_station_from_row(row: sqlx::sqlite::SqliteRow) -> Result>, +} + +impl FetchBudget { + /// Whether `account` may start one more fetch at `now`, counting it if so. + pub(super) fn take(&self, account: Uuid, now: i64) -> bool { + let mut entry = self.spent.entry(account).or_insert((now, 0)); + if now.saturating_sub(entry.0) >= BUDGET_WINDOW_MS { + *entry = (now, 0); + } + if entry.1 >= STATION_IMAGE_FETCHES_PER_HOUR { + return false; + } + entry.1 += 1; + true + } +} + +/// An image fetched and written to the store, not yet named by a row. +#[derive(Debug, Clone)] +pub(super) struct FetchedArtwork { + pub(super) hash: String, + pub(super) format: &'static str, + pub(super) byte_size: i64, +} + +impl DomainServices { + /// Fetches the image at `url` and puts it in the artwork store. + /// + /// `None` when remote artwork is switched off, when the address is not one + /// the rules admit, when the answer is not an image within the ceiling, or + /// when the store cannot be written. Each is logged as a verdict, under + /// `subject` (`podcast cover` or `station image`), and nothing is kept: the + /// caller tries again on the next refresh or the next save. + pub(super) async fn fetch_remote_artwork( + &self, + url: &str, + allow_http: bool, + subject: &'static str, + ) -> Option { + if !self.remote_artwork.enabled { + return None; + } + let Ok(parsed) = url::Url::parse(url) else { + tracing::warn!(subject, reason = "unparsable", "remote artwork not fetched"); + return None; + }; + let policy = crate::podcast::FetchPolicy { + addresses: self.remote_artwork.addresses, + allow_http, + timeout: self.remote_artwork.timeout, + }; + let image = match crate::portrait::fetch_followed(&parsed, policy).await { + Ok(image) => image, + Err(refusal) => { + tracing::warn!(subject, reason = %refusal, "remote artwork not fetched"); + return None; + } + }; + if let Err(error) = crate::portrait::store(&self.artwork_dir, &image).await { + tracing::warn!(subject, %error, "remote artwork could not be stored"); + return None; + } + Some(FetchedArtwork { + byte_size: i64::try_from(image.bytes.len()).unwrap_or(i64::MAX), + hash: image.hash, + format: image.format, + }) + } + + /// Whether the file behind an `artwork` row is in the store. + /// + /// Asked before an unchanged address is trusted to have its cover: the + /// sweep's window (see `artwork.rs`) can carry off a file whose row was + /// written an instant later, and a cover from another host, unlike a + /// scanned one, has no scan to write it back. Re-fetching is the repair. + pub(super) async fn artwork_file_present(&self, hash: &str) -> bool { + let format: Option = sqlx::query_scalar("SELECT format FROM artwork WHERE hash=?") + .bind(hash) + .fetch_optional(self.db.pool()) + .await + .ok() + .flatten(); + let Some(format) = format else { + return false; + }; + tokio::fs::try_exists(self.artwork_dir.join(format!("{hash}.{format}"))) + .await + .unwrap_or(false) + } +} + +/// The `artwork` row for a fetched image, in the transaction that names it, +/// so the sweep never sees the row unreferenced. +pub(super) async fn record_artwork( + tx: &mut sqlx::Transaction<'static, sqlx::Sqlite>, + artwork: &FetchedArtwork, + source: &'static str, + now: i64, +) -> Result<(), sqlx::Error> { + sqlx::query( + "INSERT INTO artwork (hash, format, source, byte_size, created_at) \ + VALUES (?, ?, ?, ?, ?) ON CONFLICT (hash) DO NOTHING", + ) + .bind(&artwork.hash) + .bind(artwork.format) + .bind(source) + .bind(artwork.byte_size) + .bind(now) + .execute(&mut **tx) + .await?; + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn an_account_spends_its_hourly_budget_and_gets_it_back_an_hour_later() { + let budget = FetchBudget::default(); + let (one, other) = (Uuid::new_v4(), Uuid::new_v4()); + for _ in 0..STATION_IMAGE_FETCHES_PER_HOUR { + assert!(budget.take(one, 1_000)); + } + assert!(!budget.take(one, 1_000 + BUDGET_WINDOW_MS - 1)); + // Another account's budget is its own. + assert!(budget.take(other, 1_000)); + assert!(budget.take(one, 1_000 + BUDGET_WINDOW_MS)); + } +} diff --git a/src/subsonic/podcasts.rs b/src/subsonic/podcasts.rs index 37962ad0..f76c30fb 100644 --- a/src/subsonic/podcasts.rs +++ b/src/subsonic/podcasts.rs @@ -103,9 +103,11 @@ pub(super) async fn podcasts( } } -/// A channel as `PodcastChannel` has it. The cover is the feed's own address, -/// as `originalImageUrl`; there is no `coverArt`, because the server does not -/// fetch the image (decision 10). +/// A channel as `PodcastChannel` has it. The cover is `coverArt`, the hash of +/// the copy the server fetched into the artwork store, which `getCoverArt` +/// serves (decision 10, amended). The feed's own address for it, +/// `originalImageUrl`, is given to a manager only, like the feed's: a private +/// feed's cover can carry its subscriber's token. fn channel_node(channel: &PodcastChannelItem) -> Node { Node::new("channel") .attr("id", channel.id.to_string()) @@ -121,6 +123,7 @@ fn channel_node(channel: &PodcastChannelItem) -> Node { ) .attr("title", channel.title.clone()) .maybe_attr("description", channel.description.clone()) + .maybe_attr("coverArt", channel.artwork_hash.clone()) .maybe_attr("originalImageUrl", channel.image_url.clone()) .attr("status", channel.status.clone()) .maybe_attr("errorMessage", channel.error.clone()) @@ -141,6 +144,8 @@ pub(super) fn episode_node(episode: &PodcastEpisodeItem) -> Node { .attr("title", episode.title.clone()) .attr("album", episode.channel_title.clone()) .attr("artist", episode.channel_title.clone()) + // The channel's cover: an episode's own `itunes:image` is not fetched. + .maybe_attr("coverArt", episode.artwork_hash.clone()) .maybe_attr("description", episode.description.clone()) .maybe_attr("publishDate", episode.published_at.map(iso_time)) .maybe_attr( diff --git a/src/subsonic/radio.rs b/src/subsonic/radio.rs index 04cc1858..2ca6b902 100644 --- a/src/subsonic/radio.rs +++ b/src/subsonic/radio.rs @@ -51,6 +51,9 @@ pub(super) async fn radio_stations( params.first("name").ok_or_else(missing)?, params.first("streamUrl").ok_or_else(missing)?, params.first("homepageUrl"), + // The protocol has no image parameter: a station created + // here has none, and one edited here keeps its own. + None, ) .await .map_err(service_protocol)?; @@ -65,6 +68,7 @@ pub(super) async fn radio_stations( params.first("name").ok_or_else(missing)?, params.first("streamUrl").ok_or_else(missing)?, params.first("homepageUrl"), + crate::services::StationImage::Keep, ) .await .map_err(service_protocol)?; diff --git a/tests/podcasts.rs b/tests/podcasts.rs index 1618109a..a07451b7 100644 --- a/tests/podcasts.rs +++ b/tests/podcasts.rs @@ -5,10 +5,17 @@ //! length is not declared, one declaring entities, redirects to a private //! address for a feed and for an enclosure, a redirect loop, and an enclosure //! that is a page of HTML. +//! +//! So are the images fetched from another host (RFC-025, amended): a feed's +//! cover and a radio station's image go through one fetcher, so they are +//! proved against the same host — an image over the ceiling, a page of HTML +//! labelled as an image, a redirect to a private address, an unchanged address +//! not fetched twice, and the sweep keeping what a channel or a station names +//! until it is deleted. use std::sync::{ atomic::{AtomicBool, Ordering}, - Arc, + Arc, Mutex, }; use axum::{ @@ -43,6 +50,12 @@ struct Host { hits: Arc>, /// The feed answers 500 while this is set. failing: Arc, + /// `/cover.jpg` answers 500 while this is set. + cover_failing: Arc, + /// The cover `/feed.xml` names, as a path on this host. + cover: Arc>, + /// `/gated-cover.jpg` holds its answer until this is set. + gate_open: Arc, } impl Host { @@ -51,13 +64,32 @@ impl Host { } } +/// A JPEG by its first bytes, which is all the server checks: the start of +/// image and an APP0 marker, `seed` to tell two covers apart, padding. +fn jpeg(seed: u8) -> Vec { + let mut bytes = vec![0xff, 0xd8, 0xff, 0xe0, seed]; + bytes.extend((0..4096).map(|index| (index % 251) as u8)); + bytes +} + +/// A PNG by its signature. +fn png() -> Vec { + let mut bytes = b"\x89PNG\r\n\x1a\n".to_vec(); + bytes.extend((0..4096).map(|index| (index % 241) as u8)); + bytes +} + /// The feed, its enclosures carrying `tail` — a private feed's enclosures /// carry its subscriber's token too. -fn feed(base: &str) -> String { - private_feed(base, "") +fn feed(base: &str, cover: &str) -> String { + private_feed(base, "", cover) } -fn private_feed(base: &str, tail: &str) -> String { +/// A feed naming `cover` as its image: a path on the host, with the private +/// feed's token on it as `cover=` rather than `auth=`, so a test can tell the +/// cover's address from an enclosure's. +fn private_feed(base: &str, tail: &str, cover: &str) -> String { + let cover_tail = tail.replacen("?auth=", "?cover=", 1); format!( r#" @@ -65,7 +97,7 @@ fn private_feed(base: &str, tail: &str) -> String { Night Shift Talk after dark {base}/ - + Through a redirectep-1 Fri, 05 Jan 2024 10:00:00 +000012:00 @@ -96,19 +128,105 @@ async fn serve(State(host): State, request: Request) -> Response { { return ( [(header::CONTENT_TYPE, "application/rss+xml")], - private_feed(&host.base, &format!("?auth={token}")), + private_feed(&host.base, &format!("?auth={token}"), "/cover.jpg"), + ) + .into_response(); + } + // A feed naming whatever follows the prefix as its cover, for the + // hostile covers below. + if let Some(cover) = path.strip_prefix("/covered") { + return ( + [(header::CONTENT_TYPE, "application/rss+xml")], + feed(&host.base, cover), ) .into_response(); } + let image = |mime: &'static str, bytes: Vec| { + ([(header::CONTENT_TYPE, mime)], bytes).into_response() + }; match path.as_str() { "/feed.xml" if host.failing.load(Ordering::SeqCst) => { StatusCode::INTERNAL_SERVER_ERROR.into_response() } - "/feed.xml" => ( - [(header::CONTENT_TYPE, "application/rss+xml")], - feed(&host.base), - ) - .into_response(), + "/feed.xml" => { + let cover = host.cover.lock().unwrap().clone(); + ( + [(header::CONTENT_TYPE, "application/rss+xml")], + feed(&host.base, &cover), + ) + .into_response() + } + "/cover.jpg" if host.cover_failing.load(Ordering::SeqCst) => { + StatusCode::INTERNAL_SERVER_ERROR.into_response() + } + "/cover.jpg" => image("image/jpeg", jpeg(1)), + "/cover-2.jpg" => image("image/jpeg", jpeg(2)), + // The gated cover below, for its first request only; every later one + // answers at once with other bytes, as if the image behind the + // address changed. + "/gated-once.jpg" => { + if host.count("/gated-once.jpg") > 1 { + return image("image/jpeg", jpeg(8)); + } + for _ in 0..2_000 { + if host.gate_open.load(Ordering::SeqCst) { + break; + } + tokio::time::sleep(std::time::Duration::from_millis(5)).await; + } + image("image/jpeg", jpeg(7)) + } + // A cover host that answers only once the test opens the gate. + "/gated-cover.jpg" => { + for _ in 0..2_000 { + if host.gate_open.load(Ordering::SeqCst) { + break; + } + tokio::time::sleep(std::time::Duration::from_millis(5)).await; + } + image("image/jpeg", jpeg(7)) + } + // A cover host that answers, eventually: longer than any request + // should be held open for it. + "/slow-cover.jpg" => { + tokio::time::sleep(std::time::Duration::from_secs(60)).await; + image("image/jpeg", jpeg(6)) + } + // Object storage labels everything so; the bytes decide. + "/unlabelled.jpg" => image("application/octet-stream", jpeg(3)), + "/station.png" => image("image/png", png()), + "/redirect-cover" => redirect("/cover-2.jpg"), + // Over the 10 MiB ceiling, declared. + "/huge.jpg" => { + let mut bytes = jpeg(4); + bytes.resize(11 * 1024 * 1024, 0); + image("image/jpeg", bytes) + } + // Over the ceiling and never saying so: met while reading. + "/endless.jpg" => { + let head = futures_util::stream::once(async { Ok::<_, std::io::Error>(jpeg(5)) }); + let tail = futures_util::stream::iter( + (0..200).map(|_| Ok::<_, std::io::Error>(vec![0u8; 64 * 1024])), + ); + ( + [(header::CONTENT_TYPE, "image/jpeg")], + Body::from_stream(futures_util::StreamExt::chain(head, tail)), + ) + .into_response() + } + // A page of HTML, labelled as an image and as what it is. + "/page-as-image.jpg" => image( + "image/jpeg", + b"not a cover".to_vec(), + ), + "/page.htm" => image( + "text/html", + b"not a cover".to_vec(), + ), + // Over https, so that the address check is the only rule that refuses + // the hop: a plain-http hop would be refused for its scheme first. + "/cover-to-private" => redirect("https://10.255.255.1/cover.jpg"), + "/cover-to-mapped-private" => redirect("https://[::ffff:192.168.1.1]/cover.jpg"), // Streamed, so no length is declared and the ceiling has to be met // while reading. "/big.xml" => { @@ -148,6 +266,9 @@ async fn fake_host() -> Host { base: Arc::new(format!("http://127.0.0.1:{port}")), hits: Arc::default(), failing: Arc::default(), + cover_failing: Arc::default(), + cover: Arc::new(Mutex::new("/cover.jpg".to_owned())), + gate_open: Arc::default(), }; let router = Router::new().fallback(serve).with_state(host.clone()); tokio::spawn(async move { axum::serve(listener, router).await.unwrap() }); @@ -214,6 +335,30 @@ async fn subscribe(router: &Router, token: &str, url: &str) -> (StatusCode, serd (status, json_body(response).await) } +/// Subscribes, waits for the cover the subscription fetches in the background, +/// and answers the channel as it then reads. +async fn subscribe_settled( + router: &Router, + state: &waveflow_server::AppState, + token: &str, + url: &str, +) -> (StatusCode, serde_json::Value) { + let (status, channel) = subscribe(router, token, url).await; + if status != StatusCode::CREATED { + return (status, channel); + } + state.services.podcast_covers_settled().await; + let read = send_json( + router, + "GET", + &format!("/api/v2/podcasts/{}", uuid_of(&channel)), + token, + None, + ) + .await; + (status, json_body(read).await) +} + fn episode<'a>(channel: &'a serde_json::Value, title: &str) -> &'a serde_json::Value { channel["episodes"] .as_array() @@ -875,3 +1020,724 @@ async fn body_bytes(response: Response) -> Vec { async fn json_or_xml_code(response: Response) -> String { String::from_utf8(body_bytes(response).await).unwrap() } + +/// The hash the store files an image under. +fn hash_of(bytes: &[u8]) -> String { + blake3::hash(bytes).to_hex().to_string() +} + +/// The bytes `/api/v2/artwork/{hash}` answers, or its status when it does not. +async fn artwork(router: &Router, token: &str, hash: &str) -> Result, StatusCode> { + let response = send_json( + router, + "GET", + &format!("/api/v2/artwork/{hash}"), + token, + None, + ) + .await; + if response.status() == StatusCode::OK { + Ok(body_bytes(response).await) + } else { + Err(response.status()) + } +} + +async fn artwork_rows(state: &waveflow_server::AppState) -> i64 { + sqlx::query_scalar("SELECT COUNT(*) FROM artwork") + .fetch_one(state.db.pool()) + .await + .unwrap() +} + +async fn station( + router: &Router, + method: &str, + uri: &str, + token: &str, + body: serde_json::Value, +) -> (StatusCode, serde_json::Value) { + let response = send_json(router, method, uri, token, Some(body)).await; + let status = response.status(); + (status, json_body(response).await) +} + +/// RFC-025, amended: a feed's cover is fetched into the artwork store and +/// handed out by its hash — natively, and as Subsonic `coverArt` on channels +/// and episodes — to every account, while the address it came from is shown +/// to a manager only. An unchanged address is not fetched again; a new one +/// is, once, through a redirect, and the cover it replaces is released. +#[tokio::test] +async fn a_feeds_cover_is_fetched_once_and_served_by_its_hash() { + let host = fake_host().await; + let (_temp, config, state) = podcast_app(|_| {}).await; + let (_, admin_password, admin_key) = account(&state, "cover-admin", AccountRole::Admin).await; + let (reader, reader_password, reader_key) = + account(&state, "cover-reader", AccountRole::User).await; + let router = waveflow_server::app(&config, state.clone()); + let admin_token = login_token(&router, "cover-admin", &admin_password).await; + let reader_token = login_token(&router, "cover-reader", &reader_password).await; + + let (status, channel) = subscribe_settled( + &router, + &state, + &admin_token, + &format!("{}/feed.xml", host.base), + ) + .await; + assert_eq!(status, StatusCode::CREATED, "{channel}"); + let hash = hash_of(&jpeg(1)); + assert_eq!(channel["artwork_hash"], hash.as_str(), "{channel}"); + assert_eq!(host.count("/cover.jpg"), 1); + assert_eq!( + channel["image_url"], + format!("{}/cover.jpg", host.base).as_str(), + "a manager is shown the address the feed names" + ); + let channel_id = uuid_of(&channel); + + // A reader is given the hash, never the address. + let read = json_body( + send_json( + &router, + "GET", + &format!("/api/v2/podcasts/{channel_id}"), + &reader_token, + None, + ) + .await, + ) + .await; + assert!(read["image_url"].is_null(), "{read}"); + assert_eq!(read["artwork_hash"], hash.as_str()); + assert_eq!( + episode(&read, "Through a redirect")["artwork_hash"], + hash.as_str(), + "an episode shows its channel's cover" + ); + assert_eq!(artwork(&router, &reader_token, &hash).await, Ok(jpeg(1))); + + // Subsonic: `coverArt` on the channel and its episodes, and served by + // `getCoverArt`; `originalImageUrl` to a manager only. + let listed = subsonic_json(&router, "getPodcasts", &reader_key, "").await; + let listed = &listed["subsonic-response"]["podcasts"]["channel"][0]; + assert_eq!(listed["coverArt"], hash.as_str(), "{listed}"); + assert!(omits(listed, "originalImageUrl"), "{listed}"); + assert_eq!(listed["episode"][0]["coverArt"], hash.as_str()); + let newest = subsonic_json(&router, "getNewestPodcasts", &reader_key, "").await; + assert_eq!( + newest["subsonic-response"]["newestPodcasts"]["episode"][0]["coverArt"], + hash.as_str() + ); + let managed = subsonic_json(&router, "getPodcasts", &admin_key, "").await; + assert_eq!( + managed["subsonic-response"]["podcasts"]["channel"][0]["originalImageUrl"], + format!("{}/cover.jpg", host.base).as_str() + ); + let cover = router + .clone() + .oneshot( + Request::get(format!( + "/rest/getCoverArt.view?apiKey={reader_key}&v=1.16.1&c=golden&id={hash}" + )) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(cover.status(), StatusCode::OK); + assert_eq!(body_bytes(cover).await, jpeg(1)); + + // Refreshed with the same address: the feed is read, the cover is not. + state.services.refresh_all_podcasts().await; + assert_eq!(host.count("/feed.xml"), 2); + assert_eq!( + host.count("/cover.jpg"), + 1, + "an unchanged cover is not fetched again" + ); + + // The feed names another cover, behind a redirect: fetched once. + *host.cover.lock().unwrap() = "/redirect-cover".to_owned(); + state.services.refresh_all_podcasts().await; + state.services.refresh_all_podcasts().await; + assert_eq!(host.count("/cover-2.jpg"), 1); + let moved = state + .services + .podcast_channel(reader, channel_id) + .await + .unwrap(); + assert_eq!(moved.artwork_hash, Some(hash_of(&jpeg(2)))); + // The first cover is named by nothing now, and the sweep forgets it. + let swept = state.services.sweep_artwork_store().await.unwrap(); + assert_eq!(swept.rows_removed, 1); + assert_eq!( + artwork(&router, &reader_token, &hash).await, + Err(StatusCode::NOT_FOUND) + ); +} + +/// What a cover fetch refuses, it refuses before anything is kept: an image +/// over the 10 MiB ceiling, declared or not; a page of HTML, labelled as an +/// image or as what it is; a redirect to a private address. The channel is +/// subscribed all the same, without a cover. A fetch that fails is tried again +/// on the next refresh, and an image labelled as nothing in particular is +/// recognised by its bytes. +#[tokio::test] +async fn a_cover_that_is_not_a_safe_image_stores_nothing() { + let host = fake_host().await; + let (_temp, config, state) = podcast_app(|_| {}).await; + let (admin, password, _) = account(&state, "cover-guard", AccountRole::Admin).await; + let router = waveflow_server::app(&config, state.clone()); + let token = login_token(&router, "cover-guard", &password).await; + + use waveflow_server::portrait::Refusal; + let cases = [ + ("/huge.jpg", Refusal::TooLarge, "declared over the ceiling"), + ( + "/endless.jpg", + Refusal::TooLarge, + "over the ceiling, undeclared", + ), + ( + "/page-as-image.jpg", + Refusal::NotAnImage, + "HTML labelled as an image", + ), + ("/page.htm", Refusal::NotAnImage, "HTML labelled as HTML"), + ( + "/cover-to-private", + Refusal::Address, + "a redirect to a private address", + ), + ( + "/cover-to-mapped-private", + Refusal::Address, + "a redirect to a mapped private address", + ), + ]; + for (cover, _, why) in cases { + let (status, channel) = subscribe_settled( + &router, + &state, + &token, + &format!("{}/covered{cover}", host.base), + ) + .await; + assert_eq!(status, StatusCode::CREATED, "{why}: {channel}"); + assert!(channel["artwork_hash"].is_null(), "{why}: {channel}"); + assert_eq!(host.count(cover), 1, "{why}"); + } + // And each for its own reason, asked of the fetcher itself: a refusal and + // an unreachable host store the same nothing, and only the reason tells + // a checked hop from one that was attempted and happened to fail. + let policy = waveflow_server::podcast::FetchPolicy { + addresses: waveflow_server::portrait::AddressPolicy::AllowLoopback, + allow_http: false, + timeout: std::time::Duration::from_secs(10), + }; + for (cover, refusal, why) in cases { + let url = url::Url::parse(&format!("{}{cover}", host.base)).unwrap(); + let fetched = waveflow_server::portrait::fetch_followed(&url, policy).await; + assert_eq!(fetched.err(), Some(refusal), "{why}"); + } + assert_eq!(artwork_rows(&state).await, 0); + let written = std::fs::read_dir(&config.artwork_dir) + .map(|entries| entries.count()) + .unwrap_or(0); + assert_eq!(written, 0, "a refused cover leaves nothing in the store"); + + // Labelled as nothing in particular, and a JPEG by its bytes. + let (_, channel) = subscribe_settled( + &router, + &state, + &token, + &format!("{}/covered/unlabelled.jpg", host.base), + ) + .await; + assert_eq!(channel["artwork_hash"], hash_of(&jpeg(3)).as_str()); + + // A host that fails stores nothing, and the next refresh tries again. + host.cover_failing.store(true, Ordering::SeqCst); + let (_, channel) = + subscribe_settled(&router, &state, &token, &format!("{}/feed.xml", host.base)).await; + assert!(channel["artwork_hash"].is_null(), "{channel}"); + host.cover_failing.store(false, Ordering::SeqCst); + state.services.refresh_all_podcasts().await; + let retried = state + .services + .podcast_channel(admin, uuid_of(&channel)) + .await + .unwrap(); + assert_eq!(retried.artwork_hash, Some(hash_of(&jpeg(1)))); + assert_eq!(host.count("/cover.jpg"), 2); +} + +/// A station's image (RFC-025, amended): fetched when an account sets it, +/// served to that account alone, kept by a write that does not mention it — +/// a Subsonic edit above all — and fetched again only when the address +/// changes or the last fetch failed. +#[tokio::test] +async fn a_station_image_is_fetched_when_set_and_served_to_its_account_only() { + let host = fake_host().await; + let (_temp, config, state) = podcast_app(|_| {}).await; + let (_, owner_password, owner_key) = account(&state, "station-owner", AccountRole::User).await; + let (_, other_password, _) = account(&state, "station-other", AccountRole::User).await; + let router = waveflow_server::app(&config, state.clone()); + let owner = login_token(&router, "station-owner", &owner_password).await; + let other = login_token(&router, "station-other", &other_password).await; + let image_url = format!("{}/station.png", host.base); + let stream_url = "https://radio.example.org/stream"; + + let (status, created) = station( + &router, + "POST", + "/api/v2/radio-stations", + &owner, + json!({ "name": "Night Radio", "stream_url": stream_url, "image_url": image_url }), + ) + .await; + assert_eq!(status, StatusCode::CREATED, "{created}"); + let hash = hash_of(&png()); + assert_eq!(created["artwork_hash"], hash.as_str(), "{created}"); + assert_eq!(created["image_url"], image_url.as_str()); + assert_eq!(host.count("/station.png"), 1); + let station_id = uuid_of(&created); + let uri = format!("/api/v2/radio-stations/{station_id}"); + + assert_eq!(artwork(&router, &owner, &hash).await, Ok(png())); + assert_eq!( + artwork(&router, &other, &hash).await, + Err(StatusCode::NOT_FOUND), + "another account's station image is 404, like the station" + ); + + // Saved with the same address: not fetched again. + let (status, saved) = station( + &router, + "PUT", + &uri, + &owner, + json!({ "name": "Night Radio 2", "stream_url": stream_url, "image_url": image_url }), + ) + .await; + assert_eq!(status, StatusCode::OK, "{saved}"); + assert_eq!(saved["artwork_hash"], hash.as_str()); + assert_eq!( + host.count("/station.png"), + 1, + "an unchanged image is not fetched again" + ); + + // A write that does not mention the image keeps it, natively and through + // Subsonic, which has no image parameter at all. + let (_, kept) = station( + &router, + "PUT", + &uri, + &owner, + json!({ "name": "Night Radio 3", "stream_url": stream_url }), + ) + .await; + assert_eq!(kept["artwork_hash"], hash.as_str(), "{kept}"); + subsonic_json( + &router, + "updateInternetRadioStation", + &owner_key, + &format!("&id={station_id}&name=Renamed&streamUrl={stream_url}"), + ) + .await; + let renamed = json_body(send_json(&router, "GET", &uri, &owner, None).await).await; + assert_eq!(renamed["name"], "Renamed"); + assert_eq!(renamed["artwork_hash"], hash.as_str(), "{renamed}"); + assert_eq!(renamed["image_url"], image_url.as_str()); + // The protocol's station shape is unchanged. + let listed = subsonic_json(&router, "getInternetRadioStations", &owner_key, "").await; + let listed = &listed["subsonic-response"]["internetRadioStations"]["internetRadioStation"][0]; + assert!(omits(listed, "coverArt"), "{listed}"); + + // An image that cannot be fetched keeps its address and shows none; the + // next save that carries the address tries again. + host.cover_failing.store(true, Ordering::SeqCst); + let cover_url = format!("{}/cover.jpg", host.base); + let (status, failed) = station( + &router, + "PUT", + &uri, + &owner, + json!({ "name": "Renamed", "stream_url": stream_url, "image_url": cover_url }), + ) + .await; + assert_eq!(status, StatusCode::OK, "{failed}"); + assert_eq!(failed["image_url"], cover_url.as_str()); + assert!(failed["artwork_hash"].is_null(), "{failed}"); + host.cover_failing.store(false, Ordering::SeqCst); + let (_, retried) = station( + &router, + "PUT", + &uri, + &owner, + json!({ "name": "Renamed", "stream_url": stream_url, "image_url": cover_url }), + ) + .await; + assert_eq!( + retried["artwork_hash"], + hash_of(&jpeg(1)).as_str(), + "{retried}" + ); + assert_eq!(host.count("/cover.jpg"), 2); + + // Cleared by `null`, or by a blank value. + for cleared in [json!(null), json!(" ")] { + let (_, saved) = station( + &router, + "PUT", + &uri, + &owner, + json!({ "name": "Renamed", "stream_url": stream_url, "image_url": cleared }), + ) + .await; + assert!(saved["image_url"].is_null(), "{saved}"); + assert!(saved["artwork_hash"].is_null(), "{saved}"); + } + + // Validated as the homepage is, and refused before anything is fetched. + let (status, _) = station( + &router, + "POST", + "/api/v2/radio-stations", + &owner, + json!({ "name": "Bad", "stream_url": stream_url, "image_url": "file:///etc/passwd" }), + ) + .await; + assert_eq!(status, StatusCode::UNPROCESSABLE_ENTITY); + // A redirect to a private address is refused at the hop; the station is + // kept, without an image. + let (status, guarded) = station( + &router, + "POST", + "/api/v2/radio-stations", + &owner, + json!({ + "name": "Guarded", + "stream_url": stream_url, + "image_url": format!("{}/cover-to-private", host.base), + }), + ) + .await; + assert_eq!(status, StatusCode::CREATED); + assert!(guarded["artwork_hash"].is_null(), "{guarded}"); + // Another account cannot make this account's station fetch anything. + let (status, _) = station( + &router, + "PUT", + &uri, + &other, + json!({ + "name": "Taken", + "stream_url": stream_url, + "image_url": format!("{}/cover-2.jpg", host.base), + }), + ) + .await; + assert_eq!(status, StatusCode::NOT_FOUND); + assert_eq!(host.count("/cover-2.jpg"), 0); +} + +/// Nothing is fetched for a write that is refused. An account at its station +/// ceiling that names an image is answered 409 with no request to the image's +/// host and nothing in the store; an instance at its channel ceiling refuses a +/// subscription before the feed, and so the cover, is fetched. +#[tokio::test] +async fn a_write_refused_for_its_ceiling_fetches_no_image() { + let host = fake_host().await; + let (_temp, config, state) = podcast_app(|_| {}).await; + let (owner, password, _) = account(&state, "ceiling-admin", AccountRole::Admin).await; + let router = waveflow_server::app(&config, state.clone()); + let token = login_token(&router, "ceiling-admin", &password).await; + + // Filled in SQL: five hundred requests would prove nothing more. + let mut tx = state.db.pool().begin().await.unwrap(); + for index in 0..500 { + sqlx::query( + "INSERT INTO radio_station (id, user_id, name, stream_url, created_at, updated_at) \ + VALUES (?, ?, ?, 'https://radio.example.org/s', 1, 1)", + ) + .bind(Uuid::new_v4().to_string()) + .bind(owner.to_string()) + .bind(format!("Station {index}")) + .execute(&mut *tx) + .await + .unwrap(); + sqlx::query( + "INSERT INTO podcast_channel (id, feed_url, title, status, created_at, updated_at) \ + VALUES (?, ?, 'Filler', 'completed', 1, 1)", + ) + .bind(Uuid::new_v4().to_string()) + .bind(format!("https://feeds.example.org/{index}.xml")) + .execute(&mut *tx) + .await + .unwrap(); + } + tx.commit().await.unwrap(); + + let (status, body) = station( + &router, + "POST", + "/api/v2/radio-stations", + &token, + json!({ + "name": "One too many", + "stream_url": "https://radio.example.org/stream", + "image_url": format!("{}/station.png", host.base), + }), + ) + .await; + assert_eq!(status, StatusCode::CONFLICT, "{body}"); + assert_eq!( + host.count("/station.png"), + 0, + "the image's host was contacted" + ); + + let (status, body) = subscribe(&router, &token, &format!("{}/feed.xml", host.base)).await; + assert_eq!(status, StatusCode::CONFLICT, "{body}"); + assert_eq!(host.count("/feed.xml"), 0, "the feed's host was contacted"); + assert_eq!( + host.count("/cover.jpg"), + 0, + "the cover's host was contacted" + ); + + assert_eq!(artwork_rows(&state).await, 0); + let written = std::fs::read_dir(&config.artwork_dir) + .map(|entries| entries.count()) + .unwrap_or(0); + assert_eq!(written, 0, "a refused write leaves nothing in the store"); +} + +/// The sweep counts a channel's cover and a station's image as referenced, so +/// neither is collected while it is named, however old its file; deleting the +/// channel and the station releases both. +#[tokio::test] +async fn covers_named_by_a_channel_or_a_station_survive_the_sweep_until_deleted() { + let host = fake_host().await; + let (_temp, config, state) = podcast_app(|_| {}).await; + let (_, password, _) = account(&state, "sweep-admin", AccountRole::Admin).await; + let router = waveflow_server::app(&config, state.clone()); + let token = login_token(&router, "sweep-admin", &password).await; + + let (_, channel) = + subscribe_settled(&router, &state, &token, &format!("{}/feed.xml", host.base)).await; + let (_, created) = station( + &router, + "POST", + "/api/v2/radio-stations", + &token, + json!({ + "name": "Kept", + "stream_url": "https://radio.example.org/stream", + "image_url": format!("{}/station.png", host.base), + }), + ) + .await; + let files = [ + config + .artwork_dir + .join(format!("{}.jpg", hash_of(&jpeg(1)))), + config.artwork_dir.join(format!("{}.png", hash_of(&png()))), + ]; + // Older than the sweep's grace, so only a reference can keep them. + for file in &files { + let old = std::time::SystemTime::now() - std::time::Duration::from_secs(2 * 60 * 60); + std::fs::File::options() + .write(true) + .open(file) + .unwrap() + .set_modified(old) + .unwrap(); + } + + let swept = state.services.sweep_artwork_store().await.unwrap(); + assert_eq!(swept.rows_removed, 0, "{swept:?}"); + assert_eq!(swept.covers_removed, 0, "{swept:?}"); + assert!(files.iter().all(|file| file.exists())); + assert_eq!(artwork_rows(&state).await, 2); + + // Named by a fixed word, not by the address: a failure message has no + // reason to print an id. + for (what, uri) in [ + ("channel", format!("/api/v2/podcasts/{}", uuid_of(&channel))), + ( + "station", + format!("/api/v2/radio-stations/{}", uuid_of(&created)), + ), + ] { + let deleted = send_json(&router, "DELETE", &uri, &token, None).await; + assert_eq!(deleted.status(), StatusCode::NO_CONTENT, "{what}"); + } + let swept = state.services.sweep_artwork_store().await.unwrap(); + assert_eq!(swept.rows_removed, 2, "{swept:?}"); + assert_eq!(swept.covers_removed, 2, "{swept:?}"); + assert!(files.iter().all(|file| !file.exists())); +} + +/// A subscription answers without waiting for its cover (RFC-025 A2). The +/// cover follows in the background: a cover host as slow as the feed would +/// otherwise hold the answer for the sum of both, and on Subsonic's +/// `createPodcastChannel`, which no `TimeoutLayer` bounds, for as long as the +/// host liked. The fixture's cover deadline is ten seconds, so an answer +/// within five is one that did not wait for it. +#[tokio::test] +async fn a_slow_cover_host_does_not_hold_a_subsonic_subscription() { + let host = fake_host().await; + let (_temp, config, state) = podcast_app(|_| {}).await; + let (admin, _, key) = account(&state, "slow-cover-admin", AccountRole::Admin).await; + let router = waveflow_server::app(&config, state.clone()); + + let feed = format!("{}/covered/slow-cover.jpg", host.base); + let answered = tokio::time::timeout( + std::time::Duration::from_secs(5), + subsonic_json( + &router, + "createPodcastChannel", + &key, + &format!("&url={feed}"), + ), + ) + .await + .expect("the subscription waited for its cover"); + assert_eq!(answered["subsonic-response"]["status"], "ok", "{answered}"); + + // Created, with its cover pending. + let channels = state + .services + .podcast_channels(admin, None, false) + .await + .unwrap(); + assert_eq!(channels.len(), 1); + assert!(channels[0].artwork_hash.is_none()); + // And the cover is being asked for, outside the request. + for _ in 0..500 { + if host.count("/slow-cover.jpg") > 0 { + break; + } + tokio::time::sleep(std::time::Duration::from_millis(10)).await; + } + assert_eq!(host.count("/slow-cover.jpg"), 1); +} + +/// A subscription's cover arrives after a refresh has moved on: the late cover +/// is dropped, and the channel keeps what the refresh left — the new cover, or +/// none — never the stale one. Three ways: the feed names another cover, which +/// the refresh records; it names one the refresh cannot fetch; it names the +/// same address, whose image the refresh records anew. The dropped row is +/// left to the sweep. +#[tokio::test] +async fn a_first_cover_that_lands_after_a_refresh_moved_the_feed_is_dropped() { + let host = fake_host().await; + let (_temp, config, state) = podcast_app(|_| {}).await; + let (admin, password, _) = account(&state, "late-cover-admin", AccountRole::Admin).await; + let router = waveflow_server::app(&config, state.clone()); + let token = login_token(&router, "late-cover-admin", &password).await; + + // The subscription's cover is held at the host. + *host.cover.lock().unwrap() = "/gated-cover.jpg".to_owned(); + let (status, channel) = subscribe(&router, &token, &format!("{}/feed.xml", host.base)).await; + assert_eq!(status, StatusCode::CREATED, "{channel}"); + for _ in 0..500 { + if host.count("/gated-cover.jpg") > 0 { + break; + } + tokio::time::sleep(std::time::Duration::from_millis(10)).await; + } + assert_eq!(host.count("/gated-cover.jpg"), 1); + + // Meanwhile the feed names another cover, and a refresh records it. + *host.cover.lock().unwrap() = "/cover-2.jpg".to_owned(); + state.services.refresh_all_podcasts().await; + let channel_id = uuid_of(&channel); + let refreshed = state + .services + .podcast_channel(admin, channel_id) + .await + .unwrap(); + assert_eq!(refreshed.artwork_hash, Some(hash_of(&jpeg(2)))); + + // Then the first cover lands. + host.gate_open.store(true, Ordering::SeqCst); + state.services.podcast_covers_settled().await; + let settled = state + .services + .podcast_channel(admin, channel_id) + .await + .unwrap(); + assert_eq!( + settled.artwork_hash, + Some(hash_of(&jpeg(2))), + "a stale cover replaced the newer one" + ); + // Whatever the late fetch recorded names nothing, and the sweep forgets it. + let swept = state.services.sweep_artwork_store().await.unwrap(); + assert_eq!(swept.rows_removed, 1, "{swept:?}"); + + // Again, with a refresh that moves the feed to a cover it cannot fetch: + // no newer cover is recorded, so only the address tells the late one is + // stale. The channel ends with none. + host.gate_open.store(false, Ordering::SeqCst); + *host.cover.lock().unwrap() = "/gated-cover.jpg".to_owned(); + let (status, second) = + subscribe(&router, &token, &format!("{}/feed.xml?second", host.base)).await; + assert_eq!(status, StatusCode::CREATED, "{second}"); + for _ in 0..500 { + if host.count("/gated-cover.jpg") > 1 { + break; + } + tokio::time::sleep(std::time::Duration::from_millis(10)).await; + } + assert_eq!(host.count("/gated-cover.jpg"), 2); + *host.cover.lock().unwrap() = "/no-such-cover.jpg".to_owned(); + state.services.refresh_all_podcasts().await; + host.gate_open.store(true, Ordering::SeqCst); + state.services.podcast_covers_settled().await; + let second = state + .services + .podcast_channel(admin, uuid_of(&second)) + .await + .unwrap(); + assert_eq!( + second.artwork_hash, None, + "a cover the feed no longer names was attached" + ); + + // And with the same address both times: the refresh fetches it again + // while the first fetch is held, and records what it is now. Only the + // record tells the late cover is the older one. + host.gate_open.store(false, Ordering::SeqCst); + *host.cover.lock().unwrap() = "/gated-once.jpg".to_owned(); + let (status, third) = + subscribe(&router, &token, &format!("{}/feed.xml?third", host.base)).await; + assert_eq!(status, StatusCode::CREATED, "{third}"); + for _ in 0..500 { + if host.count("/gated-once.jpg") > 0 { + break; + } + tokio::time::sleep(std::time::Duration::from_millis(10)).await; + } + state.services.refresh_all_podcasts().await; + // Every channel names it now, so each refreshes it; only the first + // request, the subscription's, is held. + assert!(host.count("/gated-once.jpg") > 1); + host.gate_open.store(true, Ordering::SeqCst); + state.services.podcast_covers_settled().await; + let third = state + .services + .podcast_channel(admin, uuid_of(&third)) + .await + .unwrap(); + assert_eq!( + third.artwork_hash, + Some(hash_of(&jpeg(8))), + "an older fetch of the same address replaced the newer one" + ); +} diff --git a/webapp/e2e/smart-playlists-and-podcasts.spec.ts b/webapp/e2e/smart-playlists-and-podcasts.spec.ts index e1adeaf7..7de95369 100644 --- a/webapp/e2e/smart-playlists-and-podcasts.spec.ts +++ b/webapp/e2e/smart-playlists-and-podcasts.spec.ts @@ -72,6 +72,7 @@ type Episode = { id: string; channel_id: string; channel_title: string; + artwork_hash: string | null; title: string; description: string | null; published_at: number | null; @@ -97,6 +98,7 @@ const episode = (id: string, title: string, extra: Partial = {}): Episo id, channel_id: "channel-1", channel_title: "Night Shift", + artwork_hash: null, title, description: null, published_at: 1_704_448_800_000, @@ -266,6 +268,7 @@ async function mockApi(page: Page) { description: "Talk after dark", site_url: null, image_url: null, + artwork_hash: null, status: "completed", error: null, created_at: 1, @@ -659,6 +662,7 @@ const channelOf = (id: string, title: string, episodes: Episode[]) => ({ description: null, site_url: null, image_url: null, + artwork_hash: null, status: "completed", error: null, created_at: 1, @@ -748,6 +752,61 @@ test("plays an episode in the bar, across pages, and resumes it from its bookmar expect((await episodeElement(page, "ticket-episode-1"))?.currentTime).toBeGreaterThanOrEqual(12); }); +/** A one-pixel PNG, for the covers the artwork route is asked for. */ +const PIXEL = Buffer.from( + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg==", + "base64", +); + +/** + * A channel's cover is the hash of the copy the server fetched, and the page + * reads it through the artwork route like any cover: beside the channel, + * above its episodes, and in the bar and the system's controls while one of + * them plays. A channel without one keeps the podcast symbol. + */ +test("shows a channel's cover in its list, above its episodes and in the bar", async ({ + page, +}) => { + const cover = "c".repeat(64); + let served = 0; + await page.route(`**/api/v2/artwork/${cover}`, (route) => { + served += 1; + return route.fulfill({ contentType: "image/png", body: PIXEL }); + }); + await recordPlayedElements(page); + channels = [ + { + ...channelOf("channel-1", "Night Shift", [ + { ...episode("episode-1", "First night", ready), artwork_hash: cover }, + ]), + artwork_hash: cover, + }, + channelOf("channel-2", "Day Shift", []), + ]; + await page.goto("/podcasts"); + const night = page.locator(".podcast-channels li").filter({ hasText: "Night Shift" }); + const day = page.locator(".podcast-channels li").filter({ hasText: "Day Shift" }); + await expect(night.locator("img.resource-cover")).toBeVisible(); + await expect(day.locator(".resource-cover.cover-fallback svg")).toBeVisible(); + + await night.getByRole("button", { name: "Episodes" }).click(); + await expect(page.locator(".podcast-episodes img.podcast-cover")).toBeVisible(); + const first = page.locator(".podcast-episodes li").filter({ hasText: "First night" }); + await first.getByRole("button", { name: "Play First night" }).click(); + + const bar = page.locator(".player"); + await expect(bar.locator(".player-track strong")).toHaveText("First night"); + await expect(bar.locator("img.player-cover")).toBeVisible(); + await expect + .poll(() => + page.evaluate(() => navigator.mediaSession?.metadata?.artwork?.[0]?.src ?? ""), + ) + .toMatch(/^blob:/); + // One request for the hash, however many places show it. + expect(served).toBe(1); + await expectNoWcagViolations(page); +}); + test("keeps an episode playing when another channel is opened", async ({ page, }) => { @@ -876,6 +935,7 @@ test("offers a listener no podcast control it may not use", async ({ page }) => description: null, site_url: null, image_url: null, + artwork_hash: null, status: "error", error: "http_500", created_at: 1, diff --git a/webapp/e2e/studio-nocturne.spec.ts b/webapp/e2e/studio-nocturne.spec.ts index 64717012..85d35eaf 100644 --- a/webapp/e2e/studio-nocturne.spec.ts +++ b/webapp/e2e/studio-nocturne.spec.ts @@ -342,9 +342,20 @@ let stations: Array<{ name: string; stream_url: string; homepage_url: string | null; + image_url: string | null; + artwork_hash: string | null; created_at: number; updated_at: number; }> = []; +/** Every image address a station write carried, as the server would fetch it. */ +let stationImages: Array = []; + +/** + * The hash the server would store a station's image under, made up from its + * address: what the mock answers instead of fetching anything. + */ +const imageHash = (address: string) => + `station-art-${address.replace(/[^a-z0-9]/gi, "")}`; const playlists = [ { @@ -836,11 +847,15 @@ async function mockAuthenticatedApi(page: Page) { radioWrites.push("POST"); await radioGate; const draft = route.request().postDataJSON(); + stationImages.push(draft.image_url); + const image = draft.image_url ?? null; const created = { id: `station-${stations.length + 1}`, name: String(draft.name).trim(), stream_url: draft.stream_url, homepage_url: draft.homepage_url ?? null, + image_url: image, + artwork_hash: image ? imageHash(image) : null, created_at: 1_700_000_000_000, updated_at: 1_700_000_000_000, }; @@ -861,12 +876,21 @@ async function mockAuthenticatedApi(page: Page) { return; } // A replacement: every field arrives, so an absent homepage clears it. + // The image is the exception the server makes: absent, it is kept. const draft = route.request().postDataJSON(); + stationImages.push(draft.image_url); + const previous = stations.find((station) => station.id === id); + const image = + "image_url" in draft + ? (draft.image_url ?? null) + : (previous?.image_url ?? null); const replaced = { id, name: String(draft.name).trim(), stream_url: draft.stream_url, homepage_url: draft.homepage_url ?? null, + image_url: image, + artwork_hash: image ? imageHash(image) : null, created_at: 1_700_000_000_000, updated_at: 1_700_000_000_001, }; @@ -1368,6 +1392,7 @@ test.beforeEach(async ({ page }) => { exportAsks = []; stations = []; radioWrites = []; + stationImages = []; radioGate = Promise.resolve(); artistPlayed = [song(2, "Cocoon", 0, false), song(1, "Hidden Place", 5, true)]; topSongAsks = []; @@ -6125,6 +6150,8 @@ async function serveStationAndQueue(page: Page) { name: "Radio Nova", stream_url: NOVA, homepage_url: null, + image_url: null, + artwork_hash: null, created_at: 1_700_000_000_000, updated_at: 1_700_000_000_000, }, @@ -6294,6 +6321,117 @@ test("pauses the queue under a station, keeps it, and gives it back to a track", * Taking a station out of the bar leaves the queue's track there, paused * where it was, and the light theme keeps the live badge readable. */ +/** A one-pixel PNG, for every cover the artwork route is asked for. */ +const PIXEL = Buffer.from( + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg==", + "base64", +); + +/** + * A station's image is an address the account types, sent with the station; + * the server fetches it once and answers a hash, and what the page shows is + * the artwork route's answer for that hash — never the image's own host, + * which RFC-019's `img-src` would refuse and which would see the listener. + */ +test("takes an image address for a station, and shows the image the server kept", async ({ + page, +}) => { + const asked: string[] = []; + page.on("request", (request) => { + if (new URL(request.url()).hostname === "logos.example.org") { + asked.push(request.url()); + } + }); + await page.route("**/api/v2/artwork/station-art-*", (route) => + route.fulfill({ contentType: "image/png", body: PIXEL }), + ); + await page.goto("/radio"); + await page.getByLabel("Station name").fill("Radio Nova"); + await page.getByLabel("Stream address").fill(NOVA); + // Refused here first, as the server would, and the button stays shut. + await page.getByLabel("Station image (optional)").fill("ftp://logos.example.org/n.png"); + await expect( + page.getByText("An address must start with http:// or https://."), + ).toBeVisible(); + await expect(page.getByRole("button", { name: "Add station" })).toBeDisabled(); + await page + .getByLabel("Station image (optional)") + .fill("https://logos.example.org/nova.png"); + await expect( + page.getByText("The server downloads it once, when you save", { exact: false }), + ).toBeVisible(); + await page.getByRole("button", { name: "Add station" }).click(); + + const listed = page.locator(".resource-list li").filter({ hasText: "Radio Nova" }); + await expect(listed.locator("img.resource-cover")).toBeVisible(); + expect(stationImages).toEqual(["https://logos.example.org/nova.png"]); + + // Edited, the form shows the address back and sends it again; cleared, it + // sends `null` and the station keeps the symbol. + await listed.getByRole("button", { name: "Edit" }).click(); + await expect(page.getByLabel("Station image (optional)")).toHaveValue( + "https://logos.example.org/nova.png", + ); + await page.getByLabel("Station image (optional)").fill(""); + await page.getByRole("button", { name: "Save station" }).click(); + await expect(listed.locator(".resource-cover.cover-fallback svg")).toBeVisible(); + expect(stationImages).toEqual(["https://logos.example.org/nova.png", null]); + await expectNoWcagViolations(page); + expect(asked, "the browser never asks the image's own host").toEqual([]); +}); + +for (const theme of ["dark", "light"] as const) { + test(`shows a station's image in the bar, and the symbol without one, in the ${theme}`, async ({ + page, + }) => { + if (theme === "light") await chooseLightTheme(page); + await serveStationAndQueue(page); + stations = [ + { ...stations[0], image_url: "https://logos.example.org/nova.png", artwork_hash: "station-art-nova" }, + { + ...stations[0], + id: "station-2", + name: "Radio Plain", + }, + ]; + let served = 0; + await page.route("**/api/v2/artwork/station-art-nova", (route) => { + served += 1; + return route.fulfill({ contentType: "image/png", body: PIXEL }); + }); + await page.goto("/radio"); + const nova = page.locator(".resource-list li").filter({ hasText: "Radio Nova" }); + await expect(nova.locator("img.resource-cover")).toBeVisible(); + await nova.getByRole("button", { name: "Listen" }).click(); + + const bar = page.locator(".player"); + await expect(bar.locator(".player-track strong")).toHaveText("Radio Nova"); + await expect(bar.locator("img.player-cover")).toBeVisible(); + // The system's controls are given the same cover, by its object URL. + await expect + .poll(() => + page.evaluate( + () => navigator.mediaSession?.metadata?.artwork?.[0]?.src ?? "", + ), + ) + .toMatch(/^blob:/); + expect(served).toBe(1); + await expectNoWcagViolations(page); + + // A station without an image keeps the radio symbol. + const plain = page.locator(".resource-list li").filter({ hasText: "Radio Plain" }); + await plain.getByRole("button", { name: "Listen" }).click(); + await expect(bar.locator(".player-track strong")).toHaveText("Radio Plain"); + await expect(bar.locator(".player-cover.cover-fallback svg")).toBeVisible(); + await expect(bar.locator("img.player-cover")).toHaveCount(0); + await expect + .poll(() => + page.evaluate(() => navigator.mediaSession?.metadata?.artwork?.length ?? 0), + ) + .toBe(0); + }); +} + test("keeps a live station readable in the light, and closes back to the queue", async ({ page, }) => { diff --git a/webapp/src/api/podcasts.ts b/webapp/src/api/podcasts.ts index 64e16f11..1fb8ded8 100644 --- a/webapp/src/api/podcasts.ts +++ b/webapp/src/api/podcasts.ts @@ -17,8 +17,14 @@ export type PodcastChannel = { title: string; description: string | null; site_url: string | null; - /** The feed's own cover address. The server does not fetch it. */ + /** + * The cover address the feed names, to an account that may manage + * podcasts; `null` to everyone else, like `url`. Never loaded by the + * browser: the server fetches the cover and hands out `artwork_hash`. + */ image_url: string | null; + /** The cover, in the artwork store; `null` until a fetch succeeds. */ + artwork_hash: string | null; status: "completed" | "error"; /** Why the last fetch failed, as a word: `unreached`, `http_404`, `invalid_feed`… */ error: string | null; @@ -40,6 +46,8 @@ export type PodcastEpisode = { id: string; channel_id: string; channel_title: string; + /** The channel's cover: an episode shows its channel's. */ + artwork_hash: string | null; title: string; description: string | null; published_at: number | null; diff --git a/webapp/src/api/radio.ts b/webapp/src/api/radio.ts index b52ebc0d..1af31d7a 100644 --- a/webapp/src/api/radio.ts +++ b/webapp/src/api/radio.ts @@ -7,12 +7,21 @@ import { call } from "./client"; * stores the address and hands it over without ever fetching it, so the * browser is what connects — which is also why a station that will not play * fails in the player and not in a server response. + * + * Its image is the exception: the server fetches it once, when it is set, into + * the artwork store, and hands out `artwork_hash`. The browser never loads the + * image from the station's host (RFC-019's `img-src`), so `image_url` is only + * what the form shows back. */ export type RadioStation = { id: string; name: string; stream_url: string; homepage_url: string | null; + /** The image address the account gave, kept even when it could not be fetched. */ + image_url: string | null; + /** The fetched image; `null` when there is none, or its fetch failed. */ + artwork_hash: string | null; created_at: number; updated_at: number; }; @@ -22,6 +31,12 @@ export type RadioStationDraft = { name: string; stream_url: string; homepage_url?: string | null; + /** + * Absent, the stored image stays; `null`, it is cleared. The one field a + * replacement does not clear by omission, so a client that never heard of + * it — a Subsonic one above all — cannot take a station's image away. + */ + image_url?: string | null; }; export const listRadioStations = () => diff --git a/webapp/src/components/artwork.tsx b/webapp/src/components/artwork.tsx index 87cc079b..4dd3ba52 100644 --- a/webapp/src/components/artwork.tsx +++ b/webapp/src/components/artwork.tsx @@ -1,4 +1,4 @@ -import { useCallback, useEffect, useState } from "react"; +import { type ReactNode, useCallback, useEffect, useState } from "react"; import { artworkUrl, cachedArtworkUrl } from "../api"; @@ -6,6 +6,11 @@ type ArtworkProps = { artworkId: string | null; title: string; className?: string; + /** + * What the placeholder shows instead of the title's first letter: a radio + * station or a podcast keeps its symbol until, or unless, its image comes. + */ + placeholder?: ReactNode; }; /** @@ -42,7 +47,12 @@ const observable = typeof IntersectionObserver !== "undefined"; * make `` work and let the HTTP cache serve between sessions, which * is the deeper fix and is not this one. */ -export function Artwork({ artworkId, title, className = "" }: ArtworkProps) { +export function Artwork({ + artworkId, + title, + className = "", + placeholder, +}: ArtworkProps) { // Read straight out of the cache, during the render rather than after it. // A cover whose bytes are already held has nothing to wait for, and starting // at `null` put the grey placeholder on screen for a frame every time this @@ -106,7 +116,9 @@ export function Artwork({ artworkId, title, className = "" }: ArtworkProps) { className={`${classes} cover-fallback`} aria-hidden="true" > - {title.slice(0, 1).toLocaleUpperCase()} + {/* A symbol is centred as the grid places it, where the letter is set + in the corner by `.cover-fallback span`. */} + {placeholder ?? {title.slice(0, 1).toLocaleUpperCase()}} ); } diff --git a/webapp/src/lib/aside.test.ts b/webapp/src/lib/aside.test.ts index 767f6737..c82bf5f8 100644 --- a/webapp/src/lib/aside.test.ts +++ b/webapp/src/lib/aside.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest"; import type { PodcastEpisode, RadioStation } from "../api"; import { + asideArtwork, asideMetadata, asideTransport, bookmarkWrite, @@ -28,6 +29,8 @@ const station: RadioStation = { name: "Radio Nova", stream_url: "https://nova.example.org/live", homepage_url: null, + image_url: null, + artwork_hash: null, created_at: 1, updated_at: 1, }; @@ -36,6 +39,7 @@ const episode: PodcastEpisode = { id: "episode-1", channel_id: "channel-1", channel_title: "Night Shift", + artwork_hash: null, title: "First night", description: null, published_at: null, @@ -49,6 +53,35 @@ const episode: PodcastEpisode = { created_at: 1, }; +describe("the cover an aside shows", () => { + it("is the station's image, or the episode's channel's cover", () => { + expect(asideArtwork({ kind: "live", station })).toBeNull(); + expect( + asideArtwork({ + kind: "live", + station: { ...station, artwork_hash: "s" }, + }), + ).toBe("s"); + expect( + asideArtwork({ + kind: "episode", + episode: { ...episode, artwork_hash: "c" }, + }), + ).toBe("c"); + }); + + it("is never the address an image came from", () => { + // A station whose image could not be fetched keeps its address and shows + // the symbol: the browser does not load another host's image. + expect( + asideArtwork({ + kind: "live", + station: { ...station, image_url: "https://logos.example.org/n.png" }, + }), + ).toBeNull(); + }); +}); + describe("the transport an aside offers", () => { it("has no position and nowhere to seek for a station", () => { expect(asideTransport("live")).toEqual({ progress: false, seek: false }); diff --git a/webapp/src/lib/aside.ts b/webapp/src/lib/aside.ts index eb91c8bc..9d6bc153 100644 --- a/webapp/src/lib/aside.ts +++ b/webapp/src/lib/aside.ts @@ -52,6 +52,18 @@ export function asideMetadata( }; } +/** + * The cover an aside shows: the station's image, or the episode's channel's + * cover — both fetched by the server into the artwork store, so this is a + * hash for `/api/v2/artwork`, never another host's address. `null` keeps the + * generic symbol. + */ +export function asideArtwork(aside: Aside): string | null { + return aside.kind === "live" + ? aside.station.artwork_hash + : aside.episode.artwork_hash; +} + /** * The position an episode resumes from: its bookmark, unless it was left * within the last few seconds, which is an episode heard to the end. diff --git a/webapp/src/pages/podcasts-page.tsx b/webapp/src/pages/podcasts-page.tsx index 82c76a24..5ee15597 100644 --- a/webapp/src/pages/podcasts-page.tsx +++ b/webapp/src/pages/podcasts-page.tsx @@ -13,6 +13,7 @@ import { unsubscribePodcast, } from "../api"; import { podcastChannelQuery, podcastsQuery } from "../api/queries"; +import { Artwork } from "../components/artwork"; import { Icon } from "../components/icons"; import { EmptyState, @@ -138,14 +139,24 @@ export function PodcastsPage() {