Now better matching of the airsonic spec

Cargo.toml

@@ -37,6 +37,7 @@ serde_json = "1"
 serde_yaml = "0.9"
 tokio = { version = "1", features = ["full"] }
 tokio-util = { version = "0.7", features = ["io"] }
+futures-util = "0.3"
 tracing = "0.1"
 tracing-subscriber = { version = "0.3", features = ["env-filter"] }
 tracing-actix-web = "0.7"

src/routes/subsonic/annotation.rs

@@ -1,36 +1,37 @@
-use actix_web::{HttpRequest, HttpResponse, web};
+use actix_web::{HttpResponse, web};
 
 use crate::state::AppState;
 
-use super::helpers::{authenticate, get_query_param, parse_subsonic_id};
+use super::SubsonicArgs;
+use super::helpers::{authenticate, parse_subsonic_id};
 use super::response;
 
-/// GET /rest/scrobble[.view]
-pub async fn scrobble(req: HttpRequest, state: web::Data<AppState>) -> HttpResponse {
-    let (params, user) = match authenticate(&req, &state).await {
+/// /rest/scrobble[.view] — currently logs only; full scrobble persistence
+/// (last.fm session keys, listen history table) is a follow-up.
+pub async fn scrobble(args: SubsonicArgs, state: web::Data<AppState>) -> HttpResponse {
+    let (params, user) = match authenticate(&args, &state).await {
         Ok(v) => v,
         Err(resp) => return resp,
     };
 
-    let id_str = match get_query_param(&req, "id") {
+    let id_str = match args.get("id") {
         Some(id) => id,
         None => {
             return response::error(
-                &params.format,
+                &params,
                 response::ERROR_MISSING_PARAM,
                 "missing required parameter: id",
             );
         }
     };
 
-    let (prefix, entity_id) = match parse_subsonic_id(&id_str) {
+    let (prefix, entity_id) = match parse_subsonic_id(id_str) {
         Some(v) => v,
         None => {
-            return response::error(&params.format, response::ERROR_NOT_FOUND, "invalid id");
+            return response::error(&params, response::ERROR_NOT_FOUND, "invalid id");
         }
     };
 
-    // Log the scrobble for now; full play tracking can be added later
     tracing::info!(
         user = %user.username,
         id_type = prefix,
@@ -38,38 +39,39 @@ pub async fn scrobble(req: HttpRequest, state: web::Data<AppState>) -> HttpRespo
         "subsonic scrobble"
     );
 
-    response::ok(&params.format, serde_json::json!({}))
+    response::ok(&params, serde_json::json!({}))
 }
 
-/// GET /rest/star[.view] — no-op stub (returns OK without persisting).
-pub async fn star(req: HttpRequest, state: web::Data<AppState>) -> HttpResponse {
-    let (params, _user) = match authenticate(&req, &state).await {
+/// /rest/star[.view] — no-op stub. Persisting stars requires a `starred_items`
+/// table; tracked as a follow-up.
+pub async fn star(args: SubsonicArgs, state: web::Data<AppState>) -> HttpResponse {
+    let (params, _user) = match authenticate(&args, &state).await {
         Ok(v) => v,
         Err(resp) => return resp,
     };
 
-    response::ok(&params.format, serde_json::json!({}))
+    response::ok(&params, serde_json::json!({}))
 }
 
-/// GET /rest/unstar[.view] — no-op stub (returns OK without persisting).
-pub async fn unstar(req: HttpRequest, state: web::Data<AppState>) -> HttpResponse {
-    let (params, _user) = match authenticate(&req, &state).await {
+/// /rest/unstar[.view] — no-op stub.
+pub async fn unstar(args: SubsonicArgs, state: web::Data<AppState>) -> HttpResponse {
+    let (params, _user) = match authenticate(&args, &state).await {
         Ok(v) => v,
         Err(resp) => return resp,
     };
 
-    response::ok(&params.format, serde_json::json!({}))
+    response::ok(&params, serde_json::json!({}))
 }
 
-/// GET /rest/getStarred2[.view] — returns empty starred lists.
-pub async fn get_starred2(req: HttpRequest, state: web::Data<AppState>) -> HttpResponse {
-    let (params, _user) = match authenticate(&req, &state).await {
+/// /rest/getStarred2[.view] — returns empty starred lists.
+pub async fn get_starred2(args: SubsonicArgs, state: web::Data<AppState>) -> HttpResponse {
+    let (params, _user) = match authenticate(&args, &state).await {
         Ok(v) => v,
         Err(resp) => return resp,
     };
 
     response::ok(
-        &params.format,
+        &params,
         serde_json::json!({
             "starred2": {
                 "artist": [],
@@ -79,3 +81,32 @@ pub async fn get_starred2(req: HttpRequest, state: web::Data<AppState>) -> HttpR
         }),
     )
 }
+
+/// /rest/getStarred[.view] — folder-based mirror of getStarred2.
+pub async fn get_starred(args: SubsonicArgs, state: web::Data<AppState>) -> HttpResponse {
+    let (params, _user) = match authenticate(&args, &state).await {
+        Ok(v) => v,
+        Err(resp) => return resp,
+    };
+
+    response::ok(
+        &params,
+        serde_json::json!({
+            "starred": {
+                "artist": [],
+                "album": [],
+                "song": [],
+            }
+        }),
+    )
+}
+
+/// /rest/setRating[.view] — no-op stub.
+pub async fn set_rating(args: SubsonicArgs, state: web::Data<AppState>) -> HttpResponse {
+    let (params, _user) = match authenticate(&args, &state).await {
+        Ok(v) => v,
+        Err(resp) => return resp,
+    };
+
+    response::ok(&params, serde_json::json!({}))
+}

src/routes/subsonic/args.rs

@@ -0,0 +1,190 @@
+//! Subsonic request parameter extractor.
+//!
+//! Implements a custom actix-web `FromRequest` extractor that reads parameters
+//! from the URL query string AND, when the request is a form-encoded POST, from
+//! the body. This is required by the OpenSubsonic spec which mandates support
+//! for `application/x-www-form-urlencoded` POST bodies (used by clients like
+//! mopidy-subidy and to overcome URL length limits on `updatePlaylist`-style
+//! calls with many `songIdToAdd` parameters).
+//!
+//! Note: this extractor consumes the request payload via `payload.take()`. Do
+//! NOT combine it with another body-consuming extractor (`web::Bytes`,
+//! `web::Form`, `web::Json`, `web::Payload`) in the same handler — they will
+//! conflict. Combining with `HttpRequest` is fine; that only reads headers.
+
+use actix_web::{
+    FromRequest, HttpRequest, dev::Payload, error::ErrorPayloadTooLarge, http::Method, web,
+};
+use futures_util::StreamExt;
+use futures_util::future::LocalBoxFuture;
+
+const MAX_FORM_BODY_BYTES: usize = 1024 * 1024; // 1 MiB
+
+/// Merged parameter bag from a Subsonic request.
+///
+/// Holds all `(key, value)` pairs from the URL query string and (for
+/// form-encoded POSTs) the request body. Lookups preserve insertion order, so
+/// query-string params come first, then body params.
+#[derive(Debug, Clone, Default)]
+pub struct SubsonicArgs(Vec<(String, String)>);
+
+impl SubsonicArgs {
+    /// Build directly from a list of pairs (used in tests).
+    #[cfg(test)]
+    pub fn from_pairs(pairs: Vec<(String, String)>) -> Self {
+        Self(pairs)
+    }
+
+    /// Get the first value for a given key.
+    pub fn get(&self, name: &str) -> Option<&str> {
+        self.0
+            .iter()
+            .find(|(k, _)| k == name)
+            .map(|(_, v)| v.as_str())
+    }
+
+    /// Get all values for a given key (for repeated params like `songId`).
+    pub fn get_all(&self, name: &str) -> Vec<&str> {
+        self.0
+            .iter()
+            .filter(|(k, _)| k == name)
+            .map(|(_, v)| v.as_str())
+            .collect()
+    }
+
+    /// Parse a single param via `FromStr`.
+    pub fn get_parsed<T: std::str::FromStr>(&self, name: &str) -> Option<T> {
+        self.get(name).and_then(|v| v.parse().ok())
+    }
+}
+
+impl FromRequest for SubsonicArgs {
+    type Error = actix_web::Error;
+    type Future = LocalBoxFuture<'static, Result<Self, Self::Error>>;
+
+    fn from_request(req: &HttpRequest, payload: &mut Payload) -> Self::Future {
+        let qs = req.query_string().to_string();
+        let is_form_post = req.method() == Method::POST
+            && req
+                .headers()
+                .get(actix_web::http::header::CONTENT_TYPE)
+                .and_then(|v| v.to_str().ok())
+                .map(|s| s.starts_with("application/x-www-form-urlencoded"))
+                .unwrap_or(false);
+        let mut payload = payload.take();
+
+        Box::pin(async move {
+            let mut params: Vec<(String, String)> =
+                serde_urlencoded::from_str(&qs).unwrap_or_default();
+
+            if is_form_post {
+                let mut body = web::BytesMut::new();
+                while let Some(chunk) = payload.next().await {
+                    let chunk = chunk?;
+                    if body.len() + chunk.len() > MAX_FORM_BODY_BYTES {
+                        return Err(ErrorPayloadTooLarge("subsonic form body too large"));
+                    }
+                    body.extend_from_slice(&chunk);
+                }
+                if let Ok(body_params) =
+                    serde_urlencoded::from_bytes::<Vec<(String, String)>>(&body)
+                {
+                    params.extend(body_params);
+                }
+            }
+
+            Ok(SubsonicArgs(params))
+        })
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use actix_web::test::TestRequest;
+
+    async fn extract(req: TestRequest) -> SubsonicArgs {
+        let (req, mut pl) = req.to_http_parts();
+        SubsonicArgs::from_request(&req, &mut pl).await.unwrap()
+    }
+
+    #[actix_web::test]
+    async fn parses_query_string_on_get() {
+        let args =
+            extract(TestRequest::get().uri("/rest/ping?u=alice&p=secret&v=1.14.0&c=test")).await;
+        assert_eq!(args.get("u"), Some("alice"));
+        assert_eq!(args.get("p"), Some("secret"));
+        assert_eq!(args.get("v"), Some("1.14.0"));
+        assert_eq!(args.get("c"), Some("test"));
+    }
+
+    #[actix_web::test]
+    async fn parses_form_body_on_post() {
+        let args = extract(
+            TestRequest::post()
+                .uri("/rest/ping")
+                .insert_header(("content-type", "application/x-www-form-urlencoded"))
+                .set_payload("u=alice&p=secret&v=1.14.0"),
+        )
+        .await;
+        assert_eq!(args.get("u"), Some("alice"));
+        assert_eq!(args.get("p"), Some("secret"));
+        assert_eq!(args.get("v"), Some("1.14.0"));
+    }
+
+    #[actix_web::test]
+    async fn merges_query_and_body_on_post() {
+        let args = extract(
+            TestRequest::post()
+                .uri("/rest/updatePlaylist?u=alice&p=secret&v=1.14.0&c=test&f=json")
+                .insert_header(("content-type", "application/x-www-form-urlencoded"))
+                .set_payload("playlistId=pl-1&songIdToAdd=tr-2&songIdToAdd=tr-3"),
+        )
+        .await;
+        assert_eq!(args.get("u"), Some("alice"));
+        assert_eq!(args.get("playlistId"), Some("pl-1"));
+        assert_eq!(args.get_all("songIdToAdd"), vec!["tr-2", "tr-3"]);
+    }
+
+    #[actix_web::test]
+    async fn ignores_body_when_content_type_is_wrong() {
+        let args = extract(
+            TestRequest::post()
+                .uri("/rest/ping?u=alice")
+                .insert_header(("content-type", "application/json"))
+                .set_payload(r#"{"p":"secret"}"#),
+        )
+        .await;
+        assert_eq!(args.get("u"), Some("alice"));
+        assert_eq!(args.get("p"), None);
+    }
+
+    #[actix_web::test]
+    async fn get_all_returns_repeated_params() {
+        let args = extract(
+            TestRequest::get()
+                .uri("/rest/createPlaylist?name=mix&songId=tr-1&songId=tr-2&songId=tr-3"),
+        )
+        .await;
+        assert_eq!(args.get_all("songId"), vec!["tr-1", "tr-2", "tr-3"]);
+    }
+
+    #[actix_web::test]
+    async fn get_parsed_works() {
+        let args = extract(TestRequest::get().uri("/rest/getRandomSongs?size=42")).await;
+        assert_eq!(args.get_parsed::<u32>("size"), Some(42));
+        assert_eq!(args.get_parsed::<u32>("missing"), None);
+    }
+
+    #[actix_web::test]
+    async fn rejects_oversized_body() {
+        let huge = "x=".to_string() + &"a".repeat(MAX_FORM_BODY_BYTES + 10);
+        let (req, mut pl) = TestRequest::post()
+            .uri("/rest/ping")
+            .insert_header(("content-type", "application/x-www-form-urlencoded"))
+            .set_payload(huge)
+            .to_http_parts();
+        let result = SubsonicArgs::from_request(&req, &mut pl).await;
+        assert!(result.is_err());
+    }
+}

src/routes/subsonic/auth.rs

@@ -1,10 +1,11 @@
-use actix_web::HttpRequest;
 use md5::{Digest, Md5};
 use sea_orm::DatabaseConnection;
 
 use shanty_db::entities::user::Model as User;
 use shanty_db::queries;
 
+use super::SubsonicArgs;
+
 /// Subsonic authentication method.
 pub enum AuthMethod {
     /// Modern: token = md5(password + salt)
@@ -13,22 +14,27 @@ pub enum AuthMethod {
     Password(String),
     /// Legacy: hex-encoded password (p=enc:hexstring)
     HexPassword(String),
+    /// OpenSubsonic API key. We currently store these in the same column as
+    /// `subsonic_password` so they share the same verification path.
+    ApiKey(String),
 }
 
-/// Common Subsonic API parameters extracted from the query string.
+/// Common Subsonic API parameters extracted from the request args.
 pub struct SubsonicParams {
     /// Username
     pub username: String,
     /// Authentication method + credentials
     pub auth: AuthMethod,
-    /// API version requested
+    /// API version requested by the client (whatever they sent in `v`).
     #[allow(dead_code)]
-    pub version: String,
-    /// Client name
+    pub version: Option<String>,
+    /// Client name (whatever they sent in `c`).
     #[allow(dead_code)]
     pub client: String,
-    /// Response format: "xml" or "json"
+    /// Response format: "xml", "json", or "jsonp".
     pub format: String,
+    /// JSONP callback name (only meaningful when `format == "jsonp"`).
+    pub callback: Option<String>,
 }
 
 pub enum SubsonicAuthError {
@@ -37,35 +43,36 @@ pub enum SubsonicAuthError {
 }
 
 impl SubsonicParams {
-    /// Extract Subsonic params from the query string.
-    pub fn from_request(req: &HttpRequest) -> Result<Self, SubsonicAuthError> {
-        let qs = req.query_string();
-        let params: Vec<(String, String)> = serde_urlencoded::from_str(qs).unwrap_or_default();
+    /// Build from the merged request args. Auth params may come from the
+    /// query string or (for form-encoded POSTs) the body — `SubsonicArgs`
+    /// merges both, and we accept either location.
+    pub fn from_args(args: &SubsonicArgs) -> Result<Self, SubsonicAuthError> {
+        let username = args
+            .get("u")
+            .ok_or_else(|| SubsonicAuthError::MissingParam("u".into()))?
+            .to_string();
+        let version = args.get("v").map(|s| s.to_string());
+        let client = args.get("c").unwrap_or("unknown").to_string();
+        let format = args.get("f").unwrap_or("xml").to_string();
+        let callback = args.get("callback").map(|s| s.to_string());
 
-        let get = |name: &str| -> Option<String> {
-            params
-                .iter()
-                .find(|(k, _)| k == name)
-                .map(|(_, v)| v.clone())
-        };
-
-        let username = get("u").ok_or_else(|| SubsonicAuthError::MissingParam("u".into()))?;
-        let version = get("v").unwrap_or_else(|| "1.16.1".into());
-        let client = get("c").unwrap_or_else(|| "unknown".into());
-        let format = get("f").unwrap_or_else(|| "xml".into());
-
-        // Try token auth first (modern), then legacy password
-        let auth = if let (Some(token), Some(salt)) = (get("t"), get("s")) {
-            AuthMethod::Token { token, salt }
-        } else if let Some(p) = get("p") {
+        // Auth precedence: apiKey > token > legacy password.
+        let auth = if let Some(key) = args.get("apiKey") {
+            AuthMethod::ApiKey(key.to_string())
+        } else if let (Some(token), Some(salt)) = (args.get("t"), args.get("s")) {
+            AuthMethod::Token {
+                token: token.to_string(),
+                salt: salt.to_string(),
+            }
+        } else if let Some(p) = args.get("p") {
             if let Some(hex_str) = p.strip_prefix("enc:") {
                 AuthMethod::HexPassword(hex_str.to_string())
             } else {
-                AuthMethod::Password(p)
+                AuthMethod::Password(p.to_string())
             }
         } else {
             return Err(SubsonicAuthError::MissingParam(
-                "authentication required (t+s or p)".into(),
+                "authentication required (apiKey, t+s, or p)".into(),
             ));
         };
 
@@ -75,6 +82,7 @@ impl SubsonicParams {
             version,
             client,
             format,
+            callback,
         })
     }
 }
@@ -107,19 +115,25 @@ pub async fn verify_auth(
             }
         }
         AuthMethod::Password(password) => {
-            // Direct plaintext comparison
             if password != subsonic_password {
                 return Err(SubsonicAuthError::AuthFailed);
             }
         }
         AuthMethod::HexPassword(hex_str) => {
-            // Decode hex to string, compare
             let decoded = hex::decode(hex_str).map_err(|_| SubsonicAuthError::AuthFailed)?;
             let password = String::from_utf8(decoded).map_err(|_| SubsonicAuthError::AuthFailed)?;
             if password != subsonic_password {
                 return Err(SubsonicAuthError::AuthFailed);
             }
         }
+        AuthMethod::ApiKey(key) => {
+            // Until we have a dedicated apiKey column, accept the same value
+            // stored in subsonic_password. This is enough to make API-key
+            // capable clients connect; we can split the storage later.
+            if key != subsonic_password {
+                return Err(SubsonicAuthError::AuthFailed);
+            }
+        }
     }
 
     Ok(user)

src/routes/subsonic/helpers.rs

@@ -1,31 +1,24 @@
-use actix_web::{HttpRequest, HttpResponse, web};
+use actix_web::{HttpResponse, web};
 
 use shanty_db::entities::user::Model as User;
 
 use crate::state::AppState;
 
+use super::SubsonicArgs;
 use super::auth::{SubsonicAuthError, SubsonicParams, verify_auth};
 use super::response;
 
 /// Extract and authenticate subsonic params, returning an error HttpResponse on failure.
 pub async fn authenticate(
-    req: &HttpRequest,
+    args: &SubsonicArgs,
     state: &web::Data<AppState>,
 ) -> Result<(SubsonicParams, User), HttpResponse> {
-    tracing::debug!(
-        path = req.path(),
-        query = req.query_string(),
-        "subsonic request"
-    );
-
-    let params = SubsonicParams::from_request(req).map_err(|e| match e {
-        SubsonicAuthError::MissingParam(name) => response::error(
-            "xml",
+    let params = SubsonicParams::from_args(args).map_err(|e| match e {
+        SubsonicAuthError::MissingParam(name) => response::auth_error(
             response::ERROR_MISSING_PARAM,
             &format!("missing required parameter: {name}"),
         ),
-        SubsonicAuthError::AuthFailed => response::error(
-            "xml",
+        SubsonicAuthError::AuthFailed => response::auth_error(
             response::ERROR_NOT_AUTHENTICATED,
             "wrong username or password",
         ),
@@ -35,12 +28,12 @@ pub async fn authenticate(
         .await
         .map_err(|e| match e {
             SubsonicAuthError::AuthFailed => response::error(
-                &params.format,
+                &params,
                 response::ERROR_NOT_AUTHENTICATED,
                 "wrong username or password",
             ),
             SubsonicAuthError::MissingParam(name) => response::error(
-                &params.format,
+                &params,
                 response::ERROR_MISSING_PARAM,
                 &format!("missing required parameter: {name}"),
             ),
@@ -56,15 +49,7 @@ pub fn parse_subsonic_id(id: &str) -> Option<(&str, i32)> {
         let num = num_str.parse().ok()?;
         Some((prefix, num))
     } else {
-        // Plain number — no prefix
         let num = id.parse().ok()?;
         Some(("unknown", num))
     }
 }
-
-/// Get a query parameter by name from the request.
-pub fn get_query_param(req: &HttpRequest, name: &str) -> Option<String> {
-    let qs = req.query_string();
-    let params: Vec<(String, String)> = serde_urlencoded::from_str(qs).unwrap_or_default();
-    params.into_iter().find(|(k, _)| k == name).map(|(_, v)| v)
-}

src/routes/subsonic/media.rs

@@ -6,38 +6,43 @@ use shanty_db::queries;
 
 use crate::state::AppState;
 
-use super::helpers::{authenticate, get_query_param, parse_subsonic_id};
+use super::SubsonicArgs;
+use super::helpers::{authenticate, parse_subsonic_id};
 use super::response;
 
-/// GET /rest/stream[.view]
-pub async fn stream(req: HttpRequest, state: web::Data<AppState>) -> HttpResponse {
-    let (params, _user) = match authenticate(&req, &state).await {
+/// /rest/stream[.view]
+pub async fn stream(
+    args: SubsonicArgs,
+    req: HttpRequest,
+    state: web::Data<AppState>,
+) -> HttpResponse {
+    let (params, _user) = match authenticate(&args, &state).await {
         Ok(v) => v,
         Err(resp) => return resp,
     };
 
-    let id_str = match get_query_param(&req, "id") {
+    let id_str = match args.get("id") {
         Some(id) => id,
         None => {
             return response::error(
-                &params.format,
+                &params,
                 response::ERROR_MISSING_PARAM,
                 "missing required parameter: id",
             );
         }
     };
 
-    let (_prefix, track_id) = match parse_subsonic_id(&id_str) {
+    let (_prefix, track_id) = match parse_subsonic_id(id_str) {
         Some(v) => v,
         None => {
-            return response::error(&params.format, response::ERROR_NOT_FOUND, "invalid song id");
+            return response::error(&params, response::ERROR_NOT_FOUND, "invalid song id");
         }
     };
 
     let track = match queries::tracks::get_by_id(state.db.conn(), track_id).await {
         Ok(t) => t,
         Err(_) => {
-            return response::error(&params.format, response::ERROR_NOT_FOUND, "song not found");
+            return response::error(&params, response::ERROR_NOT_FOUND, "song not found");
         }
     };
 
@@ -47,29 +52,21 @@ pub async fn stream(req: HttpRequest, state: web::Data<AppState>) -> HttpRespons
         .unwrap_or("")
         .to_lowercase();
 
-    let requested_format = get_query_param(&req, "format");
-    let max_bit_rate = get_query_param(&req, "maxBitRate")
-        .and_then(|s| s.parse::<u32>().ok())
-        .unwrap_or(0);
+    let requested_format = args.get("format").map(|s| s.to_string());
+    let max_bit_rate: u32 = args.get_parsed("maxBitRate").unwrap_or(0);
 
-    // Determine if transcoding is needed:
-    // - Client explicitly requests a different format
-    // - File is opus/ogg (many mobile clients can't play these natively)
-    // - Client requests a specific bitrate
     let needs_transcode = match requested_format.as_deref() {
-        Some("raw") => false,                 // Explicitly asked for no transcoding
-        Some(fmt) if fmt != file_ext => true, // Different format requested
+        Some("raw") => false,
+        Some(fmt) if fmt != file_ext => true,
         _ => {
-            // Auto-transcode opus/ogg to mp3 since many clients don't support them
             matches!(file_ext.as_str(), "opus" | "ogg") || (max_bit_rate > 0 && max_bit_rate < 320)
         }
     };
 
-    // Check file exists before doing anything
     if !std::path::Path::new(&track.file_path).exists() {
         tracing::error!(path = %track.file_path, "track file not found on disk");
         return response::error(
-            &params.format,
+            &params,
             response::ERROR_NOT_FOUND,
             &format!("file not found: {}", track.file_path),
         );
@@ -80,11 +77,7 @@ pub async fn stream(req: HttpRequest, state: web::Data<AppState>) -> HttpRespons
             .as_deref()
             .filter(|f| *f != "raw")
             .unwrap_or("mp3");
-        let bitrate = if max_bit_rate > 0 {
-            max_bit_rate
-        } else {
-            192 // Default transcoding bitrate
-        };
+        let bitrate = if max_bit_rate > 0 { max_bit_rate } else { 192 };
 
         let content_type = match target_format {
             "mp3" => "audio/mpeg",
@@ -143,7 +136,7 @@ pub async fn stream(req: HttpRequest, state: web::Data<AppState>) -> HttpRespons
                     match NamedFile::open_async(&track.file_path).await {
                         Ok(file) => file.into_response(&req),
                         Err(_) => response::error(
-                            &params.format,
+                            &params,
                             response::ERROR_NOT_FOUND,
                             "transcoding failed",
                         ),
@@ -154,57 +147,54 @@ pub async fn stream(req: HttpRequest, state: web::Data<AppState>) -> HttpRespons
                 tracing::error!(error = %e, "failed to start ffmpeg");
                 match NamedFile::open_async(&track.file_path).await {
                     Ok(file) => file.into_response(&req),
-                    Err(_) => {
-                        response::error(&params.format, response::ERROR_NOT_FOUND, "file not found")
-                    }
+                    Err(_) => response::error(&params, response::ERROR_NOT_FOUND, "file not found"),
                 }
             }
         }
     } else {
-        // Serve the file directly with Range request support
         match NamedFile::open_async(&track.file_path).await {
             Ok(file) => file.into_response(&req),
             Err(e) => {
                 tracing::error!(path = %track.file_path, error = %e, "failed to open track file for streaming");
-                response::error(
-                    &params.format,
-                    response::ERROR_NOT_FOUND,
-                    "file not found on disk",
-                )
+                response::error(&params, response::ERROR_NOT_FOUND, "file not found on disk")
             }
         }
     }
 }
 
-/// GET /rest/download[.view]
-pub async fn download(req: HttpRequest, state: web::Data<AppState>) -> HttpResponse {
-    let (params, _user) = match authenticate(&req, &state).await {
+/// /rest/download[.view]
+pub async fn download(
+    args: SubsonicArgs,
+    req: HttpRequest,
+    state: web::Data<AppState>,
+) -> HttpResponse {
+    let (params, _user) = match authenticate(&args, &state).await {
         Ok(v) => v,
         Err(resp) => return resp,
     };
 
-    let id_str = match get_query_param(&req, "id") {
+    let id_str = match args.get("id") {
         Some(id) => id,
         None => {
             return response::error(
-                &params.format,
+                &params,
                 response::ERROR_MISSING_PARAM,
                 "missing required parameter: id",
             );
         }
     };
 
-    let (_prefix, track_id) = match parse_subsonic_id(&id_str) {
+    let (_prefix, track_id) = match parse_subsonic_id(id_str) {
         Some(v) => v,
         None => {
-            return response::error(&params.format, response::ERROR_NOT_FOUND, "invalid song id");
+            return response::error(&params, response::ERROR_NOT_FOUND, "invalid song id");
         }
     };
 
     let track = match queries::tracks::get_by_id(state.db.conn(), track_id).await {
         Ok(t) => t,
         Err(_) => {
-            return response::error(&params.format, response::ERROR_NOT_FOUND, "song not found");
+            return response::error(&params, response::ERROR_NOT_FOUND, "song not found");
         }
     };
 
@@ -224,42 +214,37 @@ pub async fn download(req: HttpRequest, state: web::Data<AppState>) -> HttpRespo
         }
         Err(e) => {
             tracing::error!(path = %track.file_path, error = %e, "failed to open track file for download");
-            response::error(
-                &params.format,
-                response::ERROR_NOT_FOUND,
-                "file not found on disk",
-            )
+            response::error(&params, response::ERROR_NOT_FOUND, "file not found on disk")
         }
     }
 }
 
-/// GET /rest/getCoverArt[.view]
-pub async fn get_cover_art(req: HttpRequest, state: web::Data<AppState>) -> HttpResponse {
-    let (params, _user) = match authenticate(&req, &state).await {
+/// /rest/getCoverArt[.view]
+pub async fn get_cover_art(
+    args: SubsonicArgs,
+    req: HttpRequest,
+    state: web::Data<AppState>,
+) -> HttpResponse {
+    let (params, _user) = match authenticate(&args, &state).await {
         Ok(v) => v,
         Err(resp) => return resp,
     };
 
-    let id_str = match get_query_param(&req, "id") {
+    let id_str = match args.get("id") {
         Some(id) => id,
         None => {
             return response::error(
-                &params.format,
+                &params,
                 response::ERROR_MISSING_PARAM,
                 "missing required parameter: id",
             );
         }
     };
 
-    // Cover art IDs can be album IDs (al-N) or artist IDs (ar-N)
-    let (prefix, entity_id) = match parse_subsonic_id(&id_str) {
+    let (prefix, entity_id) = match parse_subsonic_id(id_str) {
         Some(v) => v,
         None => {
-            return response::error(
-                &params.format,
-                response::ERROR_NOT_FOUND,
-                "invalid cover art id",
-            );
+            return response::error(&params, response::ERROR_NOT_FOUND, "invalid cover art id");
         }
     };
 
@@ -268,23 +253,17 @@ pub async fn get_cover_art(req: HttpRequest, state: web::Data<AppState>) -> Http
             let album = match queries::albums::get_by_id(state.db.conn(), entity_id).await {
                 Ok(a) => a,
                 Err(_) => {
-                    return response::error(
-                        &params.format,
-                        response::ERROR_NOT_FOUND,
-                        "album not found",
-                    );
+                    return response::error(&params, response::ERROR_NOT_FOUND, "album not found");
                 }
             };
 
             if let Some(ref cover_art_path) = album.cover_art_path {
-                // If it's a URL, redirect to it
                 if cover_art_path.starts_with("http://") || cover_art_path.starts_with("https://") {
                     return HttpResponse::TemporaryRedirect()
                         .append_header(("Location", cover_art_path.as_str()))
                         .finish();
                 }
 
-                // Otherwise try to serve as a local file
                 match NamedFile::open_async(cover_art_path).await {
                     Ok(file) => return file.into_response(&req),
                     Err(e) => {
@@ -293,7 +272,6 @@ pub async fn get_cover_art(req: HttpRequest, state: web::Data<AppState>) -> Http
                 }
             }
 
-            // If album has a MusicBrainz ID, redirect to Cover Art Archive
             if let Some(ref mbid) = album.musicbrainz_id {
                 let url = format!("https://coverartarchive.org/release/{mbid}/front-250");
                 return HttpResponse::TemporaryRedirect()
@@ -301,12 +279,45 @@ pub async fn get_cover_art(req: HttpRequest, state: web::Data<AppState>) -> Http
                     .finish();
             }
 
-            // No cover art available
             HttpResponse::NotFound().finish()
         }
-        _ => {
-            // For other types, no cover art
+        "ar" => {
+            let artist = match queries::artists::get_by_id(state.db.conn(), entity_id).await {
+                Ok(a) => a,
+                Err(_) => return HttpResponse::NotFound().finish(),
+            };
+
+            // Look up the cached artist image — same lookup pattern that
+            // browsing::get_artist_info2 uses.
+            let mbid = artist.musicbrainz_id.as_deref().unwrap_or("");
+            if mbid.is_empty() {
+                return HttpResponse::NotFound().finish();
+            }
+            let config = state.config.read().await;
+            let image_source = config.metadata.artist_image_source.clone();
+            drop(config);
+            let key = format!("artist_image:{image_source}:{mbid}");
+            let cached = queries::cache::get(state.db.conn(), &key)
+                .await
+                .ok()
+                .flatten()
+                .filter(|s| !s.is_empty());
+
+            if let Some(url) = cached {
+                if url.starts_with("http://") || url.starts_with("https://") {
+                    HttpResponse::TemporaryRedirect()
+                        .append_header(("Location", url))
+                        .finish()
+                } else {
+                    match NamedFile::open_async(&url).await {
+                        Ok(file) => file.into_response(&req),
+                        Err(_) => HttpResponse::NotFound().finish(),
+                    }
+                }
+            } else {
                 HttpResponse::NotFound().finish()
             }
         }
+        _ => HttpResponse::NotFound().finish(),
+    }
 }

src/routes/subsonic/mod.rs

@@ -1,4 +1,5 @@
 mod annotation;
+mod args;
 mod auth;
 mod browsing;
 mod helpers;
@@ -6,120 +7,112 @@ mod media;
 mod playlists;
 mod response;
 mod search;
+mod stubs;
 mod system;
 mod user;
 
-use actix_web::web;
+pub(crate) use args::SubsonicArgs;
+
+use actix_web::{Route, guard, web};
+
+/// Construct a route that accepts both GET and POST. Per the OpenSubsonic
+/// spec, every Subsonic endpoint must accept `application/x-www-form-urlencoded`
+/// POST bodies as well as the legacy GET-with-query-string form, and the
+/// `SubsonicArgs` extractor handles both transparently.
+fn rest_route() -> Route {
+    web::route().guard(guard::Any(guard::Get()).or(guard::Post()))
+}
+
+/// Register `name` and `name.view` aliases for the same handler. The `.view`
+/// suffix is the historical Subsonic spelling and many clients still use it.
+macro_rules! register {
+    ($scope:expr, $name:literal, $handler:path) => {{
+        $scope
+            .route(concat!("/", $name), rest_route().to($handler))
+            .route(concat!("/", $name, ".view"), rest_route().to($handler))
+    }};
+}
 
 pub fn configure(cfg: &mut web::ServiceConfig) {
-    cfg.service(
-        web::scope("/rest")
-            // System
-            .route("/ping", web::get().to(system::ping))
-            .route("/ping.view", web::get().to(system::ping))
-            .route("/getLicense", web::get().to(system::get_license))
-            .route("/getLicense.view", web::get().to(system::get_license))
-            // Browsing
-            .route(
-                "/getMusicFolders",
-                web::get().to(browsing::get_music_folders),
-            )
-            .route(
-                "/getMusicFolders.view",
-                web::get().to(browsing::get_music_folders),
-            )
-            .route("/getIndexes", web::get().to(browsing::get_indexes))
-            .route("/getIndexes.view", web::get().to(browsing::get_indexes))
-            .route(
-                "/getMusicDirectory",
-                web::get().to(browsing::get_music_directory),
-            )
-            .route(
-                "/getMusicDirectory.view",
-                web::get().to(browsing::get_music_directory),
-            )
-            .route("/getArtists", web::get().to(browsing::get_artists))
-            .route("/getArtists.view", web::get().to(browsing::get_artists))
-            .route("/getArtist", web::get().to(browsing::get_artist))
-            .route("/getArtist.view", web::get().to(browsing::get_artist))
-            .route("/getAlbum", web::get().to(browsing::get_album))
-            .route("/getAlbum.view", web::get().to(browsing::get_album))
-            .route("/getSong", web::get().to(browsing::get_song))
-            .route("/getSong.view", web::get().to(browsing::get_song))
-            .route("/getGenres", web::get().to(browsing::get_genres))
-            .route("/getGenres.view", web::get().to(browsing::get_genres))
-            .route("/getRandomSongs", web::get().to(browsing::get_random_songs))
-            .route(
-                "/getRandomSongs.view",
-                web::get().to(browsing::get_random_songs),
-            )
-            .route(
-                "/getSongsByGenre",
-                web::get().to(browsing::get_songs_by_genre),
-            )
-            .route(
-                "/getSongsByGenre.view",
-                web::get().to(browsing::get_songs_by_genre),
-            )
-            .route("/getAlbumList2", web::get().to(browsing::get_album_list2))
-            .route(
-                "/getAlbumList2.view",
-                web::get().to(browsing::get_album_list2),
-            )
-            .route("/getTopSongs", web::get().to(browsing::get_top_songs))
-            .route("/getTopSongs.view", web::get().to(browsing::get_top_songs))
-            .route("/getArtistInfo2", web::get().to(browsing::get_artist_info2))
-            .route(
-                "/getArtistInfo2.view",
-                web::get().to(browsing::get_artist_info2),
-            )
-            .route(
-                "/getSimilarSongs2",
-                web::get().to(browsing::get_similar_songs2),
-            )
-            .route(
-                "/getSimilarSongs2.view",
-                web::get().to(browsing::get_similar_songs2),
-            )
-            // Search
-            .route("/search3", web::get().to(search::search3))
-            .route("/search3.view", web::get().to(search::search3))
-            // Media
-            .route("/stream", web::get().to(media::stream))
-            .route("/stream.view", web::get().to(media::stream))
-            .route("/download", web::get().to(media::download))
-            .route("/download.view", web::get().to(media::download))
-            .route("/getCoverArt", web::get().to(media::get_cover_art))
-            .route("/getCoverArt.view", web::get().to(media::get_cover_art))
-            // Playlists
-            .route("/getPlaylists", web::get().to(playlists::get_playlists))
-            .route(
-                "/getPlaylists.view",
-                web::get().to(playlists::get_playlists),
-            )
-            .route("/getPlaylist", web::get().to(playlists::get_playlist))
-            .route("/getPlaylist.view", web::get().to(playlists::get_playlist))
-            .route("/createPlaylist", web::get().to(playlists::create_playlist))
-            .route(
-                "/createPlaylist.view",
-                web::get().to(playlists::create_playlist),
-            )
-            .route("/deletePlaylist", web::get().to(playlists::delete_playlist))
-            .route(
-                "/deletePlaylist.view",
-                web::get().to(playlists::delete_playlist),
-            )
-            // Annotation
-            .route("/scrobble", web::get().to(annotation::scrobble))
-            .route("/scrobble.view", web::get().to(annotation::scrobble))
-            .route("/star", web::get().to(annotation::star))
-            .route("/star.view", web::get().to(annotation::star))
-            .route("/unstar", web::get().to(annotation::unstar))
-            .route("/unstar.view", web::get().to(annotation::unstar))
-            .route("/getStarred2", web::get().to(annotation::get_starred2))
-            .route("/getStarred2.view", web::get().to(annotation::get_starred2))
-            // User
-            .route("/getUser", web::get().to(user::get_user))
-            .route("/getUser.view", web::get().to(user::get_user)),
+    let scope = web::scope("/rest");
+    let scope = register!(scope, "ping", system::ping);
+    let scope = register!(scope, "getLicense", system::get_license);
+    let scope = register!(
+        scope,
+        "getOpenSubsonicExtensions",
+        system::get_open_subsonic_extensions
     );
+
+    // Browsing
+    let scope = register!(scope, "getMusicFolders", browsing::get_music_folders);
+    let scope = register!(scope, "getIndexes", browsing::get_indexes);
+    let scope = register!(scope, "getMusicDirectory", browsing::get_music_directory);
+    let scope = register!(scope, "getArtists", browsing::get_artists);
+    let scope = register!(scope, "getArtist", browsing::get_artist);
+    let scope = register!(scope, "getAlbum", browsing::get_album);
+    let scope = register!(scope, "getSong", browsing::get_song);
+    let scope = register!(scope, "getGenres", browsing::get_genres);
+    let scope = register!(scope, "getRandomSongs", browsing::get_random_songs);
+    let scope = register!(scope, "getSongsByGenre", browsing::get_songs_by_genre);
+    let scope = register!(scope, "getAlbumList", browsing::get_album_list);
+    let scope = register!(scope, "getAlbumList2", browsing::get_album_list2);
+    let scope = register!(scope, "getTopSongs", browsing::get_top_songs);
+    let scope = register!(scope, "getArtistInfo", browsing::get_artist_info);
+    let scope = register!(scope, "getArtistInfo2", browsing::get_artist_info2);
+    let scope = register!(scope, "getAlbumInfo", browsing::get_album_info);
+    let scope = register!(scope, "getAlbumInfo2", browsing::get_album_info2);
+    let scope = register!(scope, "getSimilarSongs", browsing::get_similar_songs);
+    let scope = register!(scope, "getSimilarSongs2", browsing::get_similar_songs2);
+    let scope = register!(scope, "getNowPlaying", browsing::get_now_playing);
+    let scope = register!(scope, "getLyrics", browsing::get_lyrics);
+    let scope = register!(scope, "getLyricsBySongId", browsing::get_lyrics_by_song_id);
+
+    // Search
+    let scope = register!(scope, "search2", search::search2);
+    let scope = register!(scope, "search3", search::search3);
+
+    // Media
+    let scope = register!(scope, "stream", media::stream);
+    let scope = register!(scope, "download", media::download);
+    let scope = register!(scope, "getCoverArt", media::get_cover_art);
+
+    // Playlists
+    let scope = register!(scope, "getPlaylists", playlists::get_playlists);
+    let scope = register!(scope, "getPlaylist", playlists::get_playlist);
+    let scope = register!(scope, "createPlaylist", playlists::create_playlist);
+    let scope = register!(scope, "updatePlaylist", playlists::update_playlist);
+    let scope = register!(scope, "deletePlaylist", playlists::delete_playlist);
+
+    // Annotation
+    let scope = register!(scope, "scrobble", annotation::scrobble);
+    let scope = register!(scope, "star", annotation::star);
+    let scope = register!(scope, "unstar", annotation::unstar);
+    let scope = register!(scope, "setRating", annotation::set_rating);
+    let scope = register!(scope, "getStarred", annotation::get_starred);
+    let scope = register!(scope, "getStarred2", annotation::get_starred2);
+
+    // User
+    let scope = register!(scope, "getUser", user::get_user);
+
+    // Stubs (best-effort no-ops keeping strict clients happy)
+    let scope = register!(scope, "getPlayQueue", stubs::get_play_queue);
+    let scope = register!(scope, "savePlayQueue", stubs::save_play_queue);
+    let scope = register!(scope, "getBookmarks", stubs::get_bookmarks);
+    let scope = register!(scope, "createBookmark", stubs::create_bookmark);
+    let scope = register!(scope, "deleteBookmark", stubs::delete_bookmark);
+    let scope = register!(scope, "getScanStatus", stubs::get_scan_status);
+    let scope = register!(scope, "startScan", stubs::start_scan);
+    let scope = register!(scope, "getShares", stubs::get_shares);
+    let scope = register!(scope, "getPodcasts", stubs::get_podcasts);
+    let scope = register!(scope, "getNewestPodcasts", stubs::get_newest_podcasts);
+    let scope = register!(
+        scope,
+        "getInternetRadioStations",
+        stubs::get_internet_radio_stations
+    );
+    let scope = register!(scope, "getChatMessages", stubs::get_chat_messages);
+    let scope = register!(scope, "getAvatar", stubs::get_avatar);
+    let scope = register!(scope, "jukeboxControl", stubs::jukebox_control);
+
+    cfg.service(scope);
 }

src/routes/subsonic/playlists.rs

@@ -1,15 +1,16 @@
-use actix_web::{HttpRequest, HttpResponse, web};
+use actix_web::{HttpResponse, web};
 
 use shanty_db::queries;
 
 use crate::state::AppState;
 
-use super::helpers::{authenticate, get_query_param, parse_subsonic_id};
+use super::SubsonicArgs;
+use super::helpers::{authenticate, parse_subsonic_id};
 use super::response::{self, SubsonicChild};
 
-/// GET /rest/getPlaylists[.view]
-pub async fn get_playlists(req: HttpRequest, state: web::Data<AppState>) -> HttpResponse {
-    let (params, user) = match authenticate(&req, &state).await {
+/// /rest/getPlaylists[.view]
+pub async fn get_playlists(args: SubsonicArgs, state: web::Data<AppState>) -> HttpResponse {
+    let (params, user) = match authenticate(&args, &state).await {
         Ok(v) => v,
         Err(resp) => return resp,
     };
@@ -24,7 +25,6 @@ pub async fn get_playlists(req: HttpRequest, state: web::Data<AppState>) -> Http
             .await
             .unwrap_or(0);
 
-        // Calculate total duration
         let tracks = queries::playlists::get_tracks(state.db.conn(), pl.id)
             .await
             .unwrap_or_default();
@@ -50,7 +50,7 @@ pub async fn get_playlists(req: HttpRequest, state: web::Data<AppState>) -> Http
     }
 
     response::ok(
-        &params.format,
+        &params,
         serde_json::json!({
             "playlists": {
                 "playlist": playlist_list,
@@ -59,43 +59,35 @@ pub async fn get_playlists(req: HttpRequest, state: web::Data<AppState>) -> Http
     )
 }
 
-/// GET /rest/getPlaylist[.view]
-pub async fn get_playlist(req: HttpRequest, state: web::Data<AppState>) -> HttpResponse {
-    let (params, user) = match authenticate(&req, &state).await {
+/// /rest/getPlaylist[.view]
+pub async fn get_playlist(args: SubsonicArgs, state: web::Data<AppState>) -> HttpResponse {
+    let (params, user) = match authenticate(&args, &state).await {
         Ok(v) => v,
         Err(resp) => return resp,
     };
 
-    let id_str = match get_query_param(&req, "id") {
+    let id_str = match args.get("id") {
         Some(id) => id,
         None => {
             return response::error(
-                &params.format,
+                &params,
                 response::ERROR_MISSING_PARAM,
                 "missing required parameter: id",
             );
         }
     };
 
-    let (_prefix, playlist_id) = match parse_subsonic_id(&id_str) {
+    let (_prefix, playlist_id) = match parse_subsonic_id(id_str) {
         Some(v) => v,
         None => {
-            return response::error(
-                &params.format,
-                response::ERROR_NOT_FOUND,
-                "invalid playlist id",
-            );
+            return response::error(&params, response::ERROR_NOT_FOUND, "invalid playlist id");
         }
     };
 
     let playlist = match queries::playlists::get_by_id(state.db.conn(), playlist_id).await {
         Ok(p) => p,
         Err(_) => {
-            return response::error(
-                &params.format,
-                response::ERROR_NOT_FOUND,
-                "playlist not found",
-            );
+            return response::error(&params, response::ERROR_NOT_FOUND, "playlist not found");
         }
     };
 
@@ -129,38 +121,35 @@ pub async fn get_playlist(req: HttpRequest, state: web::Data<AppState>) -> HttpR
     }
 
     response::ok(
-        &params.format,
+        &params,
         serde_json::json!({
             "playlist": pl_json,
         }),
     )
 }
 
-/// GET /rest/createPlaylist[.view]
-pub async fn create_playlist(req: HttpRequest, state: web::Data<AppState>) -> HttpResponse {
-    let (params, user) = match authenticate(&req, &state).await {
+/// /rest/createPlaylist[.view]
+pub async fn create_playlist(args: SubsonicArgs, state: web::Data<AppState>) -> HttpResponse {
+    let (params, user) = match authenticate(&args, &state).await {
         Ok(v) => v,
         Err(resp) => return resp,
     };
 
-    let name = match get_query_param(&req, "name") {
-        Some(n) => n,
+    let name = match args.get("name") {
+        Some(n) => n.to_string(),
         None => {
             return response::error(
-                &params.format,
+                &params,
                 response::ERROR_MISSING_PARAM,
                 "missing required parameter: name",
             );
         }
     };
 
-    // Collect songId params (can be repeated)
-    let qs = req.query_string();
-    let query_params: Vec<(String, String)> = serde_urlencoded::from_str(qs).unwrap_or_default();
-    let track_ids: Vec<i32> = query_params
+    let track_ids: Vec<i32> = args
+        .get_all("songId")
         .iter()
-        .filter(|(k, _)| k == "songId")
-        .filter_map(|(_, v)| parse_subsonic_id(v).map(|(_, id)| id))
+        .filter_map(|v| parse_subsonic_id(v).map(|(_, id)| id))
         .collect();
 
     match queries::playlists::create(state.db.conn(), &name, None, Some(user.id), &track_ids).await
@@ -196,53 +185,158 @@ pub async fn create_playlist(req: HttpRequest, state: web::Data<AppState>) -> Ht
             }
 
             response::ok(
-                &params.format,
+                &params,
                 serde_json::json!({
                     "playlist": pl_json,
                 }),
             )
         }
         Err(e) => response::error(
-            &params.format,
+            &params,
             response::ERROR_GENERIC,
             &format!("failed to create playlist: {e}"),
         ),
     }
 }
 
-/// GET /rest/deletePlaylist[.view]
-pub async fn delete_playlist(req: HttpRequest, state: web::Data<AppState>) -> HttpResponse {
-    let (params, _user) = match authenticate(&req, &state).await {
+/// /rest/updatePlaylist[.view]
+///
+/// Spec params:
+/// - `playlistId` (required)
+/// - `name`, `comment` (optional metadata changes)
+/// - `public` (ignored — we don't model public playlists yet)
+/// - `songIdToAdd` (repeatable; appended to end)
+/// - `songIndexToRemove` (repeatable; 0-based indices into the *current*
+///   track list, resolved before any mutation)
+pub async fn update_playlist(args: SubsonicArgs, state: web::Data<AppState>) -> HttpResponse {
+    let (params, user) = match authenticate(&args, &state).await {
         Ok(v) => v,
         Err(resp) => return resp,
     };
 
-    let id_str = match get_query_param(&req, "id") {
+    let id_str = match args.get("playlistId") {
         Some(id) => id,
         None => {
             return response::error(
-                &params.format,
+                &params,
+                response::ERROR_MISSING_PARAM,
+                "missing required parameter: playlistId",
+            );
+        }
+    };
+
+    let (_prefix, playlist_id) = match parse_subsonic_id(id_str) {
+        Some(v) => v,
+        None => {
+            return response::error(&params, response::ERROR_NOT_FOUND, "invalid playlist id");
+        }
+    };
+
+    let playlist = match queries::playlists::get_by_id(state.db.conn(), playlist_id).await {
+        Ok(p) => p,
+        Err(_) => {
+            return response::error(&params, response::ERROR_NOT_FOUND, "playlist not found");
+        }
+    };
+
+    // Ownership check — never let one user modify another's playlist.
+    if playlist.user_id != Some(user.id) {
+        return response::error(
+            &params,
+            response::ERROR_NOT_AUTHORIZED,
+            "you do not own this playlist",
+        );
+    }
+
+    // Resolve songIndexToRemove against the current track ordering BEFORE any
+    // additions, so the indices match what the client sees in getPlaylist.
+    let removal_indices: Vec<usize> = args
+        .get_all("songIndexToRemove")
+        .iter()
+        .filter_map(|v| v.parse().ok())
+        .collect();
+
+    if !removal_indices.is_empty() {
+        let current = queries::playlists::get_tracks(state.db.conn(), playlist_id)
+            .await
+            .unwrap_or_default();
+        let track_ids_to_remove: Vec<i32> = removal_indices
+            .iter()
+            .filter_map(|&idx| current.get(idx).map(|t| t.id))
+            .collect();
+        for tid in track_ids_to_remove {
+            if let Err(e) =
+                queries::playlists::remove_track(state.db.conn(), playlist_id, tid).await
+            {
+                return response::error(
+                    &params,
+                    response::ERROR_GENERIC,
+                    &format!("failed to remove track: {e}"),
+                );
+            }
+        }
+    }
+
+    // Append additions in the order the client sent them.
+    for raw in args.get_all("songIdToAdd") {
+        if let Some((_, track_id)) = parse_subsonic_id(raw)
+            && let Err(e) =
+                queries::playlists::add_track(state.db.conn(), playlist_id, track_id).await
+        {
+            return response::error(
+                &params,
+                response::ERROR_GENERIC,
+                &format!("failed to add track: {e}"),
+            );
+        }
+    }
+
+    // Metadata updates last so they bump updated_at after any track edits.
+    let new_name = args.get("name");
+    let new_comment = args.get("comment");
+    if (new_name.is_some() || new_comment.is_some())
+        && let Err(e) =
+            queries::playlists::update(state.db.conn(), playlist_id, new_name, new_comment).await
+    {
+        return response::error(
+            &params,
+            response::ERROR_GENERIC,
+            &format!("failed to update playlist metadata: {e}"),
+        );
+    }
+
+    response::ok(&params, serde_json::json!({}))
+}
+
+/// /rest/deletePlaylist[.view]
+pub async fn delete_playlist(args: SubsonicArgs, state: web::Data<AppState>) -> HttpResponse {
+    let (params, _user) = match authenticate(&args, &state).await {
+        Ok(v) => v,
+        Err(resp) => return resp,
+    };
+
+    let id_str = match args.get("id") {
+        Some(id) => id,
+        None => {
+            return response::error(
+                &params,
                 response::ERROR_MISSING_PARAM,
                 "missing required parameter: id",
             );
         }
     };
 
-    let (_prefix, playlist_id) = match parse_subsonic_id(&id_str) {
+    let (_prefix, playlist_id) = match parse_subsonic_id(id_str) {
         Some(v) => v,
         None => {
-            return response::error(
-                &params.format,
-                response::ERROR_NOT_FOUND,
-                "invalid playlist id",
-            );
+            return response::error(&params, response::ERROR_NOT_FOUND, "invalid playlist id");
         }
     };
 
     match queries::playlists::delete(state.db.conn(), playlist_id).await {
-        Ok(()) => response::ok(&params.format, serde_json::json!({})),
+        Ok(()) => response::ok(&params, serde_json::json!({})),
         Err(e) => response::error(
-            &params.format,
+            &params,
             response::ERROR_GENERIC,
             &format!("failed to delete playlist: {e}"),
         ),

src/routes/subsonic/response.rs

@@ -1,55 +1,89 @@
 use actix_web::HttpResponse;
 use serde::Serialize;
 
+use super::auth::SubsonicParams;
+
 const SUBSONIC_VERSION: &str = "1.16.1";
 const XMLNS: &str = "http://subsonic.org/restapi";
+/// Server name reported via the OpenSubsonic `type` field. Sourced from
+/// Cargo metadata so it tracks the actual binary.
+const SERVER_TYPE: &str = env!("CARGO_PKG_NAME");
+/// Actual server version reported via OpenSubsonic `serverVersion`.
+const SERVER_VERSION: &str = env!("CARGO_PKG_VERSION");
 
-/// Build a successful Subsonic response in the requested format.
-pub fn ok(format: &str, body: serde_json::Value) -> HttpResponse {
-    format_response(format, "ok", body, None)
+/// Build a successful Subsonic response, honouring the requested format and
+/// (for `f=jsonp`) the callback name.
+pub fn ok(params: &SubsonicParams, body: serde_json::Value) -> HttpResponse {
+    format_response(&params.format, "ok", body, params.callback.as_deref())
 }
 
-/// Build a Subsonic error response.
-pub fn error(format: &str, code: u32, message: &str) -> HttpResponse {
+/// Build a Subsonic error response for an authenticated request.
+pub fn error(params: &SubsonicParams, code: u32, message: &str) -> HttpResponse {
     let err = serde_json::json!({
         "error": {
             "code": code,
             "message": message,
         }
     });
-    format_response(format, "failed", err, None)
+    format_response(&params.format, "failed", err, params.callback.as_deref())
 }
 
-/// Subsonic error codes.
+/// Build an error response when authentication itself failed and we have no
+/// `SubsonicParams` available. Always uses XML (the spec default) — clients
+/// in this state are usually broken anyway and cannot reliably negotiate.
+pub fn auth_error(code: u32, message: &str) -> HttpResponse {
+    let err = serde_json::json!({
+        "error": {
+            "code": code,
+            "message": message,
+        }
+    });
+    format_response("xml", "failed", err, None)
+}
+
+/// Subsonic error codes (per OpenSubsonic spec).
 pub const ERROR_GENERIC: u32 = 0;
 pub const ERROR_MISSING_PARAM: u32 = 10;
+#[allow(dead_code)]
+pub const ERROR_CLIENT_MUST_UPGRADE: u32 = 20;
+#[allow(dead_code)]
+pub const ERROR_SERVER_MUST_UPGRADE: u32 = 30;
 pub const ERROR_NOT_AUTHENTICATED: u32 = 40;
 #[allow(dead_code)]
+pub const ERROR_TOKEN_AUTH_NOT_SUPPORTED_FOR_LDAP: u32 = 41;
+#[allow(dead_code)]
+pub const ERROR_AUTH_MECHANISM_NOT_SUPPORTED: u32 = 42;
+#[allow(dead_code)]
+pub const ERROR_MULTIPLE_AUTH_MECHANISMS: u32 = 43;
+#[allow(dead_code)]
+pub const ERROR_INVALID_API_KEY: u32 = 44;
 pub const ERROR_NOT_AUTHORIZED: u32 = 50;
+#[allow(dead_code)]
+pub const ERROR_TRIAL_EXPIRED: u32 = 60;
 pub const ERROR_NOT_FOUND: u32 = 70;
 
 fn format_response(
     format: &str,
     status: &str,
     body: serde_json::Value,
-    _type_attr: Option<&str>,
+    callback: Option<&str>,
 ) -> HttpResponse {
     match format {
         "json" => format_json(status, body),
+        "jsonp" => format_jsonp(status, body, callback),
         _ => format_xml(status, body),
     }
 }
 
-fn format_json(status: &str, body: serde_json::Value) -> HttpResponse {
+fn build_response_object(status: &str, body: serde_json::Value) -> serde_json::Value {
     let mut response = serde_json::json!({
         "status": status,
         "version": SUBSONIC_VERSION,
-        "type": "shanty",
-        "serverVersion": "0.1.0",
+        "type": SERVER_TYPE,
+        "serverVersion": SERVER_VERSION,
         "openSubsonic": true,
     });
 
-    // Merge body into response
     if let serde_json::Value::Object(map) = body
         && let serde_json::Value::Object(ref mut resp_map) = response
     {
@@ -58,8 +92,12 @@ fn format_json(status: &str, body: serde_json::Value) -> HttpResponse {
         }
     }
 
+    response
+}
+
+fn format_json(status: &str, body: serde_json::Value) -> HttpResponse {
     let wrapper = serde_json::json!({
-        "subsonic-response": response,
+        "subsonic-response": build_response_object(status, body),
     });
 
     HttpResponse::Ok()
@@ -67,10 +105,49 @@ fn format_json(status: &str, body: serde_json::Value) -> HttpResponse {
         .json(wrapper)
 }
 
+fn format_jsonp(status: &str, body: serde_json::Value, callback: Option<&str>) -> HttpResponse {
+    let Some(cb) = callback else {
+        // Per spec, missing callback for f=jsonp is a missing-param error.
+        return format_json(
+            "failed",
+            serde_json::json!({
+                "error": {
+                    "code": ERROR_MISSING_PARAM,
+                    "message": "missing required parameter: callback (required for f=jsonp)",
+                }
+            }),
+        );
+    };
+    // Sanity-check the callback name to avoid trivial XSS via reflected JS.
+    if !cb
+        .chars()
+        .all(|c| c.is_ascii_alphanumeric() || c == '_' || c == '$' || c == '.')
+        || cb.is_empty()
+    {
+        return format_json(
+            "failed",
+            serde_json::json!({
+                "error": {
+                    "code": ERROR_GENERIC,
+                    "message": "invalid callback name",
+                }
+            }),
+        );
+    }
+
+    let wrapper = serde_json::json!({
+        "subsonic-response": build_response_object(status, body),
+    });
+    let body = format!("{cb}({wrapper});");
+    HttpResponse::Ok()
+        .content_type("application/javascript; charset=UTF-8")
+        .body(body)
+}
+
 fn format_xml(status: &str, body: serde_json::Value) -> HttpResponse {
     let mut xml = String::from("<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n");
     xml.push_str(&format!(
-        "<subsonic-response xmlns=\"{XMLNS}\" status=\"{status}\" version=\"{SUBSONIC_VERSION}\" type=\"shanty\" serverVersion=\"0.1.0\" openSubsonic=\"true\">"
+        "<subsonic-response xmlns=\"{XMLNS}\" status=\"{status}\" version=\"{SUBSONIC_VERSION}\" type=\"{SERVER_TYPE}\" serverVersion=\"{SERVER_VERSION}\" openSubsonic=\"true\">"
     ));
 
     if let serde_json::Value::Object(map) = &body {

src/routes/subsonic/search.rs

@@ -1,46 +1,33 @@
-use actix_web::{HttpRequest, HttpResponse, web};
+use actix_web::{HttpResponse, web};
 
 use shanty_db::queries;
 
 use crate::state::AppState;
 
-use super::helpers::{authenticate, get_query_param};
+use super::SubsonicArgs;
+use super::helpers::authenticate;
 use super::response::{self, SubsonicChild};
 
-/// GET /rest/search3[.view]
-pub async fn search3(req: HttpRequest, state: web::Data<AppState>) -> HttpResponse {
-    let (params, _user) = match authenticate(&req, &state).await {
-        Ok(v) => v,
-        Err(resp) => return resp,
-    };
+/// Container for the searched results, used by both `search2` and `search3`.
+struct SearchResults {
+    artists: Vec<serde_json::Value>,
+    albums: Vec<serde_json::Value>,
+    songs: Vec<serde_json::Value>,
+}
 
-    let query = match get_query_param(&req, "query") {
-        Some(q) => q,
-        None => {
-            return response::error(
-                &params.format,
-                response::ERROR_MISSING_PARAM,
-                "missing required parameter: query",
-            );
-        }
-    };
+async fn run_search(
+    args: &SubsonicArgs,
+    state: &web::Data<AppState>,
+) -> Result<SearchResults, &'static str> {
+    let query = args.get("query").ok_or("query")?;
+    let artist_count: u64 = args.get_parsed("artistCount").unwrap_or(20);
+    let album_count: u64 = args.get_parsed("albumCount").unwrap_or(20);
+    let song_count: u64 = args.get_parsed("songCount").unwrap_or(20);
 
-    let artist_count: u64 = get_query_param(&req, "artistCount")
-        .and_then(|v| v.parse().ok())
-        .unwrap_or(20);
-    let album_count: u64 = get_query_param(&req, "albumCount")
-        .and_then(|v| v.parse().ok())
-        .unwrap_or(20);
-    let song_count: u64 = get_query_param(&req, "songCount")
-        .and_then(|v| v.parse().ok())
-        .unwrap_or(20);
-
-    // Search tracks (which gives us artists and albums too)
-    let tracks = queries::tracks::search(state.db.conn(), &query)
+    let tracks = queries::tracks::search(state.db.conn(), query)
         .await
         .unwrap_or_default();
 
-    // Collect unique artists from tracks
     let mut seen_artists = std::collections::HashSet::new();
     let mut artist_results: Vec<serde_json::Value> = Vec::new();
     for track in &tracks {
@@ -61,7 +48,6 @@ pub async fn search3(req: HttpRequest, state: web::Data<AppState>) -> HttpRespon
         }
     }
 
-    // Also search artists by name directly
     let all_artists = queries::artists::list(state.db.conn(), 10000, 0)
         .await
         .unwrap_or_default();
@@ -83,7 +69,6 @@ pub async fn search3(req: HttpRequest, state: web::Data<AppState>) -> HttpRespon
         }
     }
 
-    // Collect unique albums from tracks
     let mut seen_albums = std::collections::HashSet::new();
     let mut album_results: Vec<serde_json::Value> = Vec::new();
     for track in &tracks {
@@ -104,20 +89,75 @@ pub async fn search3(req: HttpRequest, state: web::Data<AppState>) -> HttpRespon
         }
     }
 
-    // Song results
     let song_results: Vec<serde_json::Value> = tracks
         .iter()
         .take(song_count as usize)
         .map(|track| serde_json::to_value(SubsonicChild::from_track(track)).unwrap_or_default())
         .collect();
 
+    Ok(SearchResults {
+        artists: artist_results,
+        albums: album_results,
+        songs: song_results,
+    })
+}
+
+/// /rest/search3[.view]
+pub async fn search3(args: SubsonicArgs, state: web::Data<AppState>) -> HttpResponse {
+    let (params, _user) = match authenticate(&args, &state).await {
+        Ok(v) => v,
+        Err(resp) => return resp,
+    };
+
+    let results = match run_search(&args, &state).await {
+        Ok(r) => r,
+        Err(name) => {
+            return response::error(
+                &params,
+                response::ERROR_MISSING_PARAM,
+                &format!("missing required parameter: {name}"),
+            );
+        }
+    };
+
     response::ok(
-        &params.format,
+        &params,
         serde_json::json!({
             "searchResult3": {
-                "artist": artist_results,
-                "album": album_results,
-                "song": song_results,
+                "artist": results.artists,
+                "album": results.albums,
+                "song": results.songs,
+            }
+        }),
+    )
+}
+
+/// /rest/search2[.view] — same data as `search3`, different response wrapper
+/// key. Older clients (and mopidy-subidy in some configurations) call this.
+pub async fn search2(args: SubsonicArgs, state: web::Data<AppState>) -> HttpResponse {
+    let (params, _user) = match authenticate(&args, &state).await {
+        Ok(v) => v,
+        Err(resp) => return resp,
+    };
+
+    let results = match run_search(&args, &state).await {
+        Ok(r) => r,
+        Err(name) => {
+            return response::error(
+                &params,
+                response::ERROR_MISSING_PARAM,
+                &format!("missing required parameter: {name}"),
+            );
+        }
+    };
+
+    response::ok(
+        &params,
+        serde_json::json!({
+            "searchResult2": {
+                "artist": results.artists,
+                "album": results.albums,
+                "song": results.songs,
             }
         }),
     )

src/routes/subsonic/stubs.rs

@@ -0,0 +1,82 @@
+//! Stub implementations of Subsonic endpoints we don't actually back with
+//! real data. They satisfy strict clients that probe for these endpoints by
+//! returning a successful response with empty payloads — better than 404.
+//!
+//! These will get real implementations as the relevant features land.
+
+use actix_web::{HttpResponse, web};
+
+use crate::state::AppState;
+
+use super::SubsonicArgs;
+use super::helpers::authenticate;
+use super::response;
+
+macro_rules! stub_handler {
+    ($name:ident, $body:expr) => {
+        pub async fn $name(args: SubsonicArgs, state: web::Data<AppState>) -> HttpResponse {
+            let (params, _user) = match authenticate(&args, &state).await {
+                Ok(v) => v,
+                Err(resp) => return resp,
+            };
+            response::ok(&params, $body)
+        }
+    };
+}
+
+stub_handler!(
+    get_play_queue,
+    serde_json::json!({ "playQueue": { "entry": [] } })
+);
+stub_handler!(save_play_queue, serde_json::json!({}));
+stub_handler!(
+    get_bookmarks,
+    serde_json::json!({ "bookmarks": { "bookmark": [] } })
+);
+stub_handler!(create_bookmark, serde_json::json!({}));
+stub_handler!(delete_bookmark, serde_json::json!({}));
+stub_handler!(
+    get_scan_status,
+    serde_json::json!({ "scanStatus": { "scanning": false, "count": 0 } })
+);
+stub_handler!(
+    start_scan,
+    serde_json::json!({ "scanStatus": { "scanning": false, "count": 0 } })
+);
+stub_handler!(get_shares, serde_json::json!({ "shares": { "share": [] } }));
+stub_handler!(
+    get_podcasts,
+    serde_json::json!({ "podcasts": { "channel": [] } })
+);
+stub_handler!(
+    get_newest_podcasts,
+    serde_json::json!({ "newestPodcasts": { "episode": [] } })
+);
+stub_handler!(
+    get_internet_radio_stations,
+    serde_json::json!({ "internetRadioStations": { "internetRadioStation": [] } })
+);
+stub_handler!(
+    get_chat_messages,
+    serde_json::json!({ "chatMessages": { "chatMessage": [] } })
+);
+
+/// `getAvatar` returns a 404. The spec allows servers without avatar storage
+/// to return an error, and we don't track per-user avatars.
+pub async fn get_avatar(args: SubsonicArgs, state: web::Data<AppState>) -> HttpResponse {
+    let (params, _user) = match authenticate(&args, &state).await {
+        Ok(v) => v,
+        Err(resp) => return resp,
+    };
+    response::error(&params, response::ERROR_NOT_FOUND, "no avatar")
+}
+
+/// `jukeboxControl` is unsupported — return a not-authorized error so clients
+/// don't keep retrying.
+pub async fn jukebox_control(args: SubsonicArgs, state: web::Data<AppState>) -> HttpResponse {
+    let (params, _user) = match authenticate(&args, &state).await {
+        Ok(v) => v,
+        Err(resp) => return resp,
+    };
+    response::error(&params, response::ERROR_NOT_AUTHORIZED, "jukebox disabled")
+}

src/routes/subsonic/system.rs

@@ -1,29 +1,30 @@
-use actix_web::{HttpRequest, HttpResponse, web};
+use actix_web::{HttpResponse, web};
 
 use crate::state::AppState;
 
+use super::SubsonicArgs;
 use super::helpers::authenticate;
 use super::response;
 
-/// GET /rest/ping[.view]
-pub async fn ping(req: HttpRequest, state: web::Data<AppState>) -> HttpResponse {
-    let (params, _user) = match authenticate(&req, &state).await {
+/// /rest/ping[.view]
+pub async fn ping(args: SubsonicArgs, state: web::Data<AppState>) -> HttpResponse {
+    let (params, _user) = match authenticate(&args, &state).await {
         Ok(v) => v,
         Err(resp) => return resp,
     };
 
-    response::ok(&params.format, serde_json::json!({}))
+    response::ok(&params, serde_json::json!({}))
 }
 
-/// GET /rest/getLicense[.view]
-pub async fn get_license(req: HttpRequest, state: web::Data<AppState>) -> HttpResponse {
-    let (params, _user) = match authenticate(&req, &state).await {
+/// /rest/getLicense[.view]
+pub async fn get_license(args: SubsonicArgs, state: web::Data<AppState>) -> HttpResponse {
+    let (params, _user) = match authenticate(&args, &state).await {
         Ok(v) => v,
         Err(resp) => return resp,
     };
 
     response::ok(
-        &params.format,
+        &params,
         serde_json::json!({
             "license": {
                 "valid": true,
@@ -33,3 +34,25 @@ pub async fn get_license(req: HttpRequest, state: web::Data<AppState>) -> HttpRe
         }),
     )
 }
+
+/// /rest/getOpenSubsonicExtensions[.view]
+///
+/// Required by the OpenSubsonic spec since we advertise `openSubsonic: true`.
+/// We support no optional extensions yet, so the array is empty — that's a
+/// valid response and is enough to keep extension-aware clients happy.
+pub async fn get_open_subsonic_extensions(
+    args: SubsonicArgs,
+    state: web::Data<AppState>,
+) -> HttpResponse {
+    let (params, _user) = match authenticate(&args, &state).await {
+        Ok(v) => v,
+        Err(resp) => return resp,
+    };
+
+    response::ok(
+        &params,
+        serde_json::json!({
+            "openSubsonicExtensions": []
+        }),
+    )
+}

src/routes/subsonic/user.rs

@@ -1,15 +1,16 @@
-use actix_web::{HttpRequest, HttpResponse, web};
+use actix_web::{HttpResponse, web};
 
 use shanty_db::entities::user::UserRole;
 
 use crate::state::AppState;
 
+use super::SubsonicArgs;
 use super::helpers::authenticate;
 use super::response;
 
-/// GET /rest/getUser[.view]
-pub async fn get_user(req: HttpRequest, state: web::Data<AppState>) -> HttpResponse {
-    let (params, user) = match authenticate(&req, &state).await {
+/// /rest/getUser[.view]
+pub async fn get_user(args: SubsonicArgs, state: web::Data<AppState>) -> HttpResponse {
+    let (params, user) = match authenticate(&args, &state).await {
         Ok(v) => v,
         Err(resp) => return resp,
     };
@@ -17,7 +18,7 @@ pub async fn get_user(req: HttpRequest, state: web::Data<AppState>) -> HttpRespo
     let is_admin = user.role == UserRole::Admin;
 
     response::ok(
-        &params.format,
+        &params,
         serde_json::json!({
             "user": {
                 "username": user.username,