Many improvemtns and fixes related to decoupling of svelte and rust on android.
This commit is contained in:
+32
-31
@@ -102,12 +102,18 @@ impl AuthManager {
|
||||
self.connectivity_monitor = Some(monitor);
|
||||
}
|
||||
|
||||
/// Normalize and validate server URL
|
||||
pub fn normalize_url(url: &str) -> String {
|
||||
/// Normalize and validate server URL.
|
||||
/// Enforces HTTPS — plain HTTP is rejected for security.
|
||||
pub fn normalize_url(url: &str) -> Result<String, String> {
|
||||
let mut normalized = url.trim().to_string();
|
||||
|
||||
// Reject plain HTTP — all connections must use HTTPS
|
||||
if normalized.starts_with("http://") {
|
||||
return Err("HTTP connections are not allowed. Please use HTTPS (e.g., https://your-server.com).".to_string());
|
||||
}
|
||||
|
||||
// Add https:// if no protocol specified
|
||||
if !normalized.starts_with("http://") && !normalized.starts_with("https://") {
|
||||
if !normalized.starts_with("https://") {
|
||||
normalized = format!("https://{}", normalized);
|
||||
}
|
||||
|
||||
@@ -116,12 +122,12 @@ impl AuthManager {
|
||||
normalized.pop();
|
||||
}
|
||||
|
||||
normalized
|
||||
Ok(normalized)
|
||||
}
|
||||
|
||||
/// Connect to server and get server info
|
||||
pub async fn connect_to_server(&self, server_url: &str) -> Result<ServerInfo, String> {
|
||||
let normalized_url = Self::normalize_url(server_url);
|
||||
let normalized_url = Self::normalize_url(server_url)?;
|
||||
let endpoint = format!("{}/System/Info/Public", normalized_url);
|
||||
|
||||
log::info!("[AuthManager] Connecting to server: {}", normalized_url);
|
||||
@@ -165,7 +171,7 @@ impl AuthManager {
|
||||
password: &str,
|
||||
device_id: &str,
|
||||
) -> Result<AuthResult, String> {
|
||||
let url = Self::normalize_url(server_url);
|
||||
let url = Self::normalize_url(server_url)?;
|
||||
let endpoint = format!("{}/Users/AuthenticateByName", url);
|
||||
|
||||
log::info!("[AuthManager] Authenticating user: {}", username);
|
||||
@@ -227,7 +233,7 @@ impl AuthManager {
|
||||
access_token: &str,
|
||||
device_id: &str,
|
||||
) -> Result<User, String> {
|
||||
let url = Self::normalize_url(server_url);
|
||||
let url = Self::normalize_url(server_url)?;
|
||||
let endpoint = format!("{}/Users/{}", url, user_id);
|
||||
|
||||
log::info!("[AuthManager] Verifying session for user: {}", user_id);
|
||||
@@ -290,7 +296,7 @@ impl AuthManager {
|
||||
access_token: &str,
|
||||
device_id: &str,
|
||||
) -> Result<(), String> {
|
||||
let url = Self::normalize_url(server_url);
|
||||
let url = Self::normalize_url(server_url)?;
|
||||
let endpoint = format!("{}/Sessions/Logout", url);
|
||||
|
||||
log::info!("[AuthManager] Logging out");
|
||||
@@ -337,43 +343,43 @@ mod tests {
|
||||
use super::*;
|
||||
|
||||
/// Test URL normalization - adds https:// when missing
|
||||
///
|
||||
/// Ensures that URLs without protocol are normalized to https://
|
||||
/// This prevents "builder error" when constructing HTTP requests.
|
||||
#[test]
|
||||
fn test_normalize_url_adds_https() {
|
||||
assert_eq!(
|
||||
AuthManager::normalize_url("jellyfin.example.com"),
|
||||
AuthManager::normalize_url("jellyfin.example.com").unwrap(),
|
||||
"https://jellyfin.example.com"
|
||||
);
|
||||
assert_eq!(
|
||||
AuthManager::normalize_url("192.168.1.100:8096"),
|
||||
AuthManager::normalize_url("192.168.1.100:8096").unwrap(),
|
||||
"https://192.168.1.100:8096"
|
||||
);
|
||||
}
|
||||
|
||||
/// Test URL normalization - preserves existing protocol
|
||||
/// Test URL normalization - preserves existing https
|
||||
#[test]
|
||||
fn test_normalize_url_preserves_protocol() {
|
||||
fn test_normalize_url_preserves_https() {
|
||||
assert_eq!(
|
||||
AuthManager::normalize_url("https://jellyfin.example.com"),
|
||||
AuthManager::normalize_url("https://jellyfin.example.com").unwrap(),
|
||||
"https://jellyfin.example.com"
|
||||
);
|
||||
assert_eq!(
|
||||
AuthManager::normalize_url("http://localhost:8096"),
|
||||
"http://localhost:8096"
|
||||
);
|
||||
}
|
||||
|
||||
/// Test URL normalization - rejects HTTP
|
||||
#[test]
|
||||
fn test_normalize_url_rejects_http() {
|
||||
assert!(AuthManager::normalize_url("http://localhost:8096").is_err());
|
||||
assert!(AuthManager::normalize_url("http://jellyfin.example.com").is_err());
|
||||
}
|
||||
|
||||
/// Test URL normalization - removes trailing slash
|
||||
#[test]
|
||||
fn test_normalize_url_removes_trailing_slash() {
|
||||
assert_eq!(
|
||||
AuthManager::normalize_url("https://jellyfin.example.com/"),
|
||||
AuthManager::normalize_url("https://jellyfin.example.com/").unwrap(),
|
||||
"https://jellyfin.example.com"
|
||||
);
|
||||
assert_eq!(
|
||||
AuthManager::normalize_url("jellyfin.example.com/"),
|
||||
AuthManager::normalize_url("jellyfin.example.com/").unwrap(),
|
||||
"https://jellyfin.example.com"
|
||||
);
|
||||
}
|
||||
@@ -382,25 +388,20 @@ mod tests {
|
||||
#[test]
|
||||
fn test_normalize_url_trims_whitespace() {
|
||||
assert_eq!(
|
||||
AuthManager::normalize_url(" jellyfin.example.com "),
|
||||
AuthManager::normalize_url(" jellyfin.example.com ").unwrap(),
|
||||
"https://jellyfin.example.com"
|
||||
);
|
||||
assert_eq!(
|
||||
AuthManager::normalize_url(" https://jellyfin.example.com/ "),
|
||||
AuthManager::normalize_url(" https://jellyfin.example.com/ ").unwrap(),
|
||||
"https://jellyfin.example.com"
|
||||
);
|
||||
}
|
||||
|
||||
/// Test URL normalization - complex case
|
||||
///
|
||||
/// This is the bug that caused the login issue: user enters URL
|
||||
/// without protocol, it gets stored in DB, then fails when building
|
||||
/// HTTP requests.
|
||||
/// Test URL normalization - real world case
|
||||
#[test]
|
||||
fn test_normalize_url_real_world_case() {
|
||||
// User input: "jellyfin.tourolle.paris"
|
||||
let input = "jellyfin.tourolle.paris";
|
||||
let normalized = AuthManager::normalize_url(input);
|
||||
let normalized = AuthManager::normalize_url(input).unwrap();
|
||||
|
||||
assert_eq!(normalized, "https://jellyfin.tourolle.paris");
|
||||
assert!(normalized.starts_with("https://"));
|
||||
|
||||
@@ -39,7 +39,7 @@ pub async fn auth_initialize(
|
||||
};
|
||||
|
||||
// Create session object from active session with normalized URL
|
||||
let normalized_url = crate::auth::AuthManager::normalize_url(&active_session.server_url);
|
||||
let normalized_url = crate::auth::AuthManager::normalize_url(&active_session.server_url)?;
|
||||
|
||||
let session = Session {
|
||||
user_id: active_session.user_id,
|
||||
@@ -80,7 +80,7 @@ pub async fn auth_login(
|
||||
let result = auth_manager.0.login(&server_url, &username, &password, &device_id).await?;
|
||||
|
||||
// Create session from auth result with normalized URL
|
||||
let normalized_url = crate::auth::AuthManager::normalize_url(&server_url);
|
||||
let normalized_url = crate::auth::AuthManager::normalize_url(&server_url)?;
|
||||
|
||||
let session = Session {
|
||||
user_id: result.user.id.clone(),
|
||||
@@ -156,10 +156,13 @@ pub async fn auth_set_session(
|
||||
auth_manager: State<'_, AuthManagerWrapper>,
|
||||
) -> Result<(), String> {
|
||||
// Normalize the server URL if session is provided
|
||||
let normalized_session = session.map(|mut s| {
|
||||
s.server_url = crate::auth::AuthManager::normalize_url(&s.server_url);
|
||||
s
|
||||
});
|
||||
let normalized_session = match session {
|
||||
Some(mut s) => {
|
||||
s.server_url = crate::auth::AuthManager::normalize_url(&s.server_url)?;
|
||||
Some(s)
|
||||
}
|
||||
None => None,
|
||||
};
|
||||
|
||||
auth_manager.0.set_session(normalized_session).await;
|
||||
Ok(())
|
||||
|
||||
@@ -956,7 +956,26 @@ pub async fn start_download(
|
||||
debug!("Download task started for download_id: {}", download_id);
|
||||
let worker = DownloadWorker::new();
|
||||
|
||||
match worker.download(&task).await {
|
||||
// Progress callback that emits events to the frontend
|
||||
let progress_app = app_clone.clone();
|
||||
let progress_item_id = item_id_clone.clone();
|
||||
let on_progress = move |bytes_downloaded: u64, total_bytes: Option<u64>| {
|
||||
let progress = total_bytes
|
||||
.filter(|&t| t > 0)
|
||||
.map(|t| bytes_downloaded as f64 / t as f64)
|
||||
.unwrap_or(0.0);
|
||||
|
||||
let event = DownloadEvent::Progress {
|
||||
download_id,
|
||||
item_id: progress_item_id.clone(),
|
||||
bytes_downloaded: bytes_downloaded as i64,
|
||||
total_bytes: total_bytes.map(|t| t as i64),
|
||||
progress,
|
||||
};
|
||||
let _ = progress_app.emit("download-event", event);
|
||||
};
|
||||
|
||||
match worker.download(&task, on_progress).await {
|
||||
Ok(result) => {
|
||||
info!("Download completed successfully: {} bytes", result.bytes_downloaded);
|
||||
|
||||
|
||||
@@ -195,4 +195,64 @@ mod tests {
|
||||
assert!(json.contains(&pos.to_string()));
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_playback_mode_deserialization_from_frontend() {
|
||||
// Test what frontend sends for Idle mode
|
||||
let idle_json = r#"{"type":"idle"}"#;
|
||||
let mode: PlaybackMode = serde_json::from_str(idle_json).expect("Failed to deserialize idle");
|
||||
assert_eq!(mode, PlaybackMode::Idle);
|
||||
|
||||
// Test what frontend sends for Local mode
|
||||
let local_json = r#"{"type":"local"}"#;
|
||||
let mode: PlaybackMode = serde_json::from_str(local_json).expect("Failed to deserialize local");
|
||||
assert_eq!(mode, PlaybackMode::Local);
|
||||
|
||||
// Test what frontend sends for Remote mode
|
||||
let remote_json = r#"{"type":"remote","session_id":"session-123"}"#;
|
||||
let mode: PlaybackMode = serde_json::from_str(remote_json).expect("Failed to deserialize remote");
|
||||
match mode {
|
||||
PlaybackMode::Remote { session_id } => assert_eq!(session_id, "session-123"),
|
||||
_ => panic!("Expected Remote mode"),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_play_tracks_context_deserialization() {
|
||||
use crate::commands::PlayTracksContext;
|
||||
|
||||
// Test Search context (the recently fixed issue)
|
||||
let search_json = r#"{"type":"search","searchQuery":"test query"}"#;
|
||||
let context: PlayTracksContext = serde_json::from_str(search_json)
|
||||
.expect("Failed to deserialize search context");
|
||||
match context {
|
||||
PlayTracksContext::Search { search_query } => {
|
||||
assert_eq!(search_query, "test query");
|
||||
}
|
||||
_ => panic!("Expected Search context"),
|
||||
}
|
||||
|
||||
// Test Playlist context
|
||||
let playlist_json = r#"{"type":"playlist","playlistId":"pl-123","playlistName":"My Playlist"}"#;
|
||||
let context: PlayTracksContext = serde_json::from_str(playlist_json)
|
||||
.expect("Failed to deserialize playlist context");
|
||||
match context {
|
||||
PlayTracksContext::Playlist { playlist_id, playlist_name } => {
|
||||
assert_eq!(playlist_id, "pl-123");
|
||||
assert_eq!(playlist_name, "My Playlist");
|
||||
}
|
||||
_ => panic!("Expected Playlist context"),
|
||||
}
|
||||
|
||||
// Test Custom context
|
||||
let custom_json = r#"{"type":"custom","label":"Custom Queue"}"#;
|
||||
let context: PlayTracksContext = serde_json::from_str(custom_json)
|
||||
.expect("Failed to deserialize custom context");
|
||||
match context {
|
||||
PlayTracksContext::Custom { label } => {
|
||||
assert_eq!(label, Some("Custom Queue".to_string()));
|
||||
}
|
||||
_ => panic!("Expected Custom context"),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -88,7 +88,7 @@ impl From<&crate::player::MediaItem> for MergedMediaItem {
|
||||
album: item.album.clone(),
|
||||
album_id: item.album_id.clone(),
|
||||
duration: item.duration,
|
||||
primary_image_tag: None,
|
||||
primary_image_tag: item.primary_image_tag.clone(),
|
||||
media_type: match item.media_type {
|
||||
crate::player::MediaType::Audio => "audio".to_string(),
|
||||
crate::player::MediaType::Video => "video".to_string(),
|
||||
@@ -138,43 +138,20 @@ pub enum VideoBackend {
|
||||
Html5,
|
||||
}
|
||||
|
||||
/// Request to play a single item
|
||||
/// Request to play a single video item
|
||||
///
|
||||
/// Simplified to video playback only. Audio playback uses player_play_tracks
|
||||
/// to avoid Tauri Android serialization issues with complex objects.
|
||||
#[derive(Debug, Deserialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
pub struct PlayItemRequest {
|
||||
pub id: String,
|
||||
pub title: String,
|
||||
pub artist: Option<String>,
|
||||
pub album: Option<String>,
|
||||
/// Album ID (Jellyfin ID) for remote transfer context
|
||||
#[serde(default)]
|
||||
pub album_id: Option<String>,
|
||||
/// Playlist ID (Jellyfin ID) for remote transfer context
|
||||
#[serde(default)]
|
||||
pub playlist_id: Option<String>,
|
||||
pub duration: Option<f64>,
|
||||
pub artwork_url: Option<String>,
|
||||
pub media_type: MediaType,
|
||||
pub stream_url: String,
|
||||
pub jellyfin_item_id: Option<String>,
|
||||
/// Video codec (e.g., "h264", "hevc") for video media
|
||||
#[serde(default)]
|
||||
pub video_codec: Option<String>,
|
||||
pub video_codec: String,
|
||||
/// Whether the video requires server-side transcoding
|
||||
#[serde(default)]
|
||||
pub needs_transcoding: bool,
|
||||
/// Video width in pixels
|
||||
#[serde(default)]
|
||||
pub video_width: Option<u32>,
|
||||
/// Video height in pixels
|
||||
#[serde(default)]
|
||||
pub video_height: Option<u32>,
|
||||
/// Series ID (for TV show episodes) - used for series audio preferences
|
||||
#[serde(default)]
|
||||
pub series_id: Option<String>,
|
||||
/// Server ID - used for series audio preferences
|
||||
#[serde(default)]
|
||||
pub server_id: Option<String>,
|
||||
}
|
||||
|
||||
/// Queue context for remote transfer - what type of queue is this?
|
||||
@@ -345,16 +322,20 @@ pub enum AudioTrackSwitchResponse {
|
||||
},
|
||||
}
|
||||
|
||||
/// Helper function to create MediaItem from request, checking for local downloads
|
||||
/// Helper function to create MediaItem from video request
|
||||
///
|
||||
/// PlayItemRequest is now video-only, so we create a video MediaItem.
|
||||
/// Audio playback uses player_play_tracks which fetches full metadata from backend.
|
||||
async fn create_media_item(
|
||||
req: PlayItemRequest,
|
||||
db: Option<&DatabaseWrapper>,
|
||||
) -> Result<MediaItem, String> {
|
||||
let jellyfin_id = req.jellyfin_item_id.as_ref().unwrap_or(&req.id);
|
||||
// For video-only requests, we use the item ID as the jellyfin ID
|
||||
let jellyfin_id = req.id.clone();
|
||||
|
||||
// Check if item is downloaded locally
|
||||
let local_path = if let Some(db_wrapper) = db {
|
||||
check_for_local_download(db_wrapper, jellyfin_id).await?
|
||||
check_for_local_download(db_wrapper, &jellyfin_id).await?
|
||||
} else {
|
||||
None
|
||||
};
|
||||
@@ -374,27 +355,27 @@ async fn create_media_item(
|
||||
Ok(MediaItem {
|
||||
id: req.id.clone(),
|
||||
title: req.title.clone(),
|
||||
name: Some(req.title), // Frontend compatibility
|
||||
artist: req.artist.clone(),
|
||||
album: req.album.clone(),
|
||||
album_name: req.album, // Frontend compatibility
|
||||
album_id: req.album_id,
|
||||
artist_items: None, // Not available from frontend request
|
||||
artists: req.artist.map(|a| vec![a]), // Convert single artist to array
|
||||
primary_image_tag: None, // Not available from frontend request
|
||||
item_type: None, // Not available from frontend request
|
||||
playlist_id: req.playlist_id,
|
||||
duration: req.duration,
|
||||
artwork_url: req.artwork_url,
|
||||
media_type: req.media_type,
|
||||
name: Some(req.title.clone()),
|
||||
artist: None, // Not available from video-only request
|
||||
album: None, // Not available from video-only request
|
||||
album_name: None, // Not available from video-only request
|
||||
album_id: None, // Not available from video-only request
|
||||
artist_items: None, // Not available from video-only request
|
||||
artists: None, // Not available from video-only request
|
||||
primary_image_tag: None, // Not available from video-only request
|
||||
item_type: None, // Not available from video-only request
|
||||
playlist_id: None, // Not available from video-only request
|
||||
duration: None, // Not available from video-only request
|
||||
artwork_url: None, // Not available from video-only request
|
||||
media_type: crate::player::MediaType::Video, // Video-only request
|
||||
source,
|
||||
video_codec: req.video_codec,
|
||||
video_codec: Some(req.video_codec),
|
||||
needs_transcoding: req.needs_transcoding,
|
||||
video_width: req.video_width,
|
||||
video_height: req.video_height,
|
||||
video_width: None, // Not available from video-only request
|
||||
video_height: None, // Not available from video-only request
|
||||
subtitles: vec![],
|
||||
series_id: req.series_id,
|
||||
server_id: req.server_id,
|
||||
series_id: None, // Not available from video-only request
|
||||
server_id: None, // Not available from video-only request
|
||||
})
|
||||
}
|
||||
|
||||
@@ -433,6 +414,9 @@ async fn check_for_local_download(
|
||||
|
||||
/// Play a single media item (audio or video)
|
||||
///
|
||||
/// Accepts a PlayItemRequest with all optional fields properly defaulted.
|
||||
/// This avoids Tauri's Android serialization issues with complex objects.
|
||||
///
|
||||
/// @req: UR-003 - Play videos
|
||||
/// @req: UR-004 - Play audio uninterrupted
|
||||
/// @req: UR-005 - Control media playback (play operation)
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
//! Tauri commands for database/storage operations
|
||||
|
||||
use std::sync::{Arc, Mutex};
|
||||
use std::sync::{Arc, Mutex, OnceLock};
|
||||
use tokio::sync::Semaphore;
|
||||
|
||||
use log::{debug, error, info, warn};
|
||||
use serde::{Deserialize, Serialize};
|
||||
@@ -1343,6 +1344,25 @@ pub async fn thumbnail_delete_item(
|
||||
thumbnail_cache.0.delete_item(db_service, &item_id).await
|
||||
}
|
||||
|
||||
fn mime_from_ext(ext: Option<&str>) -> &'static str {
|
||||
match ext {
|
||||
Some("jpg") | Some("jpeg") => "image/jpeg",
|
||||
Some("png") => "image/png",
|
||||
Some("gif") => "image/gif",
|
||||
Some("webp") => "image/webp",
|
||||
_ => "image/jpeg",
|
||||
}
|
||||
}
|
||||
|
||||
/// Limit concurrent image downloads to avoid saturating the connection pool.
|
||||
/// Without this, rendering a page with hundreds of album cards fires hundreds of
|
||||
/// concurrent HTTP requests, starving API calls and causing timeouts.
|
||||
static IMAGE_DOWNLOAD_SEMAPHORE: OnceLock<Semaphore> = OnceLock::new();
|
||||
|
||||
fn image_semaphore() -> &'static Semaphore {
|
||||
IMAGE_DOWNLOAD_SEMAPHORE.get_or_init(|| Semaphore::new(6))
|
||||
}
|
||||
|
||||
/// Request to get an image URL (with caching)
|
||||
#[derive(Debug, Deserialize)]
|
||||
#[serde(rename_all = "camelCase")]
|
||||
@@ -1385,32 +1405,21 @@ pub async fn image_get_url(
|
||||
&request.image_type,
|
||||
tag,
|
||||
).await {
|
||||
// Read file and return as base64
|
||||
let image_data = fs::read(&cached_path)
|
||||
.map_err(|e| format!("Failed to read cached image: {}", e))?;
|
||||
let base64_data = BASE64.encode(&image_data);
|
||||
|
||||
// Determine MIME type from file extension
|
||||
let mime_type = match cached_path.extension().and_then(|s| s.to_str()) {
|
||||
Some("jpg") | Some("jpeg") => "image/jpeg",
|
||||
Some("png") => "image/png",
|
||||
Some("gif") => "image/gif",
|
||||
Some("webp") => "image/webp",
|
||||
_ => "image/jpeg", // default
|
||||
};
|
||||
|
||||
let data_url = format!("data:{};base64,{}", mime_type, base64_data);
|
||||
debug!("[ImageCache] Cache hit for {}/{}", request.item_id, request.image_type);
|
||||
return Ok(data_url);
|
||||
let mime_type = mime_from_ext(cached_path.extension().and_then(|s| s.to_str()));
|
||||
return Ok(format!("data:{};base64,{}", mime_type, base64_data));
|
||||
}
|
||||
|
||||
// Not cached - need to fetch from repository and cache
|
||||
info!("[ImageCache] Cache miss for {}/{}, downloading...", request.item_id, request.image_type);
|
||||
// Not cached — fetch from server and cache.
|
||||
// Acquire semaphore to limit concurrent downloads (prevents connection pool starvation).
|
||||
let _permit = image_semaphore().acquire().await
|
||||
.map_err(|_| "Image download semaphore closed".to_string())?;
|
||||
|
||||
let repository = repository_manager.0.get(&repository_handle)
|
||||
.ok_or("Repository not found - user may need to log in")?;
|
||||
.ok_or_else(|| "Repository not found - user may need to log in".to_string())?;
|
||||
|
||||
// Parse image type
|
||||
let image_type_enum = match request.image_type.as_str() {
|
||||
"Primary" => ImageType::Primary,
|
||||
"Backdrop" => ImageType::Backdrop,
|
||||
@@ -1420,7 +1429,6 @@ pub async fn image_get_url(
|
||||
_ => ImageType::Primary,
|
||||
};
|
||||
|
||||
// Build image options
|
||||
let options = ImageOptions {
|
||||
max_width: request.max_width,
|
||||
max_height: request.max_height,
|
||||
@@ -1428,18 +1436,10 @@ pub async fn image_get_url(
|
||||
tag: request.tag.clone(),
|
||||
};
|
||||
|
||||
// Get image URL from repository
|
||||
let server_url = repository.get_image_url(&request.item_id, image_type_enum, Some(options));
|
||||
debug!("[ImageCache] Server URL: {}", server_url);
|
||||
|
||||
// Download image data
|
||||
let worker = ThumbnailWorker::new();
|
||||
let image_data = worker
|
||||
.download_with_retry(&server_url, 2)
|
||||
.await
|
||||
let image_data = repository.download_bytes(&server_url).await
|
||||
.map_err(|e| format!("Failed to download image: {}", e))?;
|
||||
|
||||
// Save to cache
|
||||
let cached_path = thumbnail_cache.0.save_thumbnail(
|
||||
db_service,
|
||||
&request.item_id,
|
||||
@@ -1450,18 +1450,9 @@ pub async fn image_get_url(
|
||||
request.max_height.map(|h| h as i32),
|
||||
).await?;
|
||||
|
||||
// Return as base64 data URL
|
||||
let base64_data = BASE64.encode(&image_data);
|
||||
let mime_type = match cached_path.extension().and_then(|s| s.to_str()) {
|
||||
Some("jpg") | Some("jpeg") => "image/jpeg",
|
||||
Some("png") => "image/png",
|
||||
Some("gif") => "image/gif",
|
||||
Some("webp") => "image/webp",
|
||||
_ => "image/jpeg",
|
||||
};
|
||||
let data_url = format!("data:{};base64,{}", mime_type, base64_data);
|
||||
info!("[ImageCache] Cached and returning base64 data URL");
|
||||
Ok(data_url)
|
||||
let mime_type = mime_from_ext(cached_path.extension().and_then(|s| s.to_str()));
|
||||
Ok(format!("data:{};base64,{}", mime_type, base64_data))
|
||||
}
|
||||
|
||||
// =============================================================================
|
||||
|
||||
@@ -21,6 +21,7 @@ impl DownloadWorker {
|
||||
pub fn new() -> Self {
|
||||
let client = reqwest::Client::builder()
|
||||
.timeout(Duration::from_secs(300)) // 5 minute timeout
|
||||
.https_only(true)
|
||||
.build()
|
||||
.expect("Failed to create HTTP client");
|
||||
|
||||
@@ -31,14 +32,18 @@ impl DownloadWorker {
|
||||
}
|
||||
|
||||
/// Download a file with retry logic and progress tracking
|
||||
pub async fn download(
|
||||
pub async fn download<F>(
|
||||
&self,
|
||||
task: &DownloadTask,
|
||||
) -> Result<DownloadResult, DownloadError> {
|
||||
on_progress: F,
|
||||
) -> Result<DownloadResult, DownloadError>
|
||||
where
|
||||
F: Fn(u64, Option<u64>) + Send + Sync,
|
||||
{
|
||||
let mut retries = 0;
|
||||
|
||||
loop {
|
||||
match self.try_download(task).await {
|
||||
match self.try_download(task, &on_progress).await {
|
||||
Ok(result) => return Ok(result),
|
||||
Err(e) if retries < self.max_retries && e.is_retryable() => {
|
||||
retries += 1;
|
||||
@@ -55,7 +60,10 @@ impl DownloadWorker {
|
||||
}
|
||||
|
||||
/// Attempt a single download
|
||||
async fn try_download(&self, task: &DownloadTask) -> Result<DownloadResult, DownloadError> {
|
||||
async fn try_download<F>(&self, task: &DownloadTask, on_progress: &F) -> Result<DownloadResult, DownloadError>
|
||||
where
|
||||
F: Fn(u64, Option<u64>) + Send + Sync,
|
||||
{
|
||||
// Create parent directories
|
||||
if let Some(parent) = task.target_path.parent() {
|
||||
fs::create_dir_all(parent)
|
||||
@@ -129,7 +137,7 @@ impl DownloadWorker {
|
||||
|| downloaded % (1024 * 1024) == 0
|
||||
{
|
||||
last_progress_emit = std::time::Instant::now();
|
||||
// Progress events will be emitted by the manager
|
||||
on_progress(downloaded, _total_bytes);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -20,6 +20,7 @@ impl JellyfinClient {
|
||||
pub fn new(config: JellyfinConfig) -> Result<Self, String> {
|
||||
let http_client = Client::builder()
|
||||
.timeout(std::time::Duration::from_secs(10))
|
||||
.https_only(true)
|
||||
.build()
|
||||
.map_err(|e| format!("Failed to create HTTP client: {}", e))?;
|
||||
|
||||
|
||||
@@ -49,6 +49,7 @@ impl HttpClient {
|
||||
pub fn new(config: HttpConfig) -> Result<Self, String> {
|
||||
let client = Client::builder()
|
||||
.timeout(config.timeout)
|
||||
.https_only(true)
|
||||
.build()
|
||||
.map_err(|e| format!("Failed to create HTTP client: {}", e))?;
|
||||
|
||||
|
||||
@@ -37,6 +37,12 @@ impl HybridRepository {
|
||||
}
|
||||
}
|
||||
|
||||
/// Download raw bytes from a URL using the shared authenticated HTTP client.
|
||||
/// Delegates to online repository for connection reuse and proper auth.
|
||||
pub async fn download_bytes(&self, url: &str) -> Result<Vec<u8>, String> {
|
||||
self.online.download_bytes(url).await
|
||||
}
|
||||
|
||||
/// Get video stream URL with optional seeking support.
|
||||
/// This method is online-only since offline playback uses local file paths.
|
||||
pub async fn get_video_stream_url(
|
||||
|
||||
@@ -36,6 +36,29 @@ impl OnlineRepository {
|
||||
HttpClient::build_auth_header(Some(&self.access_token), "jellytau-device")
|
||||
}
|
||||
|
||||
/// Download raw bytes from a URL using the shared authenticated HTTP client.
|
||||
/// Used by thumbnail cache to download images with proper auth and connection reuse.
|
||||
pub async fn download_bytes(&self, url: &str) -> Result<Vec<u8>, String> {
|
||||
let request = self.http_client.client.get(url)
|
||||
.header("X-Emby-Authorization", self.auth_header())
|
||||
.build()
|
||||
.map_err(|e| format!("Failed to build request: {}", e))?;
|
||||
|
||||
let response = self.http_client.request_with_retry(request).await
|
||||
.map_err(|e| format!("Download failed: {}", e))?;
|
||||
|
||||
if !response.status().is_success() {
|
||||
let status = response.status();
|
||||
let body = response.text().await.unwrap_or_default();
|
||||
let body_preview = if body.len() > 200 { &body[..200] } else { &body };
|
||||
return Err(format!("HTTP {} ({})", status, body_preview.trim()));
|
||||
}
|
||||
|
||||
response.bytes().await
|
||||
.map(|b| b.to_vec())
|
||||
.map_err(|e| format!("Failed to read bytes: {}", e))
|
||||
}
|
||||
|
||||
/// Make authenticated GET request
|
||||
async fn get_json<T: for<'de> Deserialize<'de>>(&self, endpoint: &str) -> Result<T, RepoError> {
|
||||
let url = format!("{}{}", self.server_url, endpoint);
|
||||
@@ -1005,8 +1028,12 @@ impl MediaRepository for OnlineRepository {
|
||||
image_type.as_str()
|
||||
);
|
||||
|
||||
// Authentication is handled by X-Emby-Authorization header in download_bytes()
|
||||
// Do NOT include api_key here — some Jellyfin servers reject requests when
|
||||
// api_key is present but the token doesn't match the expected format.
|
||||
let mut params: Vec<String> = Vec::new();
|
||||
|
||||
if let Some(opts) = options {
|
||||
let mut params = Vec::new();
|
||||
if let Some(width) = opts.max_width {
|
||||
params.push(format!("maxWidth={}", width));
|
||||
}
|
||||
@@ -1019,11 +1046,11 @@ impl MediaRepository for OnlineRepository {
|
||||
if let Some(tag) = opts.tag {
|
||||
params.push(format!("tag={}", tag));
|
||||
}
|
||||
}
|
||||
|
||||
if !params.is_empty() {
|
||||
url.push('?');
|
||||
url.push_str(¶ms.join("&"));
|
||||
}
|
||||
if !params.is_empty() {
|
||||
url.push('?');
|
||||
url.push_str(¶ms.join("&"));
|
||||
}
|
||||
|
||||
url
|
||||
|
||||
@@ -30,7 +30,8 @@ mod tests {
|
||||
self.server_url, item_id, image_type
|
||||
);
|
||||
|
||||
let mut params = vec![("api_key", self.access_token.clone())];
|
||||
// No api_key — image downloads use X-Emby-Authorization header
|
||||
let mut params: Vec<(&str, String)> = Vec::new();
|
||||
|
||||
if let Some(opts) = options {
|
||||
if let Some(max_width) = opts.max_width {
|
||||
@@ -304,14 +305,11 @@ mod tests {
|
||||
let subtitle_url = repo.get_subtitle_url("item123", "src123", 0, "vtt");
|
||||
let download_url = repo.get_video_download_url("item123", "720p");
|
||||
|
||||
// These URLs are constructed in BACKEND and returned to frontend
|
||||
// Frontend never receives this token directly
|
||||
assert!(image_url.contains("api_key=super_secret_token"));
|
||||
// Image URLs no longer contain api_key — auth is via X-Emby-Authorization header
|
||||
assert!(!image_url.contains("api_key="));
|
||||
// Subtitle and download URLs still use api_key (used directly, not via download_bytes)
|
||||
assert!(subtitle_url.contains("api_key=super_secret_token"));
|
||||
assert!(download_url.contains("api_key=super_secret_token"));
|
||||
|
||||
// In actual implementation, frontend would only get the URL string
|
||||
// Frontend cannot construct its own URLs or extract the token
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -335,10 +333,10 @@ mod tests {
|
||||
|
||||
let url = repo.get_image_url("id123", "Primary", None);
|
||||
|
||||
// Should be valid format
|
||||
// Should be valid format (no api_key — auth via header)
|
||||
assert!(url.starts_with("https://server.com"));
|
||||
assert!(url.contains("/Items/id123/Images/Primary"));
|
||||
assert!(url.contains("?api_key="));
|
||||
assert!(!url.contains("api_key="));
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -353,13 +351,13 @@ mod tests {
|
||||
|
||||
let url = repo.get_image_url("id123", "Primary", Some(&options));
|
||||
|
||||
// Should have single ? separator
|
||||
// Should have single ? separator with params
|
||||
let question_marks = url.matches('?').count();
|
||||
assert_eq!(question_marks, 1);
|
||||
|
||||
// Should have ampersands between params
|
||||
assert!(url.contains("?"));
|
||||
assert!(url.contains("&"));
|
||||
// Should have params for maxWidth and maxHeight
|
||||
assert!(url.contains("maxWidth=300"));
|
||||
assert!(url.contains("maxHeight=200"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -368,8 +366,7 @@ mod tests {
|
||||
|
||||
let url = repo.get_image_url("item-with-special_chars", "Primary", None);
|
||||
|
||||
// Should handle special characters in token and id
|
||||
assert!(url.contains("token_with_special-chars"));
|
||||
// Should handle special characters in id (no token in URL anymore)
|
||||
assert!(url.contains("item-with-special_chars"));
|
||||
}
|
||||
|
||||
@@ -383,9 +380,9 @@ mod tests {
|
||||
// Backend generates full URL with credentials
|
||||
let url = repo.get_image_url("item123", "Primary", None);
|
||||
|
||||
// URL is complete and ready to use
|
||||
// URL is complete and ready to use (auth via header, not api_key)
|
||||
assert!(url.starts_with("https://"));
|
||||
assert!(url.contains("api_key="));
|
||||
assert!(url.contains("/Items/item123/Images/Primary"));
|
||||
|
||||
// Frontend never constructs URLs directly
|
||||
// Frontend only receives pre-constructed URLs from backend
|
||||
@@ -408,7 +405,6 @@ mod tests {
|
||||
assert!(url.contains("maxHeight=200"));
|
||||
assert!(url.contains("quality=90"));
|
||||
assert!(url.contains("tag=abc"));
|
||||
assert!(url.contains("api_key=token"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -423,8 +419,8 @@ mod tests {
|
||||
|
||||
let url = repo.get_image_url("item123", "Primary", Some(&options));
|
||||
|
||||
// Should only have api_key
|
||||
assert!(url.contains("api_key=token"));
|
||||
// Should have no query params (no api_key, no options)
|
||||
assert!(!url.contains("?"));
|
||||
assert!(!url.contains("maxWidth"));
|
||||
assert!(!url.contains("maxHeight"));
|
||||
assert!(!url.contains("quality"));
|
||||
|
||||
@@ -12,6 +12,7 @@ impl ThumbnailWorker {
|
||||
pub fn new() -> Self {
|
||||
let client = reqwest::Client::builder()
|
||||
.timeout(Duration::from_secs(30))
|
||||
.https_only(true)
|
||||
.build()
|
||||
.expect("Failed to create HTTP client");
|
||||
|
||||
|
||||
Reference in New Issue
Block a user