From 96c3dc2e918dee1929bef722456a843a48a81c40 Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Mon, 26 Oct 2026 02:16:20 +0100 Subject: [PATCH 01/12] feat(artwork): fetch podcast covers and radio station images into the artwork store Signed-off-by: InstaZDLL --- ...261028000000_podcast_and_radio_artwork.sql | 40 ++ src/api/radio.rs | 21 + src/config.rs | 39 ++ src/portrait.rs | 97 ++- src/services/artwork.rs | 21 +- src/services/catalog.rs | 7 +- src/services/mod.rs | 18 +- src/services/podcasts.rs | 259 ++++++++- src/services/radio.rs | 246 ++++++-- src/services/remote_artwork.rs | 115 ++++ src/subsonic/podcasts.rs | 11 +- src/subsonic/radio.rs | 4 + tests/podcasts.rs | 550 +++++++++++++++++- 13 files changed, 1335 insertions(+), 93 deletions(-) create mode 100644 migrations-v2/20261028000000_podcast_and_radio_artwork.sql create mode 100644 src/services/remote_artwork.rs 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..cc81746a 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,9 @@ 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, /// Smart playlists (RFC-025): what each reader was last shown, the /// evaluation counter and the evaluation budget. smart: smart_playlists::SmartRuntime, @@ -1563,6 +1574,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 +1631,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 +1706,7 @@ impl DomainServices { crate::lrclib::LyricsLookup::disabled(), )), podcasts: podcasts::PodcastRuntime::new(config.podcasts.clone()), + remote_artwork: config.remote_artwork, smart: smart_playlists::SmartRuntime::new(config.db_max_connections), } } diff --git a/src/services/podcasts.rs b/src/services/podcasts.rs index af3c208c..34665cb0 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, @@ -149,7 +165,8 @@ impl PodcastRuntime { 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 +176,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 " @@ -328,6 +345,11 @@ impl DomainServices { return Err(ServiceError::Conflict.into()); } let feed = self.fetch_feed(&url).await.map_err(PodcastFailure::Feed)?; + // Before the gate, like the feed: a cover that cannot be fetched + // subscribes the channel without one, and the next refresh tries again. + let cover = self + .channel_cover(feed.image_url.as_deref(), &KnownCover::default()) + .await; let id = Uuid::new_v4(); let now = now_ms(); { @@ -364,6 +386,7 @@ impl DomainServices { return Err(ServiceError::Conflict.into()); } upsert_episodes(&mut tx, id, &feed.episodes, now).await?; + write_cover(&mut tx, &id.to_string(), &cover, now).await?; tx.commit().await?; } Ok(self.podcast_channel(user_id, id).await?) @@ -422,20 +445,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 +479,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 +518,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 +553,34 @@ 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) => match self + .fetch_remote_artwork(url, self.podcasts.settings.allow_http, "podcast cover") + .await + { + Some(artwork) => CoverWrite::Stored { + artwork, + url: url.to_owned(), + }, + 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 +1003,107 @@ 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, + url: String, + }, +} + +/// 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, in the transaction that writes the +/// rest of its refresh. +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=?", + ) + .bind(channel_id) + .execute(&mut **tx) + .await?; + } + CoverWrite::Stored { artwork, url } => { + super::remote_artwork::record_artwork(tx, artwork, "podcast", now).await?; + sqlx::query( + "UPDATE podcast_channel SET artwork_hash=?, artwork_url=?, artwork_fetched_at=? \ + WHERE id=?", + ) + .bind(&artwork.hash) + .bind(url) + .bind(now) + .bind(channel_id) + .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 +1221,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 +1240,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>), +} + +/// What a write stores of the image, decided before the writer gate. +enum ImageWrite { + Keep, + Store { + image_url: Option, + artwork_hash: Option, + fetched: Option, + }, +} + +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 +82,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 +100,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,16 +114,26 @@ 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 here, before the writer gate, and a station + /// whose image cannot be fetched is created all the same, 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 fetched = match &image_url { + Some(url) => self.fetch_station_image(url).await, + None => None, + }; let id = Uuid::new_v4(); let now = now_ms(); let _writer = self.db.writer_guard().await; @@ -106,16 +153,23 @@ impl DomainServices { tx.rollback().await?; return Err(ServiceError::Conflict); } + if let Some(fetched) = &fetched { + record_artwork(&mut tx, fetched, "station", now).await?; + } + let artwork_hash = fetched.map(|fetched| fetched.hash); sqlx::query( "INSERT INTO radio_station \ - (id, user_id, name, stream_url, homepage_url, created_at, updated_at) \ - VALUES (?, ?, ?, ?, ?, ?, ?)", + (id, user_id, name, stream_url, homepage_url, image_url, artwork_hash, \ + 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(image_url.as_deref()) + .bind(artwork_hash.as_deref()) .bind(now) .bind(now) .execute(&mut *tx) @@ -126,18 +180,28 @@ impl DomainServices { name, stream_url, homepage_url, + image_url, + artwork_hash, created_at: now, updated_at: now, }) } - /// 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, and a failure stores the address with no + /// image, so saving the station again is how a fetch is retried. pub async fn update_radio_station( &self, user_id: Uuid, @@ -145,44 +209,73 @@ 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 => ImageWrite::Keep, + StationImage::Set(value) => { + let image_url = validated_homepage(value)?; + self.station_image_write(user_id, station_id, image_url) + .await? + } + }; 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); - } - // `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=?") + let mut tx = self.db.begin_write(&_writer).await?; + let updated = match &image { + ImageWrite::Keep => 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? + .rows_affected(), + ImageWrite::Store { + image_url, + artwork_hash, + fetched, + } => { + if let Some(fetched) = fetched { + record_artwork(&mut tx, fetched, "station", now).await?; + } + 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(artwork_hash.as_deref()) + .bind(now) .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, - }) + .execute(&mut *tx) + .await? + .rows_affected() + } + }; + if updated == 0 { + tx.rollback().await?; + return Err(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_one(&mut *tx) + .await?; + tx.commit().await?; + radio_station_from_row(row) } /// Removes a station. @@ -191,6 +284,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 +304,62 @@ impl DomainServices { } Ok(()) } + + /// What an edit that sets the image stores, decided before the gate: the + /// image already held when the address has not changed and its file is in + /// the store, a fresh fetch otherwise, and no image when that fails. + /// + /// Reads the station without the gate, so another edit can land in + /// between; the worst that does is one fetch more or less, and the + /// `UPDATE` still names the account, so a station that is not this + /// account's is 404 whatever was read here. + async fn station_image_write( + &self, + user_id: Uuid, + station_id: Uuid, + image_url: Option, + ) -> Result { + let Some(url) = image_url else { + return Ok(ImageWrite::Store { + image_url: None, + artwork_hash: None, + fetched: None, + }); + }; + 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(self.db.pool()) + .await?; + let Some((held_url, held_hash)) = held else { + // Nothing is fetched for a station this account cannot see. + return Err(ServiceError::NotFound); + }; + if let Some(hash) = held_hash.filter(|_| held_url.as_deref() == Some(url.as_str())) { + if self.artwork_file_present(&hash).await { + return Ok(ImageWrite::Store { + image_url: Some(url), + artwork_hash: Some(hash), + fetched: None, + }); + } + } + let fetched = self.fetch_station_image(&url).await; + Ok(ImageWrite::Store { + image_url: Some(url), + artwork_hash: fetched.as_ref().map(|fetched| fetched.hash.clone()), + fetched, + }) + } + + /// A station image, fetched into the store. 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 fetch_station_image(&self, url: &str) -> Option { + self.fetch_remote_artwork(url, true, "station image").await + } } fn radio_station_from_row(row: sqlx::sqlite::SqliteRow) -> Result { @@ -216,6 +368,8 @@ fn radio_station_from_row(row: sqlx::sqlite::SqliteRow) -> Result 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(()) +} 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..1e90d0e3 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,10 @@ 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>, } impl Host { @@ -51,13 +62,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 +95,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 +126,72 @@ 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)), + // 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(), + ), + "/cover-to-private" => redirect("http://10.255.255.1/cover.jpg"), + "/cover-to-mapped-private" => redirect("http://[::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 +231,8 @@ 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())), }; let router = Router::new().fallback(serve).with_state(host.clone()); tokio::spawn(async move { axum::serve(listener, router).await.unwrap() }); @@ -875,3 +960,446 @@ 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(&router, &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; + + for (cover, why) in [ + ("/huge.jpg", "declared over the ceiling"), + ("/endless.jpg", "over the ceiling, undeclared"), + ("/page-as-image.jpg", "HTML labelled as an image"), + ("/page.htm", "HTML labelled as HTML"), + ("/cover-to-private", "a redirect to a private address"), + ( + "/cover-to-mapped-private", + "a redirect to a mapped private address", + ), + ] { + let (status, channel) = + subscribe(&router, &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}"); + } + 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( + &router, + &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(&router, &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); +} + +/// 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(&router, &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); + + for uri in [ + format!("/api/v2/podcasts/{}", uuid_of(&channel)), + 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, "{uri}"); + } + 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())); +} From b4713b6429f2583dd8d3c557a09f8546eac6a4e8 Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Mon, 26 Oct 2026 02:16:20 +0100 Subject: [PATCH 02/12] feat(webapp): show podcast covers and station images in the bar and the pages Signed-off-by: InstaZDLL --- .../e2e/smart-playlists-and-podcasts.spec.ts | 60 ++++++++ webapp/e2e/studio-nocturne.spec.ts | 138 ++++++++++++++++++ webapp/src/api/podcasts.ts | 10 +- webapp/src/api/radio.ts | 15 ++ webapp/src/components/artwork.tsx | 18 ++- webapp/src/lib/aside.test.ts | 33 +++++ webapp/src/lib/aside.ts | 12 ++ webapp/src/pages/podcasts-page.tsx | 45 ++++-- webapp/src/pages/radio-page.tsx | 77 ++++++++-- webapp/src/player/player.tsx | 43 ++++-- webapp/src/state/i18n.tsx | 10 ++ webapp/src/styles/fluid.css | 6 + webapp/src/styles/styles.css | 26 ++++ 13 files changed, 454 insertions(+), 39 deletions(-) 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 4e141d47..5f28c725 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 = []; @@ -5976,6 +6001,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, }, @@ -6145,6 +6172,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() {