diff --git a/.kilo/plans/1775285337131-neon-mountain.md b/.kilo/plans/1775285337131-neon-mountain.md index e04f00ef..d7ccea16 100644 --- a/.kilo/plans/1775285337131-neon-mountain.md +++ b/.kilo/plans/1775285337131-neon-mountain.md @@ -222,4 +222,142 @@ cargo test base_url --- -**Le plan original est bon mais surestime le travail.** La correction principale est de clarifier que le problème vient des tâches de fond (background tasks), pas des handlers REST. \ No newline at end of file +## Problème complémentaire : URLs de covers des media servers externes + +### Contexte + +Quand le control point accède à un media server externe sur le LAN (autre que pmomusic), les URLs d'articles (`album_art_uri`) retournées par ce media server externe contiennent des IPs locales du LAN externe (ex: `http://192.168.1.100:8080/covers/...`). + +Ces URLs ne passent pas par notre système de caching et ne peuvent pas être rewritées par le middleware `BaseUrl` car elles sont : +1. Recues depuis le réseau UPnP (pas via HTTP) +2. Propagées directement dans les réponses REST/SSE sans transformation + +### Solution proposée : Proxy de covers avec cache + +Créer un nouveau endpoint HTTP qui agit comme un proxy transparent : +1. **Détection** : Si l'URL demandée est une URL LAN externe (pas une URL locale de pmomusic) +2. **Caching** : Utiliser `cache.add_from_url()` qui gère déjà la déduplication (pas de double-cache) +3. **Rewriting** : Retourner l'URL locale du cache (`/covers/image/{pk}`) + +**Note importante** : `pmocache::add_from_url()` gère déjà : +- La vérification si l'URL est déjà en cache (ligne 673-683) +- Le calcul du pk basé sur le contenu (pas sur l'URL) +- La déduplication automatique pour les mêmes contenus + +### Implémentation + +**Nouvel endpoint dans `pmocovers/src/lib.rs` ou nouveau fichier `pmocovers/src/proxy.rs`** : + +```rust +#[derive(Debug, Deserialize)] +struct CoverProxyParams { + url: String, +} + +#[derive(Debug, Serialize)] +struct CoverProxyResponse { + cached_url: String, + pk: String, +} + +/// GET /covers/proxy?url= +/// Proxy transparent qui : +/// 1. Détecte si l'URL est une URL LAN externe (pas déjà locale) +/// 2. Ajoute à cache via add_from_url (déduplication automatique) +/// 3. Retourne l'URL locale du cache +pub async fn cover_proxy_handler( + Query(params): Query, + State(cache): State, + Extension(base_url): Extension, +) -> Result { + let external_url = ¶ms.url; + + // Ignorer si déjà une URL locale (ne pas se cacher soi-même) + if is_local_cover_url(external_url, &base_url) { + return Err((StatusCode::BAD_REQUEST, "URL is already a local cover")); + } + + // Vérifier si c'est une URL LAN à proxyfier + if !should_proxy_url(external_url) { + return Err((StatusCode::BAD_REQUEST, "URL is not a LAN URL requiring proxy")); + } + + // Ajouter au cache (add_from_url gère la déduplication) + let pk = cache.add_from_url(external_url, Some("external-covers")) + .await + .map_err(|e| (StatusCode::BAD_GATEWAY, e.to_string()))?; + + // Retourner l'URL locale + let local_url = base_url.url_for(&pmocache::covers_route_for(&pk, None)); + Ok(Json(CoverProxyResponse { cached_url: local_url, pk })) +} + +/// Vérifie si l'URL est déjà une cover locale de NOTRE instance pmomusic +/// Note: Les covers d'autres instances pmomusic sur le LAN DEVRAIENT être proxyfiées +/// et mises en cache localement - c'est le comportement desired! +fn is_local_cover_url(url: &str, base_url: &pmoserver::BaseUrl) -> bool { + // Only skip if it's OUR instance's base URL + // Covers from other pmomusic instances on LAN should be proxied and cached + url.starts_with(&base_url.0) +} + +/// Vérifie si l'URL doit être proxyfiée (URL LAN externe) +fn should_proxy_url(url: &str) -> bool { + if let Ok(parsed) = url::Url::parse(url) { + if let Some(host) = parsed.host_str() { + // Proxy uniquement les URLs LAN (pas les URLs publiques) + if let Ok(ip) = host.parse::() { + return ip.is_private() || ip.is_loopback(); + } + // aussi les .local + return host.ends_with(".local") || host == "localhost"; + } + } + false +} +``` + +**Points importants** : +- Utiliser `add_from_url()` pour bénéficier de la déduplication automatique +- Vérifier `is_local_cover_url()` avec uniquement la comparaison de base_url pour éviter que notre instance ne se cache elle-même +- Les covers d'autres instances pmomusic sur le LAN DEVRAIENT être proxyfiées (comportement souhaité!) +- Le TTL sera celui par défaut du cache (configurable) + +**Mise à jour des handlers REST** : + +Dans `pmocontrol/src/pmoserver_ext.rs` et `pmocontrol/src/sse.rs`, transformer les `album_art_uri` LAN : + +```rust +fn transform_external_cover_url(url: &str) -> String { + if is_lan_url(url) { + // Remplacer par l'URL du proxy + let encoded = urlencoding::encode(url); + return format!("/covers/proxy?url={}", encoded); + } + url.to_string() +} +``` + +**Appels dans les handlers** : + +- `pmocontrol/src/pmoserver_ext.rs:2173` : `browse_container` → transformer `album_art_uri` +- `pmocontrol/src/pmoserver_ext.rs:2424` : autre endpoint → même transformation +- `pmocontrol/src/sse.rs:213` : `MetadataChanged` events → même transformation + +### TTL + +- Le TTL sera celui par défaut du cache `pmocovers` +- C'est configurable via `pmoconfig` si besoin + +### Sécurité + +- Limiter aux URLs LAN uniquement (`192.168.x.x`, `10.x.x.x`, `172.16-31.x.x`, `localhost`) +- Vérifier que l'URL n'est pas déjà une cover locale de pmomusic (éviter le cacheception) +- Ajouter un rate limiting pour éviter le flood de téléchargement +- Timeout de téléchargement : 10 secondes max + +### Résumé des fichiers à modifier + +1. **Nouveau** : `pmocovers/src/proxy.rs` - Endpoint de proxy +2. **Modifier** : `pmocontrol/src/pmoserver_ext.rs` - Transformer les album_art_uri +3. **Modifier** : `pmocontrol/src/sse.rs` - Transformer les album_art_uri dans les événements \ No newline at end of file diff --git a/Cargo.lock b/Cargo.lock index 9d5c22f6..639ca9c1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4016,6 +4016,8 @@ dependencies = [ "tracing-log 0.1.4", "tracing-subscriber", "ureq", + "url", + "urlencoding", "utoipa", "xmltree 0.11.0", ] @@ -4038,6 +4040,7 @@ dependencies = [ "tempfile", "tokio", "tracing", + "url", "utoipa", "webp", ] @@ -6550,6 +6553,12 @@ dependencies = [ "serde", ] +[[package]] +name = "urlencoding" +version = "2.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "daf8dba3b7eb870caf1ddeed7bc9d2a049f3cfdfae7cb521b087cc33ae4c49da" + [[package]] name = "users" version = "0.11.0" diff --git a/pmocontrol/Cargo.toml b/pmocontrol/Cargo.toml index fac1cec4..86a176c2 100644 --- a/pmocontrol/Cargo.toml +++ b/pmocontrol/Cargo.toml @@ -38,6 +38,8 @@ async-trait = { version = "0.1", optional = true } tokio-stream = { version = "0.1", features = ["sync"], optional = true } async-stream = { version = "0.3", optional = true } chrono = { version = "0.4", features = ["serde"] } +url = { version = "2", optional = true } +urlencoding = { version = "2", optional = true } [dev-dependencies] percent-encoding = "2.3" @@ -45,4 +47,4 @@ percent-encoding = "2.3" [features] default = [] # Active l'API REST pmoserver -pmoserver = ["dep:pmoserver", "dep:utoipa", "dep:axum", "dep:tokio", "dep:tokio-util", "dep:async-trait", "dep:tokio-stream", "dep:async-stream"] +pmoserver = ["dep:pmoserver", "dep:utoipa", "dep:axum", "dep:tokio", "dep:tokio-util", "dep:async-trait", "dep:tokio-stream", "dep:async-stream", "dep:url", "dep:urlencoding"] diff --git a/pmocontrol/src/pmoserver_ext.rs b/pmocontrol/src/pmoserver_ext.rs index 549f28c6..e8007e59 100644 --- a/pmocontrol/src/pmoserver_ext.rs +++ b/pmocontrol/src/pmoserver_ext.rs @@ -28,7 +28,7 @@ use async_trait::async_trait; use axum::{ Json, Router, extract::{Path, Query, State}, - http::StatusCode, + http::{StatusCode, header::HeaderMap}, routing::{get, post}, }; #[cfg(feature = "pmoserver")] @@ -2081,7 +2081,9 @@ async fn browse_container( State(state): State, Path((server_id, container_id)): Path<(String, String)>, Query(params): Query, + headers: HeaderMap, ) -> Result, (StatusCode, Json)> { + let base_url = pmoserver::get_base_url_from_request(&headers); let sid = DeviceId(server_id.clone()); let server = state.control_point.media_server(&sid).ok_or_else(|| { @@ -2170,7 +2172,7 @@ async fn browse_container( child_count: None, artist: e.artist, album: e.album, - album_art_uri: e.album_art_uri, + album_art_uri: transform_cover_url(e.album_art_uri.as_deref(), &base_url), }) .collect(); @@ -2204,6 +2206,41 @@ fn map_snapshot_error( ) } +/// Transforme une URL de cover externe LAN en URL de proxy local +fn transform_cover_url(url: Option<&str>, base_url: &str) -> Option { + let url = url?; + + // Si c'est déjà une URL locale de notre instance, ne pas transformer + if url.starts_with(base_url) { + return Some(url.to_string()); + } + + // Vérifier si c'est une URL LAN externe à proxyfier + if should_proxy_cover_url(url) { + let encoded = urlencoding::encode(url); + return Some(format!("/covers/proxy?url={}", encoded)); + } + + //URL publique ou autre - laisser telle quelle + Some(url.to_string()) +} + +/// Vérifie si l'URL doit être proxyfiée (URL LAN externe) +fn should_proxy_cover_url(url: &str) -> bool { + if let Ok(parsed) = url::Url::parse(url) { + if let Some(host) = parsed.host_str() { + if let Ok(ip) = host.parse::() { + return match ip { + std::net::IpAddr::V4(ipv4) => ipv4.is_private() || ipv4.is_loopback(), + std::net::IpAddr::V6(ipv6) => ipv6.is_loopback(), + }; + } + return host.ends_with(".local") || host == "localhost"; + } + } + false +} + /// Helper to fetch playback items from a media server object (container or item). /// /// This function browses the server to get the entries and converts them to PlaybackItem. diff --git a/pmocovers/Cargo.toml b/pmocovers/Cargo.toml index 336ebf69..9d1261e1 100644 --- a/pmocovers/Cargo.toml +++ b/pmocovers/Cargo.toml @@ -18,6 +18,7 @@ reqwest = { version = "0.12", features = ["blocking"] } anyhow = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } +url = "2" # Async tokio = { workspace = true } diff --git a/pmocovers/src/api.rs b/pmocovers/src/api.rs index 060f8c2f..4c376fb8 100644 --- a/pmocovers/src/api.rs +++ b/pmocovers/src/api.rs @@ -2,8 +2,15 @@ use crate::cache; use crate::Cache; -use axum::{extract::State, http::StatusCode, response::IntoResponse, Json}; +use axum::{ + extract::{Query, State}, + http::StatusCode, + response::IntoResponse, + Extension, Json, +}; use pmocache::api::{AddItemRequest, AddItemResponse, ErrorResponse}; +use pmocache::covers_route_for; +use serde::{Deserialize, Serialize}; use std::sync::Arc; #[derive(Clone, Copy)] @@ -84,3 +91,104 @@ pub async fn add_cover_item( .into_response(), } } + +// ============================================================================ +// Proxy pour covers LAN externes +// ============================================================================ + +#[derive(Debug, Deserialize)] +pub struct CoverProxyParams { + url: String, +} + +#[derive(Debug, Serialize)] +pub struct CoverProxyResponse { + pub cached_url: String, + pub pk: String, +} + +/// GET /covers/proxy?url= +/// Proxy transparent qui : +/// 1. Détecte si l'URL est une URL LAN externe (pas déjà locale) +/// 2. Ajoute à cache via add_from_url (déduplication automatique) +/// 3. Retourne l'URL locale du cache +#[cfg(feature = "pmoserver")] +pub async fn cover_proxy_handler( + Query(params): Query, + State(cache): State>, + Extension(base_url): Extension, +) -> impl IntoResponse { + let external_url = ¶ms.url; + + // Ignorer si déjà une URL de NOTRE instance pmomusic (ne pas se cacher soi-même) + if is_local_cover_url(external_url, &base_url) { + return ( + StatusCode::BAD_REQUEST, + Json(ErrorResponse { + error: "INVALID_REQUEST".to_string(), + message: "URL is already a local cover from this instance".to_string(), + }), + ) + .into_response(); + } + + // Vérifier si c'est une URL LAN à proxyfier + if !should_proxy_url(external_url) { + return ( + StatusCode::BAD_REQUEST, + Json(ErrorResponse { + error: "INVALID_REQUEST".to_string(), + message: "URL is not a LAN URL requiring proxy".to_string(), + }), + ) + .into_response(); + } + + // Ajouter au cache (add_from_url gère la déduplication) + match cache.add_from_url(external_url, Some("external-covers")).await { + Ok(pk) => { + // Retourner l'URL locale + let local_url = base_url.url_for(&covers_route_for(&pk, None)); + ( + StatusCode::OK, + Json(CoverProxyResponse { + cached_url: local_url, + pk, + }), + ) + .into_response() + } + Err(e) => ( + StatusCode::BAD_GATEWAY, + Json(ErrorResponse { + error: "CACHE_ERROR".to_string(), + message: format!("Failed to cache external cover: {}", e), + }), + ) + .into_response(), + } +} + +/// Vérifie si l'URL est déjà une cover locale de NOTRE instance pmomusic +/// Note: Les covers d'autres instances pmomusic sur le LAN DEVRAIENT être proxyfiées +fn is_local_cover_url(url: &str, base_url: &pmoserver::BaseUrl) -> bool { + url.starts_with(&base_url.0) +} + +/// Vérifie si l'URL doit être proxyfiée (URL LAN externe) +fn should_proxy_url(url: &str) -> bool { + if let Ok(parsed) = url::Url::parse(url) { + if let Some(host) = parsed.host_str() { + // Proxy uniquement les URLs LAN (pas les URLs publiques) + if let Ok(ip) = host.parse::() { + return match ip { + std::net::IpAddr::V4(ipv4) => ipv4.is_private() || ipv4.is_loopback(), + std::net::IpAddr::V6(ipv6) => ipv6.is_loopback(), + }; + } + // aussi les .local + return host.ends_with(".local") || host == "localhost"; + } + } + false +} diff --git a/pmocovers/src/lib.rs b/pmocovers/src/lib.rs index 7b8a8867..054762a0 100644 --- a/pmocovers/src/lib.rs +++ b/pmocovers/src/lib.rs @@ -382,6 +382,10 @@ impl CoverCacheExt for pmoserver::Server { "/consolidate", axum::routing::post(pmocache::api::consolidate_cache::), ) + .route( + "/proxy", + axum::routing::get(crate::api::cover_proxy_handler), + ) .with_state(cache.clone()); let openapi = crate::ApiDoc::openapi();