From 2c970be1f75f651e1016e0fa3d7a5a8a6ac4e4fa Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Wed, 21 Oct 2026 07:58:38 +0200 Subject: [PATCH 1/5] feat: upload .lrc, .txt and .ttml lyrics to a track MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Step 4 of RFC-012 (decision 6). Uploaded sheets live in their own table, track_lyrics_upload, beside the scanner's track_lyrics, which every scan rebuilds; nothing is written next to the audio. - PUT /api/v2/tracks/{id}/lyrics/{format} stores lrc, txt or ttml, at most a mebibyte (the route's own body ceiling; the router's default is 16 KiB), decoded through waveflow_core::text_encoding, replacing an earlier upload in the same format. A sheet with nothing usable in it is refused with 422. Owners and managers only; a listener gets the 404 an unseen track would. - DELETE removes an upload; the track falls back to what was scanned. - GET /api/v2/tracks/{id}/lyrics now lists every sheet, uploads included, in one order — uploaded TTML, uploaded LRC, scanned synced, uploaded TXT, scanned plain — empty ones left out, each naming its source. The Subsonic methods keep reading the scanned sheets, without a source: their contract is frozen. - GET /api/v2/tracks/{id}/lyrics/file downloads the first sheet, or one upload by ?format=, as application/octet-stream, an attachment, and nosniff, so an uploaded TTML is never rendered in this origin. - TTML is read with roxmltree, whose default options refuse a DTD: lines from each

