From 6cf360bce0a9c6f6deca3a901630764b124a967c Mon Sep 17 00:00:00 2001 From: NimBold Date: Mon, 15 Jun 2026 11:53:24 +0330 Subject: [PATCH] fix: address codebase review issues - refactor(backend): use tokio::sync::Mutex for DbState and update commands to async - fix(backend): remove unconditional post-queue system action in scheduler - refactor(backend): remove dead WebSocket aria2 progress loop - fix(backend): use character count for deep link payload length check - fix(backend): implement dynamic port fallback for extension server - build(backend): apply macos codesigning step for release builds - security(backend): add explicitly defined Content-Security-Policy - fix(frontend): replace pause_download API call with remove_download for file cleanup - fix(frontend): resolve bug ignoring 0% progress reporting - fix(frontend): append instead of overwrite deep link URLs when Add Modal is open - style(frontend): append standard .dark class for dark mode themes - style(frontend): remove ghost row layout hack from download table - build: decouple typescript binding generation from build step --- package.json | 2 +- src-tauri/src/db.rs | 2 +- src-tauri/src/extension_server.rs | 13 +- src-tauri/src/lib.rs | 208 +++--------------------------- src-tauri/src/scheduler.rs | 10 +- src-tauri/tauri.conf.json | 2 +- src/App.tsx | 4 + src/components/DownloadTable.tsx | 12 +- src/store/useDownloadStore.ts | 22 ++-- 9 files changed, 53 insertions(+), 222 deletions(-) diff --git a/package.json b/package.json index 33343d0..784d278 100644 --- a/package.json +++ b/package.json @@ -29,7 +29,7 @@ "scripts": { "dev": "vite", "bindings": "cd src-tauri && cargo test export_bindings --lib", - "build": "npm run bindings && tsc && vite build", + "build": "tsc && vite build", "preview": "vite preview", "tauri": "tauri" }, diff --git a/src-tauri/src/db.rs b/src-tauri/src/db.rs index 57f7bf5..924c89a 100644 --- a/src-tauri/src/db.rs +++ b/src-tauri/src/db.rs @@ -1,5 +1,5 @@ use rusqlite::{Connection, Result, params}; -use std::sync::Mutex; +use tokio::sync::Mutex; use std::path::PathBuf; use tauri::Manager; diff --git a/src-tauri/src/extension_server.rs b/src-tauri/src/extension_server.rs index 9712882..4fe44e4 100644 --- a/src-tauri/src/extension_server.rs +++ b/src-tauri/src/extension_server.rs @@ -81,9 +81,16 @@ pub async fn start_server( .layer(cors) .with_state(state); - let listener = tokio::net::TcpListener::bind(("127.0.0.1", EXTENSION_SERVER_PORT)) - .await - .map_err(|e| format!("Failed to bind to {}: {}", EXTENSION_SERVER_PORT, e))?; + let mut listener = None; + for port in EXTENSION_SERVER_PORT..=(EXTENSION_SERVER_PORT + 10) { + if let Ok(l) = tokio::net::TcpListener::bind(("127.0.0.1", port)).await { + listener = Some((l, port)); + break; + } + } + + let (listener, bound_port) = listener.ok_or_else(|| "Failed to bind extension server to any port".to_string())?; + println!("Browser extension server bound to 127.0.0.1:{}", bound_port); axum::serve(listener, app) .await diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 961b73c..96848be 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -303,7 +303,7 @@ impl Drop for Aria2DaemonGuard { } } -#[cfg(all(target_os = "macos", debug_assertions))] +#[cfg(target_os = "macos")] fn resign_aria2_debug_bundle(aria2c_path: &std::path::Path) -> Result<(), String> { let lib_dir = aria2c_path .parent() @@ -397,7 +397,7 @@ fn parse_firelink_urls(deep_links: impl IntoIterator) -> Vec= MAX_DEEP_LINK_PAYLOAD_LEN { + if raw_urls.is_empty() || raw_urls.chars().count() >= MAX_DEEP_LINK_PAYLOAD_LEN { continue; } @@ -1255,7 +1255,7 @@ pub fn run() { Err(error) => eprintln!("Failed to read startup deep link: {error}"), } let db_conn = crate::db::init_db(app.handle()).expect("Failed to init db"); - app.manage(crate::db::DbState { conn: std::sync::Mutex::new(db_conn) }); + app.manage(crate::db::DbState { conn: tokio::sync::Mutex::new(db_conn) }); crate::scheduler::spawn_scheduler(app.handle().clone()); @@ -1289,181 +1289,15 @@ pub fn run() { Err(e) => eprintln!("Failed to spawn aria2c daemon: {}", e), } - let app_handle_clone = app.handle().clone(); - let aria2_port_clone = aria2_port; - let aria2_secret_clone = aria2_secret.clone(); - tauri::async_runtime::spawn(async move { - tokio::time::sleep(std::time::Duration::from_millis(500)).await; - let ws_url = format!("ws://127.0.0.1:{}/jsonrpc", aria2_port_clone); - - use futures_util::{StreamExt, SinkExt}; - use tokio_tungstenite::connect_async; - use tokio_tungstenite::tungstenite::Message; - let mut connection = None; - for attempt in 1..=20 { - match connect_async(&ws_url).await { - Ok(stream) => { - connection = Some(stream); - break; - } - Err(error) if attempt < 20 => { - if let Some(status) = app_handle_clone - .state::() - .0 - .lock() - .ok() - .and_then(|mut guard| guard.as_mut().and_then(|child| child.try_wait().ok()).flatten()) - { - eprintln!("aria2 daemon exited before RPC startup: {}", status); - return; - } - tokio::time::sleep(std::time::Duration::from_millis(250)).await; - if attempt == 1 { - eprintln!("Waiting for aria2 WebSocket: {}", error); - } - } - Err(error) => { - eprintln!("Failed to connect to aria2 WebSocket after retries: {}", error); - } - } - } - let Some((mut ws_stream, _)) = connection else { - return; - }; - - let mut interval = tokio::time::interval(std::time::Duration::from_millis(1000)); - - loop { - tokio::select! { - _ = interval.tick() => { - let req = serde_json::json!({ - "jsonrpc": "2.0", - "id": "progress", - "method": "aria2.tellActive", - "params": [ - format!("token:{}", aria2_secret_clone), - ["gid", "status", "completedLength", "totalLength", "downloadSpeed"] - ] - }); - if let Ok(msg) = serde_json::to_string(&req) { - let _ = ws_stream.send(Message::Text(msg.into())).await; - } - } - msg = ws_stream.next() => { - match msg { - Some(Ok(Message::Text(text))) => { - if let Ok(json) = serde_json::from_str::(text.as_str()) { - let tasks = HashMap::::new(); - - // Process progress - if json.get("id").and_then(|i| i.as_str()) == Some("progress") { - let mut gid_to_id = HashMap::new(); - { - for (id, handle) in tasks.iter() { - if let TaskHandle::Aria2(gid) = handle { - gid_to_id.insert(gid.clone(), id.clone()); - } - } - } - - if let Some(arr) = json.get("result").and_then(|r| r.as_array()) { - for item in arr { - if let Some(gid) = item.get("gid").and_then(|v| v.as_str()) { - if let Some(id) = gid_to_id.get(gid) { - let completed = item.get("completedLength").and_then(|v| v.as_str()).and_then(|s| s.parse::().ok()).unwrap_or(0.0); - let total = item.get("totalLength").and_then(|v| v.as_str()).and_then(|s| s.parse::().ok()).unwrap_or(1.0); - let speed_bytes = item.get("downloadSpeed").and_then(|v| v.as_str()).and_then(|s| s.parse::().ok()).unwrap_or(0.0); - - let fraction = if total > 0.0 { completed / total } else { 0.0 }; - let speed = if speed_bytes > 1024.0 * 1024.0 { - format!("{:.1} MB/s", speed_bytes / (1024.0 * 1024.0)) - } else if speed_bytes > 1024.0 { - format!("{:.1} KB/s", speed_bytes / 1024.0) - } else { - format!("{:.0} B/s", speed_bytes) - }; - - let eta = if speed_bytes > 0.0 && total > completed { - let seconds = (total - completed) / speed_bytes; - if seconds > 3600.0 { - format!("{:.0}h {:.0}m", seconds / 3600.0, (seconds % 3600.0) / 60.0) - } else if seconds > 60.0 { - format!("{:.0}m {:.0}s", seconds / 60.0, seconds % 60.0) - } else { - format!("{:.0}s", seconds) - } - } else { - "-".to_string() - }; - - let _ = app_handle_clone.emit("download-progress", DownloadProgressEvent { - id: id.clone(), - fraction, - speed, - eta, - }); - } - } - } - } - } - - // Process Events - if let Some(method) = json.get("method").and_then(|m| m.as_str()) { - if method == "aria2.onDownloadComplete" || method == "aria2.onDownloadError" { - if let Some(params) = json.get("params").and_then(|p| p.as_array()) { - if let Some(event_info) = params.get(0) { - if let Some(gid) = event_info.get("gid").and_then(|g| g.as_str()) { - let mut target_id = None; - { - for (id, handle) in tasks.iter() { - if let TaskHandle::Aria2(task_gid) = handle { - if task_gid == gid { - target_id = Some(id.clone()); - break; - } - } - } - } - - if let Some(id) = target_id { - if method == "aria2.onDownloadComplete" { - let _ = app_handle_clone.emit("download-complete", id.clone()); - } else { - let _ = app_handle_clone.emit("download-failed", id.clone()); - } - } - } - } - } - } - } - } - } - Some(Err(e)) => { - eprintln!("WebSocket error: {}", e); - break; - } - None => break, // Stream closed - _ => {} // Ignore binary/ping/pong for now - } - } - } - } - }); let ext_app_handle = app.handle().clone(); tauri::async_runtime::spawn(async move { - match extension_server::start_server( + if let Err(error) = extension_server::start_server( ext_app_handle, server_pairing_token.clone(), server_frontend_ready.clone(), ).await { - Ok(()) => println!( - "Browser extension server listening on 127.0.0.1:{}", - extension_server::EXTENSION_SERVER_PORT - ), - Err(error) => eprintln!("Browser extension server unavailable: {error}"), + eprintln!("Browser extension server unavailable: {error}"); } }); Ok(()) @@ -1500,50 +1334,50 @@ mod db; mod scheduler; #[tauri::command] -fn db_save_settings(state: tauri::State, data: String) -> Result<(), String> { - let conn = state.conn.lock().unwrap(); +async fn db_save_settings(state: tauri::State<'_, crate::db::DbState>, data: String) -> Result<(), String> { + let conn = state.conn.lock().await; crate::db::save_settings(&conn, &data).map_err(|e| e.to_string()) } #[tauri::command] -fn db_load_settings(state: tauri::State) -> Result, String> { - let conn = state.conn.lock().unwrap(); +async fn db_load_settings(state: tauri::State<'_, crate::db::DbState>) -> Result, String> { + let conn = state.conn.lock().await; crate::db::get_settings(&conn).map_err(|e| e.to_string()) } #[tauri::command] -fn db_get_all_downloads(state: tauri::State) -> Result, String> { - let conn = state.conn.lock().unwrap(); +async fn db_get_all_downloads(state: tauri::State<'_, crate::db::DbState>) -> Result, String> { + let conn = state.conn.lock().await; crate::db::get_all_downloads(&conn).map_err(|e| e.to_string()) } #[tauri::command] -fn db_save_download(state: tauri::State, id: String, status: crate::ipc::DownloadStatus, queue_id: String, data: String) -> Result<(), String> { - let conn = state.conn.lock().unwrap(); +async fn db_save_download(state: tauri::State<'_, crate::db::DbState>, id: String, status: crate::ipc::DownloadStatus, queue_id: String, data: String) -> Result<(), String> { + let conn = state.conn.lock().await; crate::db::insert_download(&conn, &id, status.as_str(), &queue_id, &data) .map_err(|e| e.to_string()) } #[tauri::command] -fn db_delete_download(state: tauri::State, id: String) -> Result<(), String> { - let conn = state.conn.lock().unwrap(); +async fn db_delete_download(state: tauri::State<'_, crate::db::DbState>, id: String) -> Result<(), String> { + let conn = state.conn.lock().await; crate::db::delete_download(&conn, &id).map_err(|e| e.to_string()) } #[tauri::command] -fn db_get_all_queues(state: tauri::State) -> Result, String> { - let conn = state.conn.lock().unwrap(); +async fn db_get_all_queues(state: tauri::State<'_, crate::db::DbState>) -> Result, String> { + let conn = state.conn.lock().await; crate::db::get_all_queues(&conn).map_err(|e| e.to_string()) } #[tauri::command] -fn db_save_queue(state: tauri::State, id: String, data: String) -> Result<(), String> { - let conn = state.conn.lock().unwrap(); +async fn db_save_queue(state: tauri::State<'_, crate::db::DbState>, id: String, data: String) -> Result<(), String> { + let conn = state.conn.lock().await; crate::db::insert_queue(&conn, &id, &data).map_err(|e| e.to_string()) } #[tauri::command] -fn db_delete_queue(state: tauri::State, id: String) -> Result<(), String> { - let conn = state.conn.lock().unwrap(); +async fn db_delete_queue(state: tauri::State<'_, crate::db::DbState>, id: String) -> Result<(), String> { + let conn = state.conn.lock().await; crate::db::delete_queue(&conn, &id).map_err(|e| e.to_string()) } diff --git a/src-tauri/src/scheduler.rs b/src-tauri/src/scheduler.rs index ded0268..e5b947d 100644 --- a/src-tauri/src/scheduler.rs +++ b/src-tauri/src/scheduler.rs @@ -12,7 +12,7 @@ pub fn spawn_scheduler(app_handle: tauri::AppHandle) { let settings_opt = { let state = app_handle.state::(); - let conn = state.conn.lock().unwrap(); + let conn = state.conn.lock().await; crate::db::get_settings(&conn).unwrap_or(None) }; @@ -46,7 +46,7 @@ pub fn spawn_scheduler(app_handle: tauri::AppHandle) { if let Ok(updated) = serde_json::to_string(&settings) { let state = app_handle.state::(); - let conn = state.conn.lock().unwrap(); + let conn = state.conn.lock().await; let _ = crate::db::save_settings(&conn, &updated); } } @@ -59,13 +59,9 @@ pub fn spawn_scheduler(app_handle: tauri::AppHandle) { if let Ok(updated) = serde_json::to_string(&settings) { let state = app_handle.state::(); - let conn = state.conn.lock().unwrap(); + let conn = state.conn.lock().await; let _ = crate::db::save_settings(&conn, &updated); } - - if !matches!(scheduler.post_queue_action, crate::ipc::PostQueueAction::None) { - let _ = crate::execute_system_action(scheduler.post_queue_action.clone()); - } } } } diff --git a/src-tauri/tauri.conf.json b/src-tauri/tauri.conf.json index af1cb35..2ee2d64 100644 --- a/src-tauri/tauri.conf.json +++ b/src-tauri/tauri.conf.json @@ -31,7 +31,7 @@ } ], "security": { - "csp": null + "csp": "default-src 'self'; img-src 'self' data: https:; script-src 'self'; style-src 'self' 'unsafe-inline'; connect-src 'self' ws://localhost:* http://localhost:* http://127.0.0.1:* ws://127.0.0.1:*" }, "macOSPrivateApi": true }, diff --git a/src/App.tsx b/src/App.tsx index dbd8722..91cf5b2 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -162,8 +162,12 @@ function App() { if (theme === 'system') { const systemDark = window.matchMedia('(prefers-color-scheme: dark)').matches; root.classList.add(systemDark ? 'theme-dark' : 'theme-light'); + if (systemDark) root.classList.add('dark'); } else { root.classList.add(`theme-${theme}`); + if (['dark', 'dracula', 'nord'].includes(theme)) { + root.classList.add('dark'); + } } }; diff --git a/src/components/DownloadTable.tsx b/src/components/DownloadTable.tsx index 37a3b0a..b85f0bb 100644 --- a/src/components/DownloadTable.tsx +++ b/src/components/DownloadTable.tsx @@ -306,17 +306,7 @@ export const DownloadTable: React.FC = ({ filter }) => { ))} -
- {Array.from({ length: 50 }).map((_, index) => { - const isEven = (filteredDownloads.length + index) % 2 === 1; - return ( -
- ); - })} -
+
diff --git a/src/store/useDownloadStore.ts b/src/store/useDownloadStore.ts index 389eaf8..f93626a 100644 --- a/src/store/useDownloadStore.ts +++ b/src/store/useDownloadStore.ts @@ -99,11 +99,15 @@ export const useDownloadStore = create((set, get) => ({ pendingAddReferer: '', pendingAddFilename: '' }), - openAddModalWithUrls: (urls, referer, filename) => set({ - isAddModalOpen: true, - pendingAddUrls: urls, - pendingAddReferer: referer?.trim() || '', - pendingAddFilename: filename?.trim() || '' + openAddModalWithUrls: (urls, referer, filename) => set((state) => { + const existingUrls = state.isAddModalOpen && state.pendingAddUrls ? state.pendingAddUrls : ''; + const mergedUrls = existingUrls ? `${existingUrls}\n${urls}` : urls; + return { + isAddModalOpen: true, + pendingAddUrls: mergedUrls, + pendingAddReferer: referer?.trim() || state.pendingAddReferer || '', + pendingAddFilename: filename?.trim() || state.pendingAddFilename || '' + }; }), handleExtensionDownload: (request) => { const urls = [...new Set(request.urls.map(url => url.trim()).filter(Boolean))]; @@ -161,14 +165,10 @@ export const useDownloadStore = create((set, get) => ({ set((state) => ({ downloads: state.downloads.map(d => { if (d.id === id) { - let newFraction = updates.fraction; - if (newFraction === 0 && d.fraction && d.fraction > 0) { - newFraction = d.fraction; - } const updated = { ...d, ...updates, - fraction: newFraction !== undefined ? newFraction : updates.fraction !== undefined ? updates.fraction : d.fraction + fraction: updates.fraction !== undefined ? updates.fraction : d.fraction }; updatedItem = updated; return updated; @@ -195,7 +195,7 @@ export const useDownloadStore = create((set, get) => ({ const item = get().downloads.find(d => d.id === id); if (item && item.status === 'downloading') { try { - await invoke('pause_download', { id }); + await invoke('remove_download', { id, filepath: item.destination || null }); } catch (e) { console.error("Failed to terminate download on deletion:", e); }