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
This commit is contained in:
NimBold
2026-06-15 11:53:24 +03:30
parent 2ddcd2d99a
commit 6cf360bce0
9 changed files with 53 additions and 222 deletions
+1 -1
View File
@@ -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"
},
+1 -1
View File
@@ -1,5 +1,5 @@
use rusqlite::{Connection, Result, params};
use std::sync::Mutex;
use tokio::sync::Mutex;
use std::path::PathBuf;
use tauri::Manager;
+10 -3
View File
@@ -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
+21 -187
View File
@@ -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<Item = url::Url>) -> Vec<St
else {
continue;
};
if raw_urls.is_empty() || raw_urls.len() >= 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::<Aria2DaemonGuard>()
.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::<serde_json::Value>(text.as_str()) {
let tasks = HashMap::<String, TaskHandle>::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::<f64>().ok()).unwrap_or(0.0);
let total = item.get("totalLength").and_then(|v| v.as_str()).and_then(|s| s.parse::<f64>().ok()).unwrap_or(1.0);
let speed_bytes = item.get("downloadSpeed").and_then(|v| v.as_str()).and_then(|s| s.parse::<f64>().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<crate::db::DbState>, 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<crate::db::DbState>) -> Result<Option<String>, String> {
let conn = state.conn.lock().unwrap();
async fn db_load_settings(state: tauri::State<'_, crate::db::DbState>) -> Result<Option<String>, 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<crate::db::DbState>) -> Result<Vec<String>, String> {
let conn = state.conn.lock().unwrap();
async fn db_get_all_downloads(state: tauri::State<'_, crate::db::DbState>) -> Result<Vec<String>, 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<crate::db::DbState>, 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<crate::db::DbState>, 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<crate::db::DbState>) -> Result<Vec<String>, String> {
let conn = state.conn.lock().unwrap();
async fn db_get_all_queues(state: tauri::State<'_, crate::db::DbState>) -> Result<Vec<String>, 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<crate::db::DbState>, 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<crate::db::DbState>, 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())
}
+3 -7
View File
@@ -12,7 +12,7 @@ pub fn spawn_scheduler(app_handle: tauri::AppHandle) {
let settings_opt = {
let state = app_handle.state::<crate::db::DbState>();
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::<crate::db::DbState>();
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::<crate::db::DbState>();
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());
}
}
}
}
+1 -1
View File
@@ -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
},
+4
View File
@@ -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');
}
}
};
+1 -11
View File
@@ -306,17 +306,7 @@ export const DownloadTable: React.FC<DownloadTableProps> = ({ filter }) => {
</div>
</div>
))}
<div className="flex-1 overflow-hidden flex flex-col pointer-events-none">
{Array.from({ length: 50 }).map((_, index) => {
const isEven = (filteredDownloads.length + index) % 2 === 1;
return (
<div
key={`ghost-${index}`}
className={`download-ghost-row ${isEven ? 'striped' : ''}`}
/>
);
})}
</div>
<div className="flex-1 bg-transparent pointer-events-none"></div>
</div>
</div>
</div>
+11 -11
View File
@@ -99,11 +99,15 @@ export const useDownloadStore = create<DownloadState>((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<DownloadState>((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<DownloadState>((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);
}