with a begin in clock or offset time, frames refused, xml:lang as the language. Word timings stay in the stored original. - The web client's Properties shows each sheet with its source, the one shown first, and lets owners and managers upload and remove a sheet; anyone downloads the shown one under the server's name. Signed-off-by: InstaZDLL --- Cargo.lock | 1 + Cargo.toml | 4 + docs/rfcs/RFC-012-library-browser.md | 10 +- .../20261021000000_track_lyrics_upload.sql | 22 ++ src/api/mod.rs | 12 + src/api/tracks.rs | 95 +++++- src/lib.rs | 3 + src/lyrics.rs | 319 ++++++++++++++++++ src/media.rs | 2 +- src/services/lyrics_uploads.rs | 309 +++++++++++++++++ src/services/mod.rs | 4 + tests/native_api.rs | 273 +++++++++++++++ webapp/e2e/studio-nocturne.spec.ts | 122 +++++++ webapp/src/api/index.ts | 5 + webapp/src/api/tracks.ts | 61 +++- webapp/src/components/lyrics-section.tsx | 215 ++++++++++++ webapp/src/components/track-properties.tsx | 22 +- webapp/src/lib/download.test.ts | 36 +- webapp/src/lib/download.ts | 45 ++- webapp/src/state/i18n.tsx | 31 ++ webapp/src/styles/styles.css | 25 ++ 21 files changed, 1586 insertions(+), 30 deletions(-) create mode 100644 migrations-v2/20261021000000_track_lyrics_upload.sql create mode 100644 src/services/lyrics_uploads.rs create mode 100644 webapp/src/components/lyrics-section.tsx diff --git a/Cargo.lock b/Cargo.lock index 90f74f6e..70136239 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3660,6 +3660,7 @@ dependencies = [ "percent-encoding", "rand 0.10.3", "reqwest", + "roxmltree", "rust-embed", "serde", "serde_json", diff --git a/Cargo.toml b/Cargo.toml index 5c2dda97..2906a04d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -39,6 +39,10 @@ encoding_rs = "0.8.42" futures-util = "0.3" hex = "0.4" lofty = "0.25" +# TTML lyrics uploads (RFC-012 decision 6). Already in the lock file through +# the artwork renderer; named here because the server now parses XML itself. +# Its default options refuse a DTD, which is what keeps entities out. +roxmltree = "0.21" libc = "0.2" md-5 = "0.11" # Already in the tree through `url`; declared because `playlist_file` reads diff --git a/docs/rfcs/RFC-012-library-browser.md b/docs/rfcs/RFC-012-library-browser.md index 4a0d708d..c3bf9c55 100644 --- a/docs/rfcs/RFC-012-library-browser.md +++ b/docs/rfcs/RFC-012-library-browser.md @@ -3,10 +3,12 @@ - **Status**: Proposed - **Implemented by**: in progress, in the order of "The order of the work" below: [#298](https://github.com/InstaZDLL/waveflow-server/pull/298) (the - context menu and Properties, with `relative_path` on the track) and the pull - request adding download (`?download=1` on the stream ticket). The *Status* - field above never flips in this project; this line is the one that says what - is running. + context menu and Properties, with `relative_path` on the track), + [#299](https://github.com/InstaZDLL/waveflow-server/pull/299) (download, + `?download=1` on the stream ticket) and the pull request adding uploaded + lyrics (`track_lyrics_upload`, the TTML parser, the order, and the lyrics + download). The *Status* field above never flips in this project; this line + is the one that says what is running. - **Date**: 2026-10-21 - **Revised**: 2026-10-21, after review on #297: the **Received** folder's contract, the depth check on a move, the folder cap that bounds the tree's diff --git a/migrations-v2/20261021000000_track_lyrics_upload.sql b/migrations-v2/20261021000000_track_lyrics_upload.sql new file mode 100644 index 00000000..9bf551f3 --- /dev/null +++ b/migrations-v2/20261021000000_track_lyrics_upload.sql @@ -0,0 +1,22 @@ +-- Lyrics uploaded to a track (RFC-012 decision 6). +-- +-- A table of its own because `track_lyrics` belongs to the scanner: every +-- scan rebuilds a track's rows from what the file and its sidecars hold, and +-- would erase anything else on the next pass. An upload is a correction, and +-- corrections live beside the file; nothing here is ever written to disk. +-- +-- One upload per track and format: uploading again replaces it. `content` is +-- the uploaded file as it came, decoded to UTF-8, so a download gives back +-- what was uploaded rather than a re-serialisation of it. +CREATE TABLE track_lyrics_upload ( + track_id TEXT NOT NULL, + library_id TEXT NOT NULL, + format TEXT NOT NULL CHECK (format IN ('lrc', 'txt', 'ttml')), + lang TEXT NOT NULL CHECK (length(trim(lang)) > 0), + content TEXT NOT NULL CHECK (length(content) > 0), + uploaded_by TEXT NOT NULL, + uploaded_at INTEGER NOT NULL, + PRIMARY KEY (track_id, format), + FOREIGN KEY (track_id, library_id) + REFERENCES track(id, library_id) ON DELETE CASCADE +) STRICT; diff --git a/src/api/mod.rs b/src/api/mod.rs index dbae6090..60c0f703 100644 --- a/src/api/mod.rs +++ b/src/api/mod.rs @@ -138,6 +138,18 @@ pub fn router(state: AppState) -> Router { get(get_track).patch(update_track), ) .route("/api/v2/tracks/{track_id}/lyrics", get(get_track_lyrics)) + // A sheet is text, but a mebibyte of it: the one route besides the + // upload chunks that asks for more than the global 16 KiB. + .route( + "/api/v2/tracks/{track_id}/lyrics/{format}", + put(put_track_lyrics).delete(delete_track_lyrics).layer( + axum::extract::DefaultBodyLimit::max(crate::services::MAX_LYRICS_UPLOAD_BYTES), + ), + ) + .route( + "/api/v2/tracks/{track_id}/lyrics/file", + get(get_track_lyrics_file), + ) .route( "/api/v2/tracks/{track_id}/overrides", get(get_track_overrides), diff --git a/src/api/tracks.rs b/src/api/tracks.rs index c9837b0f..0e350b9f 100644 --- a/src/api/tracks.rs +++ b/src/api/tracks.rs @@ -99,14 +99,107 @@ pub async fn get_track_lyrics( headers: HeaderMap, ) -> Result, ApiError> { let user = authenticated(&state, &headers, Access::Read).await?; + // The native answer: uploads included, in RFC-012's order, each sheet + // naming its source. The Subsonic façade reads `lyrics` instead. state .services - .lyrics(user.id, track_id) + .track_lyrics(user.id, track_id) .await .map(Json) .map_err(service_error) } +fn lyrics_format(value: &str) -> Result { + crate::lyrics::UploadFormat::parse(value).ok_or(ApiError::NotFound) +} + +/// Uploads a lyrics sheet to a track (RFC-012 decision 6). +/// +/// The body is the file, as it was saved: `lrc`, `txt` or `ttml`, at most a +/// mebibyte, in any encoding `waveflow_core::text_encoding` reads. Uploading +/// again in the same format replaces it. A sheet with nothing usable in it is +/// refused with `422`. Owners and managers only; anyone else gets the `404` a +/// track they could not see would give. +#[utoipa::path(put, path = "/api/v2/tracks/{track_id}/lyrics/{format}", tag = "catalog", params(("track_id" = Uuid, Path), ("format" = String, Path, description = "lrc, txt or ttml")), request_body(content = Vec, content_type = "application/octet-stream"), responses((status = 200, body = crate::lyrics::LyricsList), (status = 401, body = ErrorResponse), (status = 404, body = ErrorResponse), (status = 413), (status = 422, body = ErrorResponse)))] +pub async fn put_track_lyrics( + State(state): State, + Path((track_id, format)): Path<(Uuid, String)>, + headers: HeaderMap, + body: axum::body::Bytes, +) -> Result, ApiError> { + let user = authenticated(&state, &headers, Access::Write).await?; + let format = lyrics_format(&format)?; + state + .services + .upload_lyrics(user.id, track_id, format, &body) + .await + .map(Json) + .map_err(service_error) +} + +/// Removes an uploaded sheet; the track falls back to what the scanner found. +#[utoipa::path(delete, path = "/api/v2/tracks/{track_id}/lyrics/{format}", tag = "catalog", params(("track_id" = Uuid, Path), ("format" = String, Path, description = "lrc, txt or ttml")), responses((status = 204), (status = 401, body = ErrorResponse), (status = 404, body = ErrorResponse)))] +pub async fn delete_track_lyrics( + State(state): State, + Path((track_id, format)): Path<(Uuid, String)>, + headers: HeaderMap, +) -> Result { + let user = authenticated(&state, &headers, Access::Write).await?; + let format = lyrics_format(&format)?; + state + .services + .delete_lyrics_upload(user.id, track_id, format) + .await + .map(|()| StatusCode::NO_CONTENT) + .map_err(service_error) +} + +#[derive(Debug, Deserialize)] +pub struct LyricsFileQuery { + pub format: Option, +} + +/// A track's lyrics as a file to save. +/// +/// The sheet the lyrics route puts first, or with `format`, one upload by +/// name. Never rendered: `application/octet-stream`, an attachment, and +/// `nosniff`, whatever the format — an uploaded `.ttml` is XML somebody else +/// wrote, and inline under an XML type it would be a document running in this +/// server's origin. +#[utoipa::path(get, path = "/api/v2/tracks/{track_id}/lyrics/file", tag = "catalog", params(("track_id" = Uuid, Path), ("format" = Option, Query, description = "lrc, txt or ttml: one upload by name")), responses((status = 200, content_type = "application/octet-stream"), (status = 401, body = ErrorResponse), (status = 404, body = ErrorResponse)))] +pub async fn get_track_lyrics_file( + State(state): State, + Path(track_id): Path, + Query(query): Query, + headers: HeaderMap, +) -> Result { + let user = authenticated(&state, &headers, Access::Read).await?; + let format = query.format.as_deref().map(lyrics_format).transpose()?; + let file = state + .services + .lyrics_file(user.id, track_id, format) + .await + .map_err(service_error)?; + Ok(( + [ + ( + header::CONTENT_TYPE, + HeaderValue::from_static("application/octet-stream"), + ), + ( + header::CONTENT_DISPOSITION, + crate::media::attachment(&file.name), + ), + ( + header::X_CONTENT_TYPE_OPTIONS, + HeaderValue::from_static("nosniff"), + ), + ], + file.content, + ) + .into_response()) +} + /// Corrects some of a track's tags, leaving the rest as they are. /// /// A partial patch in three states. A field **absent** from the body leaves diff --git a/src/lib.rs b/src/lib.rs index 80e98463..91559084 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -142,6 +142,9 @@ fn chunk_body_limit(limits: &config::UploadLimits) -> usize { api::get_track, api::update_track, api::get_track_lyrics, + api::put_track_lyrics, + api::delete_track_lyrics, + api::get_track_lyrics_file, api::get_track_overrides, api::list_albums, api::list_genres, diff --git a/src/lyrics.rs b/src/lyrics.rs index 2e1f2c07..1b6167a7 100644 --- a/src/lyrics.rs +++ b/src/lyrics.rs @@ -29,6 +29,234 @@ pub struct StructuredLyrics { pub synced: bool, #[serde(rename = "line")] pub lines: Vec, + /// Where this sheet came from: `upload_ttml`, `upload_lrc`, `upload_txt`, + /// `sidecar_lrc`, `sidecar_text` or `embedded` (RFC-012 decision 6). + /// + /// Sent by the native route only. The Subsonic façade builds its sheets + /// without it, and `None` is never serialised, so its frozen answer keeps + /// the shape validated clients were tested against. + #[serde(skip_serializing_if = "Option::is_none")] + pub source: Option, +} + +/// The formats a sheet can be uploaded in (RFC-012 decision 6). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum UploadFormat { + Lrc, + Txt, + Ttml, +} + +impl UploadFormat { + pub fn parse(value: &str) -> Option { + match value { + "lrc" => Some(Self::Lrc), + "txt" => Some(Self::Txt), + "ttml" => Some(Self::Ttml), + _ => None, + } + } + + /// As stored, and as the file extension a download carries. + pub fn as_str(self) -> &'static str { + match self { + Self::Lrc => "lrc", + Self::Txt => "txt", + Self::Ttml => "ttml", + } + } + + /// The `source` a sheet read from this upload reports. + pub fn source(self) -> &'static str { + match self { + Self::Lrc => "upload_lrc", + Self::Txt => "upload_txt", + Self::Ttml => "upload_ttml", + } + } +} + +/// An uploaded sheet, read: its language and its lines. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ParsedUpload { + pub lang: String, + pub synced: bool, + pub lines: Vec, +} + +/// The language a sheet that names none carries, as OpenSubsonic spells it. +pub const UNKNOWN_LANG: &str = "xxx"; + +/// Read an uploaded sheet, or `None` when it holds nothing usable. +/// +/// "Usable" is the RFC's rule, so an empty upload is refused at the door rather +/// than stored to hide a real sheet later: a `.txt` with no non-blank line, an +/// `.lrc` with no line of text, a `.ttml` from which no timed line parses. +pub fn parse_upload(format: UploadFormat, content: &str) -> Option { + let parsed = match format { + UploadFormat::Txt => ParsedUpload { + lang: UNKNOWN_LANG.to_owned(), + synced: false, + lines: lines(content, false), + }, + UploadFormat::Lrc => { + let synced = has_lrc_timestamps(content); + ParsedUpload { + lang: lrc_language(content).unwrap_or_else(|| UNKNOWN_LANG.to_owned()), + synced, + lines: lines(content, synced), + } + } + UploadFormat::Ttml => parse_ttml(content)?, + }; + has_text(&parsed.lines).then_some(parsed) +} + +/// Whether any line says something. A sheet of blank lines is no sheet. +pub fn has_text(lines: &[LyricsLine]) -> bool { + lines.iter().any(|line| !line.value.trim().is_empty()) +} + +/// The `[la:…]` tag an LRC file may carry, the only language it can name. +fn lrc_language(content: &str) -> Option { + normalize_content(content).lines().find_map(|line| { + let tag = line.trim().strip_prefix("[la:")?.strip_suffix(']')?.trim(); + (!tag.is_empty()).then(|| tag.to_owned()) + }) +} + +/// The lines of a TTML document: each `

` with a `begin` it can read. +/// +/// Word timings in ``s are kept in the stored original and not served — +/// both contracts carry line timings only (RFC-012 decision 6) — so a line's +/// text is its words joined, whitespace collapsed. A `

` whose `begin` is in +/// frames, or missing, is left out rather than guessed at. The document's +/// `xml:lang`, on the root or the body, names the language. +/// +/// `roxmltree`'s default options refuse a DTD, so no entity is ever expanded: +/// an uploaded document cannot reach the file system or inflate itself. +pub fn parse_ttml(content: &str) -> Option { + let document = roxmltree::Document::parse(content).ok()?; + let root = document.root_element(); + if root.tag_name().name() != "tt" { + return None; + } + let lang = root + .descendants() + .filter(|node| node.is_element()) + .find_map(|node| node.attribute(("http://www.w3.org/XML/1998/namespace", "lang"))) + .map(str::trim) + .filter(|lang| !lang.is_empty()) + .unwrap_or(UNKNOWN_LANG) + .to_owned(); + let mut lines = Vec::new(); + for (order, paragraph) in root + .descendants() + .filter(|node| node.is_element() && node.tag_name().name() == "p") + .enumerate() + { + let Some(start) = paragraph.attribute("begin").and_then(ttml_time) else { + continue; + }; + let text = paragraph + .descendants() + .filter(|node| node.is_text()) + .filter_map(|node| node.text()) + .collect::(); + let value = text.split_whitespace().collect::>().join(" "); + lines.push((start, order, value)); + } + lines.sort_by_key(|(start, order, _)| (*start, *order)); + Some(ParsedUpload { + lang, + synced: true, + lines: lines + .into_iter() + .map(|(start, _, value)| LyricsLine { + start: Some(start), + value, + }) + .collect(), + }) +} + +/// A TTML time expression in milliseconds. +/// +/// Clock time — `hh:mm:ss`, `mm:ss` or `ss`, each with an optional fraction +/// — and offset time with a metric: `h`, `m`, `s` or `ms`. Frames (a fourth +/// clock field, or the `f` and `t` metrics) need the document's frame and tick +/// rates, which a lyrics sheet has no business making the server compute; they +/// answer `None`, and the line is left out. +fn ttml_time(value: &str) -> Option { + let value = value.trim(); + let offset = |number: &str, unit_ms: f64| -> Option { + let number: f64 = number.parse().ok()?; + let ms = number * unit_ms; + (ms.is_finite() && ms >= 0.0 && ms <= i64::MAX as f64).then(|| ms.round() as i64) + }; + if let Some(number) = value.strip_suffix("ms") { + return offset(number, 1.0); + } + if let Some(number) = value.strip_suffix('h') { + return offset(number, 3_600_000.0); + } + if let Some(number) = value.strip_suffix('m') { + return offset(number, 60_000.0); + } + if let Some(number) = value.strip_suffix('s') { + return offset(number, 1_000.0); + } + let fields: Vec<&str> = value.split(':').collect(); + if fields.is_empty() || fields.len() > 3 || fields.iter().any(|field| field.is_empty()) { + return None; + } + let (whole, last) = fields.split_at(fields.len() - 1); + let seconds: f64 = last[0].parse().ok()?; + if !seconds.is_finite() || seconds < 0.0 || (!whole.is_empty() && seconds >= 60.0) { + return None; + } + let mut total = 0i64; + for (index, field) in whole.iter().enumerate() { + let number: i64 = field.parse().ok()?; + // Hours are unbounded; minutes under hours are not. + if number < 0 || (index == whole.len() - 1 && whole.len() == 2 && number >= 60) { + return None; + } + let unit = if whole.len() - index == 2 { + 3_600_000 + } else { + 60_000 + }; + total = total.checked_add(number.checked_mul(unit)?)?; + } + total.checked_add((seconds * 1000.0).round() as i64) +} + +/// A synced sheet written back as LRC, for a download of lyrics the scanner +/// found: `[mm:ss.cc]` per line, minutes unbounded as LRC allows. +pub fn to_lrc(lines: &[LyricsLine]) -> String { + let mut out = String::new(); + for line in lines { + if let Some(start) = line.start { + let centis = (start % 1000) / 10; + let seconds = (start / 1000) % 60; + let minutes = start / 60_000; + out.push_str(&format!("[{minutes:02}:{seconds:02}.{centis:02}]")); + } + out.push_str(&line.value); + out.push('\n'); + } + out +} + +/// A plain sheet written back as text, one line per line. +pub fn to_text(lines: &[LyricsLine]) -> String { + let mut out = String::new(); + for line in lines { + out.push_str(&line.value); + out.push('\n'); + } + out } #[derive(Debug, Clone, Serialize, ToSchema, PartialEq, Eq)] @@ -174,6 +402,97 @@ mod tests { ); } + #[test] + fn reads_every_ttml_clock_and_offset_form() { + assert_eq!(ttml_time("01:02:03.5"), Some(3_723_500)); + assert_eq!(ttml_time("2:03.250"), Some(123_250)); + assert_eq!(ttml_time("12.345"), Some(12_345)); + assert_eq!(ttml_time("1.5s"), Some(1_500)); + assert_eq!(ttml_time("1500ms"), Some(1_500)); + assert_eq!(ttml_time("2m"), Some(120_000)); + assert_eq!(ttml_time("1h"), Some(3_600_000)); + // Frames need a frame rate the sheet does not get to set. + assert_eq!(ttml_time("00:00:01:12"), None); + assert_eq!(ttml_time("12f"), None); + // Out of range, and not a time at all. + assert_eq!(ttml_time("00:61:00"), None); + assert_eq!(ttml_time("-1s"), None); + assert_eq!(ttml_time(""), None); + } + + #[test] + fn reads_a_ttml_sheet_with_word_spans() { + let ttml = r#" + +

+

夜の 街

+

first

+

no time, left out

+

frames, left out

+
+"#; + let parsed = parse_ttml(ttml).expect("a sheet"); + assert_eq!(parsed.lang, "ja"); + assert!(parsed.synced); + assert_eq!( + parsed.lines, + vec![ + LyricsLine { + start: Some(1_000), + value: "first".into(), + }, + LyricsLine { + start: Some(5_500), + value: "夜の 街".into(), + }, + ] + ); + } + + #[test] + fn refuses_a_ttml_document_with_a_dtd() { + // An entity is how an XML upload reaches a file or inflates itself; + // the default parser options refuse the DTD that would declare one. + let hostile = r#" +]> +

&x;

"#; + assert_eq!(parse_ttml(hostile), None); + assert_eq!(parse_upload(UploadFormat::Ttml, hostile), None); + } + + #[test] + fn refuses_uploads_with_nothing_usable_in_them() { + assert_eq!(parse_upload(UploadFormat::Txt, " \n\t\n"), None); + assert_eq!( + parse_upload(UploadFormat::Lrc, "[ar:Someone]\n[00:01.00]\n"), + None + ); + assert_eq!( + parse_upload( + UploadFormat::Ttml, + r#"

untimed

"# + ), + None + ); + assert_eq!( + parse_upload(UploadFormat::Ttml, "

x

"), + None + ); + let lrc = parse_upload(UploadFormat::Lrc, "[la:fr]\n[00:01.00]bonjour").expect("a sheet"); + assert_eq!(lrc.lang, "fr"); + assert!(lrc.synced); + let plain = parse_upload(UploadFormat::Lrc, "no timestamps\nat all").expect("a sheet"); + assert!(!plain.synced, "an .lrc without timings is plain text"); + } + + #[test] + fn writes_scanned_sheets_back_as_lrc_and_text() { + let synced = lines("[00:01.25]one\n[61:02.5]two", true); + assert_eq!(to_lrc(&synced), "[00:01.25]one\n[61:02.50]two\n"); + assert_eq!(lines(&to_lrc(&synced), true), synced, "round trip"); + assert_eq!(to_text(&lines("a\n\nb", false)), "a\n\nb\n"); + } + #[test] fn rejects_negative_and_overflowing_timestamps() { assert_eq!(parse_timestamp("-1:00"), None); diff --git a/src/media.rs b/src/media.rs index 2bc16228..3e76d5aa 100644 --- a/src/media.rs +++ b/src/media.rs @@ -1319,7 +1319,7 @@ async fn download_name(state: &AppState, user_id: Uuid, track: &StreamTrack) -> /// ASCII — replaced by `_`, and a separator too, so no browser builds a path /// from it. `filename*` carries the real name, percent-encoded UTF-8, and /// every current browser prefers it. -fn attachment(name: &str) -> HeaderValue { +pub(crate) fn attachment(name: &str) -> HeaderValue { let cleaned: String = name .chars() .map(|c| match c { diff --git a/src/services/lyrics_uploads.rs b/src/services/lyrics_uploads.rs new file mode 100644 index 00000000..04db88b7 --- /dev/null +++ b/src/services/lyrics_uploads.rs @@ -0,0 +1,309 @@ +//! Lyrics uploaded to a track, and the one order every reader chooses by +//! (RFC-012 decision 6). +//! +//! Uploads live in `track_lyrics_upload`, beside the scanner's +//! `track_lyrics`, and nothing here writes a file. The native lyrics route, +//! the properties panel and the download all read [`DomainServices::track_lyrics`], +//! so the order is decided once. The Subsonic façade keeps reading +//! [`DomainServices::lyrics`], which knows nothing of uploads: its contract is +//! frozen. + +use std::str::FromStr; + +use sqlx::Row; +use uuid::Uuid; + +use super::{DomainServices, ServiceError}; +use crate::authentication::now_ms; +use crate::lyrics::{self, LyricsList, StructuredLyrics, UploadFormat}; + +/// The sidecar's cap, so an upload is held to what a scan would read. +pub const MAX_UPLOAD_BYTES: usize = 1024 * 1024; + +/// One sheet, ready to be ranked. +struct Candidate { + rank: u8, + tie: u8, + position: i64, + sheet: StructuredLyrics, +} + +/// A lyrics file to hand over as a download. +#[derive(Debug)] +pub struct LyricsFile { + /// The name it is saved under, extension included. + pub name: String, + pub content: String, +} + +impl DomainServices { + /// A track's lyrics as the native API answers them: every sheet, uploaded + /// ones included, in the order of RFC-012 decision 6, empty ones left + /// out — so the first entry is always the sheet to show. + /// + /// 1. an uploaded `.ttml`; + /// 2. an uploaded `.lrc` with timings; + /// 3. a scanned synced sheet, a sidecar before embedded lyrics; + /// 4. an uploaded `.txt`, then an uploaded `.lrc` without timings; + /// 5. scanned plain text, a sidecar before embedded lyrics. + pub async fn track_lyrics( + &self, + user_id: Uuid, + track_id: Uuid, + ) -> Result { + let track = sqlx::query( + "SELECT t.title, t.artist_display FROM track t \ + JOIN library_member m ON m.library_id=t.library_id \ + WHERE m.user_id=? AND t.id=? AND t.is_available=1", + ) + .bind(user_id.to_string()) + .bind(track_id.to_string()) + .fetch_optional(self.db.pool()) + .await? + .ok_or(ServiceError::NotFound)?; + let display_title: String = track.try_get("title")?; + let display_artist: Option = track.try_get("artist_display")?; + let sheet = |lang: String, synced: bool, lines, source: &str| StructuredLyrics { + display_artist: display_artist.clone(), + display_title: display_title.clone(), + lang, + synced, + lines, + source: Some(source.to_owned()), + }; + + let mut candidates = Vec::new(); + let scanned = sqlx::query( + "SELECT source, lang, synced, content, position FROM track_lyrics \ + WHERE track_id=? ORDER BY position", + ) + .bind(track_id.to_string()) + .fetch_all(self.db.pool()) + .await?; + for row in scanned { + let source: String = row.try_get("source")?; + let synced = row.try_get::("synced")? != 0; + let content: String = row.try_get("content")?; + let lines = lyrics::lines(&content, synced); + if !lyrics::has_text(&lines) { + continue; + } + candidates.push(Candidate { + rank: if synced { 2 } else { 4 }, + tie: u8::from(source == "embedded"), + position: row.try_get("position")?, + sheet: sheet(row.try_get("lang")?, synced, lines, &source), + }); + } + + let uploads = + sqlx::query("SELECT format, content FROM track_lyrics_upload WHERE track_id=?") + .bind(track_id.to_string()) + .fetch_all(self.db.pool()) + .await?; + for row in uploads { + let Some(format) = UploadFormat::parse(row.try_get::<&str, _>("format")?) else { + continue; + }; + let content: String = row.try_get("content")?; + // Read again rather than trusted: a row an earlier version stored + // with nothing in it is skipped like any empty source. + let Some(parsed) = lyrics::parse_upload(format, &content) else { + continue; + }; + let (rank, tie) = match (format, parsed.synced) { + (UploadFormat::Ttml, _) => (0, 0), + (UploadFormat::Lrc, true) => (1, 0), + (UploadFormat::Txt, _) => (3, 0), + (UploadFormat::Lrc, false) => (3, 1), + }; + candidates.push(Candidate { + rank, + tie, + position: 0, + sheet: sheet(parsed.lang, parsed.synced, parsed.lines, format.source()), + }); + } + + candidates.sort_by_key(|candidate| (candidate.rank, candidate.tie, candidate.position)); + Ok(LyricsList { + track_id, + structured_lyrics: candidates + .into_iter() + .map(|candidate| candidate.sheet) + .collect(), + }) + } + + /// Store an uploaded sheet, replacing any earlier upload in the same + /// format, and answer the track's lyrics as they now read. + /// + /// Gated like a metadata correction — owner and manager — and refused with + /// `Invalid` when the file holds nothing usable, so an empty sheet never + /// stands in front of a real one. The bytes are decoded through + /// `waveflow_core::text_encoding`, so a sheet saved in GB18030 or + /// Shift_JIS reads as its author wrote it. + pub async fn upload_lyrics( + &self, + user_id: Uuid, + track_id: Uuid, + format: UploadFormat, + bytes: &[u8], + ) -> Result { + if bytes.is_empty() || bytes.len() > MAX_UPLOAD_BYTES { + return Err(ServiceError::Invalid); + } + let content = waveflow_core::text_encoding::decode(bytes).into_owned(); + let parsed = lyrics::parse_upload(format, &content).ok_or(ServiceError::Invalid)?; + { + let _writer = self.db.writer_guard().await; + let mut tx = self.db.pool().begin().await?; + let library_id = self + .lyrics_writer_library(&mut tx, user_id, track_id) + .await?; + sqlx::query( + "INSERT INTO track_lyrics_upload \ + (track_id, library_id, format, lang, content, uploaded_by, uploaded_at) \ + VALUES (?, ?, ?, ?, ?, ?, ?) \ + ON CONFLICT(track_id, format) DO UPDATE SET \ + lang=excluded.lang, content=excluded.content, \ + uploaded_by=excluded.uploaded_by, uploaded_at=excluded.uploaded_at", + ) + .bind(track_id.to_string()) + .bind(library_id) + .bind(format.as_str()) + .bind(&parsed.lang) + .bind(&content) + .bind(user_id.to_string()) + .bind(now_ms()) + .execute(&mut *tx) + .await?; + tx.commit().await?; + } + self.track_lyrics(user_id, track_id).await + } + + /// Remove one upload; the track falls back to what the scanner found. + /// `NotFound` when the track has no upload in that format. + pub async fn delete_lyrics_upload( + &self, + user_id: Uuid, + track_id: Uuid, + format: UploadFormat, + ) -> Result<(), ServiceError> { + let _writer = self.db.writer_guard().await; + let mut tx = self.db.pool().begin().await?; + self.lyrics_writer_library(&mut tx, user_id, track_id) + .await?; + let removed = sqlx::query("DELETE FROM track_lyrics_upload WHERE track_id=? AND format=?") + .bind(track_id.to_string()) + .bind(format.as_str()) + .execute(&mut *tx) + .await? + .rows_affected(); + tx.commit().await?; + if removed == 0 { + return Err(ServiceError::NotFound); + } + Ok(()) + } + + /// The lyrics as a file: one upload by name when `format` is given, and + /// otherwise the sheet [`track_lyrics`](Self::track_lyrics) puts first. + /// + /// An upload comes back as it was uploaded. A scanned sheet is written as + /// `.lrc` when it is synced and `.txt` when it is not. + pub async fn lyrics_file( + &self, + user_id: Uuid, + track_id: Uuid, + format: Option, + ) -> Result { + let list = self.track_lyrics(user_id, track_id).await?; + let first = list.structured_lyrics.first(); + let stem = { + let title = first + .map(|sheet| sheet.display_title.trim().to_owned()) + .unwrap_or_default(); + match first + .and_then(|sheet| sheet.display_artist.as_deref()) + .map(str::trim) + .filter(|artist| !artist.is_empty()) + { + Some(artist) => format!("{artist} - {title}"), + None => title, + } + }; + let stem = if stem.is_empty() { + "lyrics".to_owned() + } else { + stem + }; + + let upload = match format { + Some(format) => Some(format), + None => { + let first = first.ok_or(ServiceError::NotFound)?; + match first.source.as_deref() { + Some("upload_ttml") => Some(UploadFormat::Ttml), + Some("upload_lrc") => Some(UploadFormat::Lrc), + Some("upload_txt") => Some(UploadFormat::Txt), + _ => { + let (content, extension) = if first.synced { + (lyrics::to_lrc(&first.lines), "lrc") + } else { + (lyrics::to_text(&first.lines), "txt") + }; + return Ok(LyricsFile { + name: format!("{stem}.{extension}"), + content, + }); + } + } + } + }; + let upload = upload.ok_or(ServiceError::NotFound)?; + // Membership was proved by `track_lyrics` above, and the upload row + // carries the track's library: reading it by track id is reading + // within what the caller may see. + let content: String = sqlx::query_scalar( + "SELECT content FROM track_lyrics_upload WHERE track_id=? AND format=?", + ) + .bind(track_id.to_string()) + .bind(upload.as_str()) + .fetch_optional(self.db.pool()) + .await? + .ok_or(ServiceError::NotFound)?; + Ok(LyricsFile { + name: format!("{stem}.{}", upload.as_str()), + content, + }) + } + + /// The track's library, if the caller may write its metadata. `NotFound` + /// for a track they cannot see and `Forbidden` for one they may only read, + /// which every surface blurs into the same `404`. + async fn lyrics_writer_library( + &self, + tx: &mut sqlx::Transaction<'_, sqlx::Sqlite>, + user_id: Uuid, + track_id: Uuid, + ) -> Result { + let row = sqlx::query( + "SELECT t.library_id, m.role FROM track t \ + JOIN library_member m ON m.library_id=t.library_id \ + WHERE t.id=? AND m.user_id=?", + ) + .bind(track_id.to_string()) + .bind(user_id.to_string()) + .fetch_optional(&mut **tx) + .await? + .ok_or(ServiceError::NotFound)?; + let role = crate::database::LibraryRole::from_str(row.try_get::<&str, _>("role")?) + .map_err(|_| ServiceError::Invalid)?; + if !role.may_write_metadata() { + return Err(ServiceError::Forbidden); + } + Ok(row.try_get("library_id")?) + } +} diff --git a/src/services/mod.rs b/src/services/mod.rs index a05497e8..044e834f 100644 --- a/src/services/mod.rs +++ b/src/services/mod.rs @@ -1364,6 +1364,7 @@ mod credentials; mod favorites; mod lastfm; mod library_events; +mod lyrics_uploads; mod playback; mod playlists; mod radio; @@ -1392,6 +1393,7 @@ pub use lastfm::{ LastFmApproval, LastFmJourneyStart, LastFmSessionExchange, LASTFM_CALLBACK_PREFIX, LASTFM_JOURNEY_COOKIE, }; +pub use lyrics_uploads::{LyricsFile, MAX_UPLOAD_BYTES as MAX_LYRICS_UPLOAD_BYTES}; pub use scrobbling::{ ScrobbleDestinationName, ScrobbleDrain, ScrobbleEnvelope, ScrobbleLinkState, ScrobbleProvider, ScrobbleTarget, ScrobbleUnavailable, ScrobbleVerdict, UncertainScrobble, @@ -1916,6 +1918,8 @@ fn lyrics_list_from_rows( .unwrap_or_else(|| "xxx".into()), synced, lines: lyrics::lines(&content, synced), + // The façade's sheets name no source: its answer is frozen. + source: None, }); } Ok(LyricsList { diff --git a/tests/native_api.rs b/tests/native_api.rs index 3383dab0..1899a3cb 100644 --- a/tests/native_api.rs +++ b/tests/native_api.rs @@ -4040,3 +4040,276 @@ async fn radio_stations_round_trip_and_stay_inside_one_account() { assert_eq!(status, StatusCode::NOT_FOUND, "{method}"); } } + +/// Lyrics uploaded to a track (RFC-012 decision 6), through the router. +/// +/// Passwords are drawn at run time: a literal handed to `hash_password` is +/// what CodeQL's `rust/hard-coded-cryptographic-value` reports, and the +/// ruleset blocks a pull request on it. +#[tokio::test] +async fn uploaded_lyrics_lead_the_order_download_as_they_came_and_stay_off_subsonic() { + let (_temp, config, state) = test_app().await; + let password = uuid::Uuid::new_v4().to_string(); + let hash = security::hash_password(&password).unwrap(); + let owner = state + .db + .create_account("lyrics-owner", &hash, AccountRole::User, now_ms()) + .await + .unwrap(); + let listener = state + .db + .create_account("lyrics-listener", &hash, AccountRole::User, now_ms()) + .await + .unwrap(); + let music = config.data_dir.join("uploaded-lyrics"); + std::fs::create_dir_all(&music).unwrap(); + write_test_wav(&music.join("Song.wav")); + // A scanned synced sheet, so every rank below has a neighbour to beat. + std::fs::write(music.join("Song.lrc"), "[00:01.00]from the sidecar\n").unwrap(); + let root = std::fs::canonicalize(&music).unwrap(); + let library_id = state + .db + .create_library(owner, "Lyrics", &root, LibraryVisibility::Private, now_ms()) + .await + .unwrap(); + state + .db + .add_library_member( + owner, + library_id, + listener, + waveflow_server::database::LibraryRole::Listener, + now_ms(), + ) + .await + .unwrap(); + run_scan( + &state, + owner, + LibraryRecord { + id: library_id, + name: "Lyrics".into(), + root_path: root, + }, + ) + .await; + let track = state + .db + .list_tracks_for_user(owner, library_id) + .await + .unwrap()[0] + .id; + let router = waveflow_server::app(&config, state.clone()); + let owner_token = login_token(&router, "lyrics-owner", &password).await; + let listener_token = login_token(&router, "lyrics-listener", &password).await; + + let send = |method: Method, uri: String, token: &str, body: Vec| { + let router = router.clone(); + let token = token.to_owned(); + async move { + router + .oneshot( + Request::builder() + .method(method) + .uri(uri) + .header("authorization", format!("Bearer {token}")) + .body(Body::from(body)) + .unwrap(), + ) + .await + .unwrap() + } + }; + let sources = |list: &serde_json::Value| -> Vec { + list["structuredLyrics"] + .as_array() + .unwrap() + .iter() + .map(|sheet| sheet["source"].as_str().unwrap().to_owned()) + .collect() + }; + let lyrics_uri = format!("/api/v2/tracks/{track}/lyrics"); + + // Before any upload: the scanned sheet, naming its source. + let before = json_body(send(Method::GET, lyrics_uri.clone(), &owner_token, vec![]).await).await; + assert_eq!(sources(&before), ["sidecar_lrc"]); + + // A plain upload does not take the timings away from a synced scan. + let txt = send( + Method::PUT, + format!("{lyrics_uri}/txt"), + &owner_token, + b"plain words\n".to_vec(), + ) + .await; + assert_eq!(txt.status(), StatusCode::OK); + assert_eq!( + sources(&json_body(txt).await), + ["sidecar_lrc", "upload_txt"] + ); + + // A synced upload outranks the scan, and TTML outranks LRC. + let lrc = send( + Method::PUT, + format!("{lyrics_uri}/lrc"), + &owner_token, + b"[la:en]\n[00:02.00]uploaded lrc\n".to_vec(), + ) + .await; + assert_eq!(lrc.status(), StatusCode::OK); + let ttml_body = concat!( + r#"
"#, + r#"

bonjour tout

"#, + r#"
"# + ); + let ttml = send( + Method::PUT, + format!("{lyrics_uri}/ttml"), + &owner_token, + ttml_body.as_bytes().to_vec(), + ) + .await; + assert_eq!(ttml.status(), StatusCode::OK); + let ordered = json_body(ttml).await; + assert_eq!( + sources(&ordered), + ["upload_ttml", "upload_lrc", "sidecar_lrc", "upload_txt"] + ); + assert_eq!(ordered["structuredLyrics"][0]["lang"], "fr"); + assert_eq!( + ordered["structuredLyrics"][0]["line"][0]["value"], + "bonjour tout" + ); + assert_eq!(ordered["structuredLyrics"][0]["line"][0]["start"], 3000); + + // The download gives the first sheet back as it was uploaded, never + // rendered; with `format`, the upload named. + let file = send( + Method::GET, + format!("{lyrics_uri}/file"), + &owner_token, + vec![], + ) + .await; + assert_eq!(file.status(), StatusCode::OK); + assert_eq!(file.headers()["content-type"], "application/octet-stream"); + assert_eq!(file.headers()["x-content-type-options"], "nosniff"); + let disposition = file.headers()["content-disposition"] + .to_str() + .unwrap() + .to_owned(); + assert!( + disposition.starts_with("attachment;") && disposition.ends_with(".ttml"), + "{disposition}" + ); + assert_eq!(body_text(file).await, ttml_body); + let txt_file = send( + Method::GET, + format!("{lyrics_uri}/file?format=txt"), + &owner_token, + vec![], + ) + .await; + assert_eq!(body_text(txt_file).await, "plain words\n"); + + // A listener reads, and may not write: refused like a track it cannot see. + let read = send(Method::GET, lyrics_uri.clone(), &listener_token, vec![]).await; + assert_eq!(read.status(), StatusCode::OK); + for (method, uri) in [ + (Method::PUT, format!("{lyrics_uri}/lrc")), + (Method::DELETE, format!("{lyrics_uri}/lrc")), + ] { + let refused = send(method, uri, &listener_token, b"[00:01.00]x".to_vec()).await; + assert_eq!(refused.status(), StatusCode::NOT_FOUND); + } + + // Nothing usable is refused, and so is a format the route does not know. + let empty = send( + Method::PUT, + format!("{lyrics_uri}/txt"), + &owner_token, + b" \n".to_vec(), + ) + .await; + assert_eq!(empty.status(), StatusCode::UNPROCESSABLE_ENTITY); + let unknown = send( + Method::PUT, + format!("{lyrics_uri}/srt"), + &owner_token, + b"1".to_vec(), + ) + .await; + assert_eq!(unknown.status(), StatusCode::NOT_FOUND); + // More than a mebibyte is refused before it is read. + let large = send( + Method::PUT, + format!("{lyrics_uri}/txt"), + &owner_token, + // One byte past the cap: the route's own ceiling answers it, not the + // service after the body has been read. + vec![b'a'; 1024 * 1024 + 1], + ) + .await; + assert_eq!(large.status(), StatusCode::PAYLOAD_TOO_LARGE); + // And a sheet well past the router's 16 KiB default is taken: the route + // carries its own ceiling. + let long = send( + Method::PUT, + format!("{lyrics_uri}/txt"), + &owner_token, + "a line of a long song +" + .repeat(10_000) + .into_bytes(), + ) + .await; + assert_eq!(long.status(), StatusCode::OK); + + // Deleting an upload falls back; deleting it twice finds nothing. + for format in ["ttml", "lrc", "txt"] { + let removed = send( + Method::DELETE, + format!("{lyrics_uri}/{format}"), + &owner_token, + vec![], + ) + .await; + assert_eq!(removed.status(), StatusCode::NO_CONTENT, "{format}"); + } + let again = send( + Method::DELETE, + format!("{lyrics_uri}/txt"), + &owner_token, + vec![], + ) + .await; + assert_eq!(again.status(), StatusCode::NOT_FOUND); + let after = json_body(send(Method::GET, lyrics_uri.clone(), &owner_token, vec![]).await).await; + assert_eq!(sources(&after), ["sidecar_lrc"]); + // A scanned synced sheet downloads as LRC. + let scanned = send( + Method::GET, + format!("{lyrics_uri}/file"), + &owner_token, + vec![], + ) + .await; + assert_eq!(body_text(scanned).await, "[00:01.00]from the sidecar\n"); + + // The Subsonic service the façade reads never sees an upload, and its + // sheets carry no `source`: its contract is frozen. + send( + Method::PUT, + format!("{lyrics_uri}/ttml"), + &owner_token, + ttml_body.as_bytes().to_vec(), + ) + .await; + let facade = state.services.lyrics(owner, track).await.unwrap(); + assert_eq!(facade.structured_lyrics.len(), 1); + assert!(facade.structured_lyrics[0].source.is_none()); + assert!(serde_json::to_value(&facade.structured_lyrics[0]) + .unwrap() + .get("source") + .is_none()); +} diff --git a/webapp/e2e/studio-nocturne.spec.ts b/webapp/e2e/studio-nocturne.spec.ts index febe65bc..316cbe8c 100644 --- a/webapp/e2e/studio-nocturne.spec.ts +++ b/webapp/e2e/studio-nocturne.spec.ts @@ -1484,6 +1484,128 @@ test("opens a track's properties from its context menu", async ({ page }) => { await expect(menu).toHaveCount(0); }); +/** + * The lyrics in a track's properties (RFC-012 decision 6). + * + * The sheets come in the server's order, the first marked as the one shown, + * each naming its source. An owner uploads a sheet as the bytes it is and + * takes an upload back; anyone downloads the shown sheet, saved under the + * name the server gives in `Content-Disposition`. + */ +test("uploads, removes and downloads lyrics from a track's properties", async ({ + page, +}) => { + const sheet = (source: string, synced: boolean, value: string) => ({ + displayArtist: "Björk", + displayTitle: "Army Of Me", + lang: "eng", + synced, + line: [{ start: synced ? 1000 : undefined, value }], + source, + }); + let sheets = [sheet("sidecar_lrc", true, "from the sidecar")]; + const sent: Array<{ method: string; path: string; body: string }> = []; + await page.route("**/api/v2/songs?*", (route) => + route.fulfill({ json: [correctable.song] }), + ); + await page.route("**/api/v2/tracks/song-9/lyrics", (route) => + route.fulfill({ + json: { trackId: "song-9", structuredLyrics: sheets }, + }), + ); + await page.route("**/api/v2/tracks/song-9/lyrics/ttml", (route) => { + const request = route.request(); + sent.push({ + method: request.method(), + path: "ttml", + body: request.postData() ?? "", + }); + if (request.method() === "PUT") { + sheets = [sheet("upload_ttml", true, "bonjour"), ...sheets]; + return route.fulfill({ + json: { trackId: "song-9", structuredLyrics: sheets }, + }); + } + sheets = sheets.filter((entry) => entry.source !== "upload_ttml"); + return route.fulfill({ status: 204, body: "" }); + }); + await page.route("**/api/v2/tracks/song-9/lyrics/file", (route) => + route.fulfill({ + body: "[00:01.00]from the sidecar\n", + headers: { + "content-type": "application/octet-stream", + "content-disposition": + "attachment; filename=\"Bj_rk - Army Of Me.lrc\"; filename*=UTF-8''Bj%C3%B6rk%20-%20Army%20Of%20Me.lrc", + }, + }), + ); + await page.goto("/songs"); + await page.evaluate(() => { + const seen = window as unknown as { __saved: string[] }; + seen.__saved = []; + HTMLAnchorElement.prototype.click = function (this: HTMLAnchorElement) { + seen.__saved.push(this.download); + }; + }); + await page + .locator(".songs tbody tr") + .first() + .locator("button.song-title") + .focus(); + await page.keyboard.press("Shift+F10"); + await page + .getByRole("menu", { name: "Actions for Army Of Me" }) + .getByRole("menuitem", { name: "Properties" }) + .click(); + const panel = page.getByRole("dialog", { name: "Army Of Me" }); + const lyrics = panel.locator(".lyrics-section"); + await expect(lyrics).toContainText("Shown"); + await expect(lyrics).toContainText("LRC file beside the track"); + + // An upload goes as the file's own bytes, and leads once it is in. + await panel.locator('input[type="file"]').setInputFiles({ + name: "Army Of Me.ttml", + mimeType: "application/ttml+xml", + buffer: Buffer.from(''), + }); + await expect(lyrics).toContainText("Uploaded TTML"); + expect(sent).toEqual([ + { + method: "PUT", + path: "ttml", + body: '', + }, + ]); + await expect(lyrics.locator(".property-row").first()).toContainText( + "Uploaded TTML", + ); + + // A file that is not a sheet is refused before anything is sent. + await panel.locator('input[type="file"]').setInputFiles({ + name: "notes.srt", + mimeType: "text/plain", + buffer: Buffer.from("1"), + }); + await expect(lyrics.getByRole("alert")).toHaveText( + "Choose a .lrc, .txt or .ttml file.", + ); + expect(sent).toHaveLength(1); + + // Taking the upload back leaves the scanned sheet shown again. + await lyrics.getByRole("button", { name: "Remove the TTML" }).click(); + await expect(lyrics).not.toContainText("Uploaded TTML"); + expect(sent.at(-1)?.method).toBe("DELETE"); + + // The download is saved under the server's name, accents and all. + await lyrics.getByRole("button", { name: "Download lyrics" }).click(); + await expect + .poll(() => + page.evaluate(() => (window as unknown as { __saved: string[] }).__saved), + ) + .toEqual(["Björk - Army Of Me.lrc"]); + await expectNoWcagViolations(page); +}); + /** * Download is the stream ticket with `?download=1` (RFC-012 decision 5). * diff --git a/webapp/src/api/index.ts b/webapp/src/api/index.ts index 82220287..af53150e 100644 --- a/webapp/src/api/index.ts +++ b/webapp/src/api/index.ts @@ -131,8 +131,10 @@ export { createShare, deleteShare, listShares } from "./shares"; export type { StreamUrl } from "./stream"; export { streamUrl } from "./stream"; export type { + LyricsFormat, LyricsLine, LyricsList, + LyricsSource, SongCredits, StructuredLyrics, TrackCorrectionPatch, @@ -143,10 +145,13 @@ export type { } from "./tracks"; export { correctTrack, + deleteLyricsUpload, + fetchLyricsFile, getLyrics, getTrackCredits, getTrackOverrides, getTrackProperties, + uploadLyrics, } from "./tracks"; export type { CommittedUpload, diff --git a/webapp/src/api/tracks.ts b/webapp/src/api/tracks.ts index a1a4b88c..939254ff 100644 --- a/webapp/src/api/tracks.ts +++ b/webapp/src/api/tracks.ts @@ -1,5 +1,5 @@ import type { Song } from "./catalog"; -import { call } from "./client"; +import { call, request } from "./client"; /** * A track with the credits the catalogue answers for it. The server sends these @@ -103,8 +103,24 @@ export type StructuredLyrics = { lang: string; synced: boolean; line: LyricsLine[]; + /** + * Where the sheet came from (RFC-012 decision 6). Sent by the native route + * only, which lists every sheet in the order a reader should choose by. + */ + source?: LyricsSource; }; +export type LyricsSource = + | "upload_ttml" + | "upload_lrc" + | "upload_txt" + | "sidecar_lrc" + | "sidecar_text" + | "embedded"; + +/** The formats a sheet can be uploaded in. */ +export type LyricsFormat = "lrc" | "txt" | "ttml"; + /** * What `GET /api/v2/tracks/{id}/lyrics` answers. * @@ -121,3 +137,46 @@ export type LyricsList = { export const getLyrics = (trackId: string) => call(`/api/v2/tracks/${trackId}/lyrics`); + +/** + * Upload a sheet as the bytes it is, in whatever encoding it was saved; the + * server decodes it. Answers the track's lyrics as they now read. + */ +export const uploadLyrics = ( + trackId: string, + format: LyricsFormat, + file: Blob, +) => + call(`/api/v2/tracks/${trackId}/lyrics/${format}`, { + method: "PUT", + body: file, + headers: { "content-type": "application/octet-stream" }, + }); + +export const deleteLyricsUpload = async ( + trackId: string, + format: LyricsFormat, +): Promise => { + await request(`/api/v2/tracks/${trackId}/lyrics/${format}`, { + method: "DELETE", + }); +}; + +/** + * The track's lyrics as a file: the sheet the server puts first, or one upload + * by name. Fetched with the session, since a navigation could not carry it, + * and named by the server's `Content-Disposition`. + */ +export async function fetchLyricsFile( + trackId: string, + format?: LyricsFormat, +): Promise<{ blob: Blob; name: string | null }> { + const query = format ? `?format=${format}` : ""; + const response = await request( + `/api/v2/tracks/${trackId}/lyrics/file${query}`, + ); + return { + blob: await response.blob(), + name: response.headers.get("content-disposition"), + }; +} diff --git a/webapp/src/components/lyrics-section.tsx b/webapp/src/components/lyrics-section.tsx new file mode 100644 index 00000000..d9d790d7 --- /dev/null +++ b/webapp/src/components/lyrics-section.tsx @@ -0,0 +1,215 @@ +import { useQueryClient } from "@tanstack/react-query"; +import { useRef, useState } from "react"; + +import { + ApiError, + deleteLyricsUpload, + fetchLyricsFile, + type LyricsFormat, + type LyricsList, + type LyricsSource, + type Song, + uploadLyrics, +} from "../api"; +import { lyricsQuery } from "../api/queries"; +import { fileNameFrom, lyricsFormatOf, saveBlob } from "../lib/download"; +import { type TranslationKey, useI18n } from "../state/i18n"; +import { mayCorrectTracks, useLibraryScope } from "../state/library-scope"; +import { Icon } from "./icons"; + +const SOURCE_LABELS: Record = { + upload_ttml: "lyrics.source.uploadTtml", + upload_lrc: "lyrics.source.uploadLrc", + upload_txt: "lyrics.source.uploadTxt", + sidecar_lrc: "lyrics.source.sidecarLrc", + sidecar_text: "lyrics.source.sidecarText", + embedded: "lyrics.source.embedded", +}; + +const UPLOADED: Partial> = { + upload_ttml: "ttml", + upload_lrc: "lrc", + upload_txt: "txt", +}; + +/** + * The lyrics of a track in its properties panel (RFC-012 decision 6). + * + * Every sheet the track has, in the order the server chose — the first is the + * one every reader shows — each with where it came from. Anyone may download + * the first; owners and managers may also upload a `.lrc`, `.txt` or `.ttml` + * and take an upload back. Nothing here touches the audio file. + */ +export function LyricsSection({ + song, + list, + known, +}: { + song: Song; + list: LyricsList | undefined; + /** Whether the lyrics have been answered, so "none" is not said early. */ + known: boolean; +}) { + const { t } = useI18n(); + const client = useQueryClient(); + const { libraries } = useLibraryScope(); + const mayUpload = mayCorrectTracks( + libraries.find((library) => library.id === song.library_id), + ); + const picker = useRef(null); + const [busy, setBusy] = useState(false); + const [problem, setProblem] = useState(null); + const sheets = list?.structuredLyrics ?? []; + + // The new list is the server's answer to the write itself; setting it first + // keeps the panel from flashing the old one while everything else re-reads. + async function settle(next?: LyricsList) { + if (next) client.setQueryData(lyricsQuery(song.id).queryKey, next); + await client.invalidateQueries({ queryKey: lyricsQuery(song.id).queryKey }); + } + + async function upload(file: File) { + const format = lyricsFormatOf(file.name); + if (!format) { + setProblem(t("lyrics.unsupported")); + return; + } + setBusy(true); + setProblem(null); + try { + await settle(await uploadLyrics(song.id, format, file)); + } catch (error) { + setProblem( + t( + error instanceof ApiError && error.status === 422 + ? "lyrics.empty" + : error instanceof ApiError && error.status === 413 + ? "lyrics.tooLarge" + : "lyrics.failed", + ), + ); + } finally { + setBusy(false); + } + } + + async function remove(format: LyricsFormat) { + setBusy(true); + setProblem(null); + try { + await deleteLyricsUpload(song.id, format); + await settle(); + } catch { + setProblem(t("lyrics.failed")); + } finally { + setBusy(false); + } + } + + async function download() { + setProblem(null); + try { + const { blob, name } = await fetchLyricsFile(song.id); + saveBlob(blob, fileNameFrom(name) ?? `${song.title}.txt`); + } catch { + setProblem(t("lyrics.downloadFailed")); + } + } + + return ( +
+

{t("properties.lyrics")}

+
+ {sheets.map((sheet, index) => { + const uploaded = sheet.source ? UPLOADED[sheet.source] : undefined; + return ( +
+
+ {index === 0 + ? t("lyrics.shown") + : t("properties.lyricsSheet", { number: index + 1 })} +
+
+ + {t(sheet.synced ? "properties.synced" : "properties.plain", { + lang: sheet.lang, + })} + + {sheet.source ? ( + + {t(SOURCE_LABELS[sheet.source])} + + ) : null} + {uploaded && mayUpload ? ( + + ) : null} +
+
+ ); + })} + {known && sheets.length === 0 ? ( +
+
{t("properties.lyricsSheets")}
+
{t("properties.noLyrics")}
+
+ ) : null} +
+
+ {sheets.length > 0 ? ( + + ) : null} + {mayUpload ? ( + <> + + { + const file = event.target.files?.[0]; + // Cleared, so choosing the same file again is a change too. + event.target.value = ""; + if (file) void upload(file); + }} + /> + + ) : null} +
+ {problem ? ( +

+ {problem} +

+ ) : null} +
+ ); +} diff --git a/webapp/src/components/track-properties.tsx b/webapp/src/components/track-properties.tsx index a437f146..5c532d4f 100644 --- a/webapp/src/components/track-properties.tsx +++ b/webapp/src/components/track-properties.tsx @@ -6,6 +6,7 @@ import { lyricsQuery, trackPropertiesQuery } from "../api/queries"; import { useI18n } from "../state/i18n"; import { Artwork } from "./artwork"; import { Icon } from "./icons"; +import { LyricsSection } from "./lyrics-section"; /** One row of a list, left out entirely when it has nothing to say. */ type Row = { label: string; value: ReactNode; corrected?: string | null }; @@ -205,25 +206,6 @@ export function TrackProperties({ { label: t("properties.bpm"), value: known(track?.bpm) }, ]; - const sheets = lyrics?.structuredLyrics ?? []; - // "None" is an answer, so it waits for one: while the lyrics are still - // being asked for, the section says nothing rather than something false. - const words: Row[] = sheets.length - ? sheets.map((sheet, index) => ({ - label: t("properties.lyricsSheet", { number: index + 1 }), - value: t(sheet.synced ? "properties.synced" : "properties.plain", { - lang: sheet.lang, - }), - })) - : lyricsKnown - ? [ - { - label: t("properties.lyricsSheets"), - value: t("properties.noLyrics"), - }, - ] - : []; - return ( // biome-ignore lint/a11y/useKeyWithClickEvents: Escape closes a natively
-
+
); } diff --git a/webapp/src/lib/download.test.ts b/webapp/src/lib/download.test.ts index 120e0315..b82b14b3 100644 --- a/webapp/src/lib/download.test.ts +++ b/webapp/src/lib/download.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; -import { downloadUrl } from "./download"; +import { downloadUrl, fileNameFrom, lyricsFormatOf } from "./download"; describe("a download's URL", () => { it("asks the ticket for the file", () => { @@ -15,3 +15,37 @@ describe("a download's URL", () => { ); }); }); + +describe("a name from Content-Disposition", () => { + it("prefers the UTF-8 form", () => { + expect( + fileNameFrom( + "attachment; filename=\"Bj_rk - Joga.lrc\"; filename*=UTF-8''Bj%C3%B6rk%20-%20J%C3%B3ga.lrc", + ), + ).toBe("Björk - Jóga.lrc"); + }); + + it("falls back to the ASCII form", () => { + expect(fileNameFrom('attachment; filename="plain.txt"')).toBe("plain.txt"); + expect( + fileNameFrom( + "attachment; filename=\"ok.txt\"; filename*=UTF-8''%E0%A4%A", + ), + ).toBe("ok.txt"); + }); + + it("has nothing to say without a header", () => { + expect(fileNameFrom(null)).toBeNull(); + expect(fileNameFrom("attachment")).toBeNull(); + }); +}); + +describe("a lyrics file's format", () => { + it("is read off its extension, and nothing else is one", () => { + expect(lyricsFormatOf("Song.LRC")).toBe("lrc"); + expect(lyricsFormatOf("a.b.ttml")).toBe("ttml"); + expect(lyricsFormatOf("notes.txt")).toBe("txt"); + expect(lyricsFormatOf("song.srt")).toBeNull(); + expect(lyricsFormatOf("lrc")).toBeNull(); + }); +}); diff --git a/webapp/src/lib/download.ts b/webapp/src/lib/download.ts index 92b4dbf3..75e739b7 100644 --- a/webapp/src/lib/download.ts +++ b/webapp/src/lib/download.ts @@ -6,10 +6,10 @@ * file in `Content-Disposition`; `download` without a value leaves that name * alone, and asks the browser to save rather than open whatever it receives. */ -export function saveFrom(url: string): void { +export function saveFrom(url: string, name = ""): void { const link = document.createElement("a"); link.href = url; - link.download = ""; + link.download = name; link.rel = "noopener"; link.style.display = "none"; document.body.append(link); @@ -21,3 +21,44 @@ export function saveFrom(url: string): void { export function downloadUrl(ticketUrl: string): string { return `${ticketUrl}${ticketUrl.includes("?") ? "&" : "?"}download=1`; } + +/** + * The name a `Content-Disposition` gives, preferring `filename*`. + * + * The server sends both forms (RFC 6266): an ASCII fallback in `filename` + * and the real name, percent-encoded UTF-8, in `filename*`. A browser saving + * from a navigation reads them itself; a blob saved from script has to be + * told, or it is saved as a random identifier. + */ +export function fileNameFrom(disposition: string | null): string | null { + if (!disposition) return null; + const extended = /filename\*\s*=\s*UTF-8''([^;]+)/i.exec(disposition); + if (extended) { + try { + return decodeURIComponent(extended[1].trim()); + } catch { + // A malformed escape falls back to the plain form below. + } + } + const plain = /filename\s*=\s*"([^"]*)"/i.exec(disposition); + return plain ? plain[1] : null; +} + +/** Save a blob under a name, through a short-lived object URL. */ +export function saveBlob(blob: Blob, name: string): void { + const url = URL.createObjectURL(blob); + saveFrom(url, name); + // Revoked on the next turn, once the click has handed the URL over. + setTimeout(() => URL.revokeObjectURL(url), 0); +} + +/** The lyrics format a file's name announces, or `null` for any other. */ +export function lyricsFormatOf(name: string): "lrc" | "txt" | "ttml" | null { + const dot = name.lastIndexOf("."); + // No dot, no extension: a file called `lrc` announces nothing. + if (dot < 0) return null; + const extension = name.slice(dot + 1).toLowerCase(); + return extension === "lrc" || extension === "txt" || extension === "ttml" + ? extension + : null; +} diff --git a/webapp/src/state/i18n.tsx b/webapp/src/state/i18n.tsx index 79b0f6d8..1be51a87 100644 --- a/webapp/src/state/i18n.tsx +++ b/webapp/src/state/i18n.tsx @@ -156,6 +156,21 @@ const en = { "properties.synced": "Synced, {lang}", "properties.plain": "Plain text, {lang}", "properties.noLyrics": "None", + "lyrics.source.uploadTtml": "Uploaded TTML", + "lyrics.source.uploadLrc": "Uploaded LRC", + "lyrics.source.uploadTxt": "Uploaded text", + "lyrics.source.sidecarLrc": "LRC file beside the track", + "lyrics.source.sidecarText": "Text file beside the track", + "lyrics.source.embedded": "Embedded in the file", + "lyrics.shown": "Shown", + "lyrics.download": "Download lyrics", + "lyrics.upload": "Upload lyrics…", + "lyrics.remove": "Remove the {format}", + "lyrics.unsupported": "Choose a .lrc, .txt or .ttml file.", + "lyrics.empty": "That file holds no lyrics.", + "lyrics.tooLarge": "That file is larger than 1 MB.", + "lyrics.failed": "The lyrics could not be saved. Try again.", + "lyrics.downloadFailed": "The lyrics could not be downloaded. Try again.", "properties.lyricsSheets": "Sheets", "properties.kbps": "{value} kbps", "properties.khz": "{value} kHz", @@ -768,6 +783,22 @@ const fr: Record = { "properties.synced": "Synchronisées, {lang}", "properties.plain": "Texte brut, {lang}", "properties.noLyrics": "Aucune", + "lyrics.source.uploadTtml": "TTML envoyé", + "lyrics.source.uploadLrc": "LRC envoyé", + "lyrics.source.uploadTxt": "Texte envoyé", + "lyrics.source.sidecarLrc": "Fichier LRC à côté du titre", + "lyrics.source.sidecarText": "Fichier texte à côté du titre", + "lyrics.source.embedded": "Intégrées au fichier", + "lyrics.shown": "Affichées", + "lyrics.download": "Télécharger les paroles", + "lyrics.upload": "Envoyer des paroles…", + "lyrics.remove": "Retirer le {format}", + "lyrics.unsupported": "Choisissez un fichier .lrc, .txt ou .ttml.", + "lyrics.empty": "Ce fichier ne contient pas de paroles.", + "lyrics.tooLarge": "Ce fichier dépasse 1 Mo.", + "lyrics.failed": "Les paroles n’ont pas pu être enregistrées. Réessayez.", + "lyrics.downloadFailed": + "Les paroles n’ont pas pu être téléchargées. Réessayez.", "properties.lyricsSheets": "Feuilles", "properties.kbps": "{value} kbit/s", "properties.khz": "{value} kHz", diff --git a/webapp/src/styles/styles.css b/webapp/src/styles/styles.css index 5d2f3c03..28b9e921 100644 --- a/webapp/src/styles/styles.css +++ b/webapp/src/styles/styles.css @@ -2835,3 +2835,28 @@ button.danger { background: transparent; color: var(--text); } + +/* The lyrics in a track's properties: each sheet with its source, and what + may be done with them under the list. */ +.lyrics-actions { + display: flex; + flex-wrap: wrap; + gap: var(--space-2); + margin-top: var(--space-3); +} + +.lyrics-action { + display: inline-flex; + align-items: center; + gap: var(--space-2); +} + +.lyrics-remove { + justify-self: start; + min-height: 1.75rem; + padding: 0 var(--space-2); + border: 0; + background: transparent; + color: var(--danger); + font-size: var(--text-xs); +} From c6d8bdd5a9611ec5a25431d360501b7b4012d71e Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Wed, 21 Oct 2026 08:11:39 +0200 Subject: [PATCH 2/5] fix: refuse lyrics uploads to an unavailable track The write checked membership and role but not availability, while every read asks for it: a sheet stored for a missing track answered 404 yet went in. The test counts the row, since the 404 alone came from the read after the write. Signed-off-by: InstaZDLL --- src/services/lyrics_uploads.rs | 4 +++- tests/native_api.rs | 27 +++++++++++++++++++++++++++ 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/src/services/lyrics_uploads.rs b/src/services/lyrics_uploads.rs index 04db88b7..6cbdd5a6 100644 --- a/src/services/lyrics_uploads.rs +++ b/src/services/lyrics_uploads.rs @@ -290,9 +290,11 @@ impl DomainServices { track_id: Uuid, ) -> Result { let row = sqlx::query( + // Available, as the read requires: lyrics stored for a track + // nobody can read would answer 200 here and 404 there. "SELECT t.library_id, m.role FROM track t \ JOIN library_member m ON m.library_id=t.library_id \ - WHERE t.id=? AND m.user_id=?", + WHERE t.id=? AND m.user_id=? AND t.is_available=1", ) .bind(track_id.to_string()) .bind(user_id.to_string()) diff --git a/tests/native_api.rs b/tests/native_api.rs index 1899a3cb..42b1e39f 100644 --- a/tests/native_api.rs +++ b/tests/native_api.rs @@ -4312,4 +4312,31 @@ async fn uploaded_lyrics_lead_the_order_download_as_they_came_and_stay_off_subso .unwrap() .get("source") .is_none()); + + // A track whose file is gone takes no upload: the read refuses it, and a + // write the read would refuse is no write at all. + sqlx::query("UPDATE track SET is_available = 0 WHERE id = ?") + .bind(track.to_string()) + .execute(state.db.pool()) + .await + .unwrap(); + let gone = send( + Method::PUT, + format!("{lyrics_uri}/lrc"), + &owner_token, + b"[00:01.00]too late".to_vec(), + ) + .await; + assert_eq!(gone.status(), StatusCode::NOT_FOUND); + // The answer alone proves nothing: the lyrics read after the write says + // 404 for this track whether or not the row went in. + let stored: i64 = sqlx::query_scalar( + "SELECT COUNT(*) FROM track_lyrics_upload WHERE track_id = ? AND content LIKE ?", + ) + .bind(track.to_string()) + .bind("%too late%") + .fetch_one(state.db.pool()) + .await + .unwrap(); + assert_eq!(stored, 0); } From 03730bca6ba4175e7f3e8e9e2f2d79fbf964349f Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Wed, 21 Oct 2026 08:11:40 +0200 Subject: [PATCH 3/5] fix: read each ttml paragraph's text once A

nested in a

made every enclosing paragraph copy the inner text again, quadratic in a document the 1 MiB cap does not bound. Only the outermost paragraphs are kept, found without walking into one. Signed-off-by: InstaZDLL --- src/lyrics.rs | 43 ++++++++++++++++++++++++++++++++++++++----- 1 file changed, 38 insertions(+), 5 deletions(-) diff --git a/src/lyrics.rs b/src/lyrics.rs index 1b6167a7..a40cad41 100644 --- a/src/lyrics.rs +++ b/src/lyrics.rs @@ -149,12 +149,21 @@ pub fn parse_ttml(content: &str) -> Option { .filter(|lang| !lang.is_empty()) .unwrap_or(UNKNOWN_LANG) .to_owned(); + // Outermost paragraphs only, found without walking into one: a `

` + // nested in a `

` is not TTML, and reading each paragraph's whole + // subtree would copy the text once per level of nesting — quadratic in a + // document the size cap alone does not bound. Each text node is read once. + let mut paragraphs = Vec::new(); + let mut pending: Vec<_> = root.children().rev().collect(); + while let Some(node) = pending.pop() { + if node.is_element() && node.tag_name().name() == "p" { + paragraphs.push(node); + } else { + pending.extend(node.children().rev()); + } + } let mut lines = Vec::new(); - for (order, paragraph) in root - .descendants() - .filter(|node| node.is_element() && node.tag_name().name() == "p") - .enumerate() - { + for (order, paragraph) in paragraphs.into_iter().enumerate() { let Some(start) = paragraph.attribute("begin").and_then(ttml_time) else { continue; }; @@ -356,6 +365,30 @@ fn parse_timestamp(value: &str) -> Option { mod tests { use super::*; + #[test] + fn a_paragraph_nested_in_a_paragraph_reads_once() { + let parsed = parse_ttml( + r#"

+

outer

inner

deepest

+

next

+
"#, + ) + .unwrap(); + assert_eq!( + parsed.lines, + vec![ + LyricsLine { + start: Some(1000), + value: "outer inner deepest".into(), + }, + LyricsLine { + start: Some(4000), + value: "next".into(), + }, + ] + ); + } + #[test] fn parses_and_orders_lrc_timestamps() { assert_eq!( From 979bb44fc8f5801b6fc88008c731514ee0cceb1c Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Wed, 21 Oct 2026 08:21:04 +0200 Subject: [PATCH 4/5] fix: read a ttml sheet's language from its root or body, and break words at br A xml:lang anywhere in the document named the whole sheet, so a span or a head element could. A
joined the words on either side of it. Both tests fall against the previous parser. Signed-off-by: InstaZDLL --- src/lyrics.rs | 50 +++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 45 insertions(+), 5 deletions(-) diff --git a/src/lyrics.rs b/src/lyrics.rs index a40cad41..cbc55fe2 100644 --- a/src/lyrics.rs +++ b/src/lyrics.rs @@ -141,9 +141,13 @@ pub fn parse_ttml(content: &str) -> Option { if root.tag_name().name() != "tt" { return None; } - let lang = root - .descendants() - .filter(|node| node.is_element()) + // The root's, else the body's: a `xml:lang` deeper down names one + // fragment — a span, a paragraph, a head element — not the sheet. + let lang = std::iter::once(root) + .chain( + root.children() + .filter(|node| node.is_element() && node.tag_name().name() == "body"), + ) .find_map(|node| node.attribute(("http://www.w3.org/XML/1998/namespace", "lang"))) .map(str::trim) .filter(|lang| !lang.is_empty()) @@ -167,10 +171,19 @@ pub fn parse_ttml(content: &str) -> Option { let Some(start) = paragraph.attribute("begin").and_then(ttml_time) else { continue; }; + // A `
` breaks the line on screen; here it keeps the words on + // either side of it apart. let text = paragraph .descendants() - .filter(|node| node.is_text()) - .filter_map(|node| node.text()) + .filter_map(|node| { + if node.is_text() { + node.text() + } else if node.is_element() && node.tag_name().name() == "br" { + Some(" ") + } else { + None + } + }) .collect::(); let value = text.split_whitespace().collect::>().join(" "); lines.push((start, order, value)); @@ -365,6 +378,33 @@ fn parse_timestamp(value: &str) -> Option { mod tests { use super::*; + #[test] + fn a_ttml_line_break_keeps_its_words_apart() { + let parsed = parse_ttml( + r#"
+

hello
world

+
"#, + ) + .unwrap(); + assert_eq!(parsed.lines[0].value, "hello world"); + } + + #[test] + fn a_ttml_language_comes_from_the_root_or_the_body_only() { + let deep = parse_ttml( + r#" +

hi

"#, + ) + .unwrap(); + assert_eq!(deep.lang, UNKNOWN_LANG); + let body = parse_ttml( + r#" +

hi

"#, + ) + .unwrap(); + assert_eq!(body.lang, "ko"); + } + #[test] fn a_paragraph_nested_in_a_paragraph_reads_once() { let parsed = parse_ttml( From 8e38b612eac7bd0be0976847b40ca592be86975d Mon Sep 17 00:00:00 2001 From: InstaZDLL Date: Wed, 21 Oct 2026 08:32:45 +0200 Subject: [PATCH 5/5] fix: skip a blank ttml language on the root for the body's find_map kept the root's empty xml:lang and the filter after it turned it into xxx, though the body named one. Signed-off-by: InstaZDLL --- src/lyrics.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/lyrics.rs b/src/lyrics.rs index cbc55fe2..832272fc 100644 --- a/src/lyrics.rs +++ b/src/lyrics.rs @@ -148,9 +148,12 @@ pub fn parse_ttml(content: &str) -> Option { root.children() .filter(|node| node.is_element() && node.tag_name().name() == "body"), ) - .find_map(|node| node.attribute(("http://www.w3.org/XML/1998/namespace", "lang"))) - .map(str::trim) - .filter(|lang| !lang.is_empty()) + // An empty one says nothing, and the body's, nested inside it, wins. + .find_map(|node| { + node.attribute(("http://www.w3.org/XML/1998/namespace", "lang")) + .map(str::trim) + .filter(|lang| !lang.is_empty()) + }) .unwrap_or(UNKNOWN_LANG) .to_owned(); // Outermost paragraphs only, found without walking into one: a `

` @@ -403,6 +406,12 @@ mod tests { ) .unwrap(); assert_eq!(body.lang, "ko"); + let blank_root = parse_ttml( + r#" +

hi

"#, + ) + .unwrap(); + assert_eq!(blank_root.lang, "ko"); } #[test]