From 79c0e48c4381032fcc4ae88a8d47c9c3e18ec4f9 Mon Sep 17 00:00:00 2001 From: NimBold Date: Mon, 3 Aug 2026 03:56:15 +0330 Subject: [PATCH] fix(torrents): harden removal reservation recovery --- TORRENT_FEATURES.md | 22 ++- src-tauri/Cargo.lock | 10 + src-tauri/Cargo.toml | 1 + src-tauri/src/db.rs | 279 +++++++++++++++++++++++++++- src-tauri/src/download_ownership.rs | 31 +++- src-tauri/src/lib.rs | 233 +++++++---------------- src-tauri/src/platform.rs | 50 ++++- src-tauri/src/queue.rs | 42 ++--- src/ipc.ts | 2 + src/store/useDownloadStore.test.ts | 32 ++++ src/store/useDownloadStore.ts | 22 +++ 11 files changed, 508 insertions(+), 216 deletions(-) diff --git a/TORRENT_FEATURES.md b/TORRENT_FEATURES.md index 78f5de2..c977946 100644 --- a/TORRENT_FEATURES.md +++ b/TORRENT_FEATURES.md @@ -11,7 +11,7 @@ Reference: [Aria2 1.37.0 manual](https://aria2.github.io/manual/en/html/aria2c.h ## Audit basis -- Audited on 2026-08-03 at Firelink `cba485e` (`main`) plus the current working +- Audited on 2026-08-03 at Firelink `32034e9` (`main`) plus the current working tree, with the cumulative Torrent work reviewed from `edc76a7`. - Source of truth: `src-tauri/src/torrent.rs`, `torrent_probe.rs`, `queue.rs`, @@ -98,8 +98,13 @@ Reference: [Aria2 1.37.0 manual](https://aria2.github.io/manual/en/html/aria2c.h - Validated encryption policies mapped consistently to `bt-force-encryption`, `bt-require-crypto`, and `bt-min-crypto-level`. - Optional `bt-remove-unselected-file` cleanup after successful completion, - only with an explicit partial selection and confirmation. Cancellation, - failure, replacement, and cleanup races are conservative. + only with an explicit partial selection and confirmation. The selected-file + ownership and unselected-file reservation are committed atomically; Firelink + clears the reservation only after Aria2's reserved paths are absent, + including when a transfer fails. Restart recovery preserves queued/paused + and orphaned reservations while reclaiming only observed failed or completed + cleanup. Disabling the option after a detach clears the reservation before + the edited item is persisted. ### Trackers, peers, and network identity @@ -124,8 +129,9 @@ Reference: [Aria2 1.37.0 manual](https://aria2.github.io/manual/en/html/aria2c.h ### Evidence already present in the tree - Rust unit coverage for bencode/hash/path validation, option normalization, - queue ownership, lifecycle fencing, persistence sanitization, and native - startup argument construction. + queue ownership, lifecycle fencing, persistence sanitization, atomic Torrent + removal reservations, conservative restart recovery, host case-insensitive + path identity, and native startup argument construction. - `src-tauri/tests/torrent_rpc.rs` covers the production authenticated JSON-RPC HTTP boundary in a Windows-compatible integration-test target. - `npm run smoke:torrent` and `npm run smoke:torrent:failure-paths` cover @@ -175,11 +181,7 @@ Before any new Torrent feature is promoted, keep these gates mandatory: ### Tier 1 — high-value user behavior -1. **Unselected-file removal crash/restart audit.** Add post-crash tests around - the persisted removal reservation, Aria2 completion cleanup, path reuse, and - case-insensitive path equality. Do not change cleanup ordering until the - ownership postconditions are proven. -2. **DHT routing-table persistence policy.** Decide and implement app-managed +1. **DHT routing-table persistence policy.** Decide and implement app-managed `dht-file-path`/`dht-file-path6` behavior, especially for portable mode, permissions, reset, and privacy. This should be opt-in if it expands data retention beyond the current download metadata contract. diff --git a/src-tauri/Cargo.lock b/src-tauri/Cargo.lock index 51d3a7e..4db3a78 100644 --- a/src-tauri/Cargo.lock +++ b/src-tauri/Cargo.lock @@ -1406,6 +1406,7 @@ dependencies = [ "tower-http 0.7.0", "trash", "ts-rs", + "unicode-normalization", "url", "uuid", "windows-native-keyring-store", @@ -5725,6 +5726,15 @@ version = "1.0.24" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e6e4313cd5fcd3dad5cafa179702e2b244f760991f45397d14d4ebf38247da75" +[[package]] +name = "unicode-normalization" +version = "0.1.25" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5fd4f6878c9cb28d874b009da9e8d183b5abc80117c40bbd187a1fde336be6e8" +dependencies = [ + "tinyvec", +] + [[package]] name = "unicode-segmentation" version = "1.13.3" diff --git a/src-tauri/Cargo.toml b/src-tauri/Cargo.toml index 9990e28..237cbfd 100644 --- a/src-tauri/Cargo.toml +++ b/src-tauri/Cargo.toml @@ -65,6 +65,7 @@ keyring-core = "1.0.0" [target.'cfg(target_os = "macos")'.dependencies] apple-native-keyring-store = { version = "1.0.1", features = ["keychain"] } objc = "0.2.7" +unicode-normalization = "0.1.25" [target.'cfg(target_os = "windows")'.dependencies] windows-native-keyring-store = "1.1.0" diff --git a/src-tauri/src/db.rs b/src-tauri/src/db.rs index 78f2fae..984fd2d 100644 --- a/src-tauri/src/db.rs +++ b/src-tauri/src/db.rs @@ -1,7 +1,7 @@ use rusqlite::{params, Connection, OptionalExtension, Transaction}; use serde_json::Value; use std::fs; -use std::path::{Path, PathBuf}; +use std::path::{Component, Path, PathBuf}; use std::sync::Mutex; const DATABASE_NAME: &str = "firelink.sqlite"; @@ -1356,9 +1356,15 @@ pub fn set_ownership_and_removal_paths( paths: &[String], removal_paths: &[String], ) -> Result<(), String> { - set_ownership_paths_checked(connection, id, primary_path, paths, removal_paths)?; + // Ownership and the planned deletion reservation are one safety + // boundary. Do not leave a partially-written reservation behind if the + // second table write fails or the process crashes between writes. + let transaction = connection + .unchecked_transaction() + .map_err(|error| format!("failed to begin torrent ownership transaction: {error}"))?; + set_ownership_paths_checked(&transaction, id, primary_path, paths, removal_paths)?; if removal_paths.is_empty() { - connection + transaction .execute( "DELETE FROM download_removal_paths WHERE id = ?1", params![id], @@ -1367,7 +1373,7 @@ pub fn set_ownership_and_removal_paths( } else { let encoded_paths = serde_json::to_string(removal_paths) .map_err(|error| format!("failed to encode torrent removal paths: {error}"))?; - connection + transaction .execute( "INSERT INTO download_removal_paths (id, paths) VALUES (?1, ?2) ON CONFLICT(id) DO UPDATE SET paths = excluded.paths", @@ -1375,7 +1381,89 @@ pub fn set_ownership_and_removal_paths( ) .map_err(|error| format!("failed to save torrent removal paths: {error}"))?; } - Ok(()) + transaction + .commit() + .map_err(|error| format!("failed to commit torrent ownership transaction: {error}")) +} + +/// Reclaim reservations left behind by a process crash after a terminal +/// Torrent outcome. Active, queued, and paused records remain reserved: a +/// future lifecycle may still ask Aria2 to remove those paths. A terminal +/// record is reclaimed only after every reserved path is absent, proving that +/// Aria2's unselected-file cleanup was observed. Failed records are treated +/// conservatively as well because Aria2 may leave unselected files behind on +/// an error. +pub fn reconcile_torrent_removal_paths_after_restart( + connection: &Connection, +) -> Result { + let mut statement = connection + .prepare( + "SELECT removal.id, removal.paths, downloads.status + FROM download_removal_paths AS removal + LEFT JOIN downloads ON downloads.id = removal.id", + ) + .map_err(|error| format!("failed to prepare torrent removal recovery query: {error}"))?; + let rows = statement + .query_map([], |row| { + let id: String = row.get(0)?; + let paths: String = row.get(1)?; + let status: Option = row.get(2)?; + Ok((id, paths, status)) + }) + .map_err(|error| format!("failed to query torrent removal recovery data: {error}"))?; + + let mut reclaim = Vec::new(); + for row in rows { + let (id, encoded_paths, status) = + row.map_err(|error| format!("failed to read torrent removal recovery data: {error}"))?; + let paths = match serde_json::from_str::>(&encoded_paths) { + Ok(paths) + if !paths.is_empty() + && paths.iter().all(|path| { + let path = Path::new(path); + path.is_absolute() + && !path.components().any(|component| { + matches!(component, Component::CurDir | Component::ParentDir) + }) + }) => + { + paths + } + // Malformed or empty reservations are retained for conservative + // manual recovery rather than being silently discarded. + _ => continue, + }; + let Some(status) = status else { + continue; + }; + let should_reclaim = match status.as_str() { + "failed" | "completed" => paths + .iter() + .all(|path| torrent_removal_path_is_absent(Path::new(path))), + _ => false, + }; + if should_reclaim { + reclaim.push(id); + } + } + + let mut reclaimed = 0; + for id in reclaim { + reclaimed += connection + .execute( + "DELETE FROM download_removal_paths WHERE id = ?1", + params![id], + ) + .map_err(|error| format!("failed to reclaim torrent removal paths: {error}"))?; + } + Ok(reclaimed) +} + +fn torrent_removal_path_is_absent(path: &Path) -> bool { + matches!( + fs::symlink_metadata(path), + Err(error) if error.kind() == std::io::ErrorKind::NotFound + ) } pub fn remove_ownership(connection: &Connection, id: &str) -> Result<(), String> { @@ -2587,4 +2675,185 @@ mod tests { ) .unwrap(); } + + #[test] + fn torrent_ownership_and_removal_reservation_commit_atomically() { + let temp = TempDir::new().unwrap(); + let state = init_at_path(temp.path()).unwrap(); + let connection = state.lock().unwrap(); + connection + .execute_batch( + "CREATE TRIGGER reject_torrent_removal + BEFORE INSERT ON download_removal_paths + BEGIN SELECT RAISE(ABORT, 'test rejection'); END;", + ) + .unwrap(); + + let error = set_ownership_and_removal_paths( + &connection, + "torrent", + "/downloads/selected.bin", + &["/downloads/selected.bin".to_string()], + &["/downloads/unselected.bin".to_string()], + ) + .expect_err("the injected reservation failure must abort the transaction"); + + assert!(error.contains("test rejection")); + assert!(load_ownership(&connection).unwrap().is_empty()); + assert!(load_torrent_removal_paths(&connection, "torrent") + .unwrap() + .is_empty()); + } + + #[test] + fn restart_reclaims_only_observed_terminal_torrent_reservations() { + let temp = TempDir::new().unwrap(); + let state = init_at_path(temp.path()).unwrap(); + let connection = state.lock().unwrap(); + let completed_missing = temp.path().join("completed-missing.bin"); + let completed_present = temp.path().join("completed-present.bin"); + let failed_present = temp.path().join("failed-present.bin"); + let queued_missing = temp.path().join("queued-missing.bin"); + fs::write(&completed_present, b"old").unwrap(); + fs::write(&failed_present, b"old").unwrap(); + + for (id, status, path) in [ + ("completed-missing", "completed", &completed_missing), + ("completed-present", "completed", &completed_present), + ("failed-present", "failed", &failed_present), + ("queued-missing", "queued", &queued_missing), + ] { + let selected_path = temp.path().join(format!("{id}-selected.bin")); + set_ownership_and_removal_paths( + &connection, + id, + &selected_path.to_string_lossy(), + &[selected_path.to_string_lossy().to_string()], + &[path.to_string_lossy().to_string()], + ) + .unwrap(); + let record = json!({ + "id": id, + "status": status, + "isTorrent": true, + "torrentRemoveUnselectedFile": true + }) + .to_string(); + connection + .execute( + "INSERT INTO downloads (id, status, queue_id, data) VALUES (?1, ?2, ?3, ?4)", + params![id, status, "main", record], + ) + .unwrap(); + } + + assert_eq!( + reconcile_torrent_removal_paths_after_restart(&connection).unwrap(), + 1 + ); + assert!(load_torrent_removal_paths(&connection, "completed-missing") + .unwrap() + .is_empty()); + assert_eq!( + load_torrent_removal_paths(&connection, "failed-present").unwrap(), + vec![failed_present.to_string_lossy().to_string()] + ); + assert_eq!( + load_torrent_removal_paths(&connection, "completed-present") + .unwrap(), + vec![completed_present.to_string_lossy().to_string()] + ); + assert_eq!( + load_torrent_removal_paths(&connection, "queued-missing") + .unwrap(), + vec![queued_missing.to_string_lossy().to_string()] + ); + + set_ownership_paths( + &connection, + "replacement", + &completed_missing.to_string_lossy(), + &[completed_missing.to_string_lossy().to_string()], + ) + .unwrap(); + + let error = set_ownership_paths( + &connection, + "failed-replacement", + &failed_present.to_string_lossy(), + &[failed_present.to_string_lossy().to_string()], + ) + .expect_err("an unobserved failed Torrent cleanup must keep its path reserved"); + assert!(error.contains("already owned")); + } + + #[test] + fn restart_keeps_orphaned_torrent_removal_reservations_conservative() { + let temp = TempDir::new().unwrap(); + let state = init_at_path(temp.path()).unwrap(); + let connection = state.lock().unwrap(); + let reserved = temp.path().join("orphaned-unselected.bin"); + set_ownership_and_removal_paths( + &connection, + "orphaned", + &temp.path().join("orphaned-selected.bin").to_string_lossy(), + &[temp + .path() + .join("orphaned-selected.bin") + .to_string_lossy() + .to_string()], + &[reserved.to_string_lossy().to_string()], + ) + .unwrap(); + + assert_eq!( + reconcile_torrent_removal_paths_after_restart(&connection).unwrap(), + 0 + ); + assert_eq!( + load_torrent_removal_paths(&connection, "orphaned").unwrap(), + vec![reserved.to_string_lossy().to_string()] + ); + } + + #[test] + fn restart_keeps_malformed_torrent_removal_reservations_conservative() { + let temp = TempDir::new().unwrap(); + let state = init_at_path(temp.path()).unwrap(); + let connection = state.lock().unwrap(); + for (id, encoded_paths) in [ + ("empty", "[]"), + ("relative", r#"["relative-file.bin"]"#), + ("empty-path", r#"[""]"#), + ] { + connection + .execute( + "INSERT INTO download_removal_paths (id, paths) VALUES (?1, ?2)", + params![id, encoded_paths], + ) + .unwrap(); + connection + .execute( + "INSERT INTO downloads (id, status, queue_id, data) VALUES + (?1, 'completed', 'main', '{}')", + params![id], + ) + .unwrap(); + } + + assert_eq!( + reconcile_torrent_removal_paths_after_restart(&connection).unwrap(), + 0 + ); + assert_eq!( + connection + .query_row( + "SELECT COUNT(*) FROM download_removal_paths", + [], + |row| row.get::<_, i64>(0), + ) + .unwrap(), + 3 + ); + } } diff --git a/src-tauri/src/download_ownership.rs b/src-tauri/src/download_ownership.rs index a772043..72db05f 100644 --- a/src-tauri/src/download_ownership.rs +++ b/src-tauri/src/download_ownership.rs @@ -110,16 +110,6 @@ pub fn expected_primary_path( .ok_or_else(|| "Download path could not be canonicalized".to_string()) } -pub fn register_expected( - app_handle: &tauri::AppHandle, - id: &str, - destination: &str, - filename: &str, -) -> Result<(), String> { - let path = expected_primary_path(app_handle, destination, filename)?; - set_primary_path(app_handle, id, &path) -} - pub fn set_primary_path( app_handle: &tauri::AppHandle, id: &str, @@ -292,6 +282,27 @@ pub fn clear_torrent_removal_paths( crate::db::remove_torrent_removal_paths(&connection, id) } +/// Clear a Torrent removal reservation only after every reserved path is +/// absent. The reservation protects paths that Aria2 may still remove after +/// a terminal event has been observed; callers must not release it merely +/// because the daemon reported completion or failure. +pub fn clear_torrent_removal_paths_if_absent( + app_handle: &tauri::AppHandle, + id: &str, +) -> Result { + let paths = torrent_removal_paths_for_id(app_handle, id)?; + if paths.iter().any(|path| { + !matches!( + std::fs::symlink_metadata(path), + Err(error) if error.kind() == std::io::ErrorKind::NotFound + ) + }) { + return Ok(false); + } + clear_torrent_removal_paths(app_handle, id)?; + Ok(true) +} + pub fn torrent_removal_paths_for_id( app_handle: &tauri::AppHandle, id: &str, diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 12801c0..1423887 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -5951,6 +5951,41 @@ fn expected_torrent_output_paths( })) } +/// Register the complete output ownership for one enqueue in a single +/// ownership transaction. In particular, do not first register only the +/// primary path and then add Torrent paths: a crash between those writes can +/// leave a partial ownership record that no longer describes the queued task. +fn register_download_ownership( + app_handle: &tauri::AppHandle, + item: &queue::EnqueueItem, +) -> Result<(), String> { + let torrent_paths = expected_torrent_output_paths(app_handle, item)?; + let primary = crate::download_ownership::expected_primary_path( + app_handle, + &item.destination, + &item.filename, + )?; + let (owned_paths, removal_paths) = match torrent_paths { + Some(paths) => ( + paths.selected, + if item.torrent_remove_unselected_file.unwrap_or(false) { + paths.unselected + } else { + Vec::new() + }, + ), + None => (vec![primary.clone()], Vec::new()), + }; + + crate::download_ownership::set_owned_paths_with_primary_and_removal( + app_handle, + &item.id, + &primary, + &owned_paths, + &removal_paths, + ) +} + async fn remove_magnet_metadata_probe_dir(path: &std::path::Path) -> Result<(), String> { match tokio::fs::remove_dir_all(path).await { Ok(()) => Ok(()), @@ -6199,81 +6234,14 @@ async fn enqueue_download( .reserve_enqueue_generation(&id, lifecycle_generation) .await .map_err(AppError::Internal)?; - if let Err(error) = crate::download_ownership::register_expected( - &app_handle, - &item.id, - &item.destination, - &item.filename, - ) { + if let Err(error) = register_download_ownership(&app_handle, &item) { + let _ = crate::download_ownership::remove(&app_handle, &id); state .queue_manager .rollback_enqueue_reservation(&id, lifecycle_generation, previous_generation) .await; return Err(AppError::Internal(error)); } - match expected_torrent_output_paths(&app_handle, &item) { - Ok(Some(paths)) => { - let primary = match crate::download_ownership::expected_primary_path( - &app_handle, - &item.destination, - &item.filename, - ) { - Ok(primary) => primary, - Err(error) => { - let _ = crate::download_ownership::remove(&app_handle, &id); - state - .queue_manager - .rollback_enqueue_reservation(&id, lifecycle_generation, previous_generation) - .await; - return Err(AppError::Internal(error)); - } - }; - let ownership_result = if item.torrent_remove_unselected_file.unwrap_or(false) { - crate::download_ownership::set_owned_paths_with_primary_and_removal( - &app_handle, - &item.id, - &primary, - &paths.selected, - &paths.unselected, - ) - } else { - crate::download_ownership::set_owned_paths_with_primary( - &app_handle, - &item.id, - &primary, - &paths.selected, - ) - }; - if let Err(error) = ownership_result { - let _ = crate::download_ownership::remove(&app_handle, &id); - state - .queue_manager - .rollback_enqueue_reservation(&id, lifecycle_generation, previous_generation) - .await; - return Err(AppError::Internal(error)); - } - } - Ok(None) => {} - Err(error) => { - let _ = crate::download_ownership::remove(&app_handle, &id); - state - .queue_manager - .rollback_enqueue_reservation(&id, lifecycle_generation, previous_generation) - .await; - return Err(AppError::Internal(error)); - } - } - if !item.torrent_remove_unselected_file.unwrap_or(false) { - if let Err(error) = crate::download_ownership::clear_torrent_removal_paths(&app_handle, &id) - { - let _ = crate::download_ownership::remove(&app_handle, &id); - state - .queue_manager - .rollback_enqueue_reservation(&id, lifecycle_generation, previous_generation) - .await; - return Err(AppError::Internal(error)); - } - } if let Err(error) = state .queue_manager .commit_reserved_enqueue(item.into_task(), lifecycle_generation) @@ -6361,12 +6329,8 @@ async fn enqueue_many( continue; } }; - if let Err(error) = crate::download_ownership::register_expected( - &app_handle, - &item.id, - &item.destination, - &item.filename, - ) { + if let Err(error) = register_download_ownership(&app_handle, &item) { + let _ = crate::download_ownership::remove(&app_handle, &id); state .queue_manager .rollback_enqueue_reservation(&id, lifecycle_generation, previous_generation) @@ -6379,94 +6343,6 @@ async fn enqueue_many( }); continue; } - match expected_torrent_output_paths(&app_handle, &item) { - Ok(Some(paths)) => { - let primary = match crate::download_ownership::expected_primary_path( - &app_handle, - &item.destination, - &item.filename, - ) { - Ok(primary) => primary, - Err(error) => { - let _ = crate::download_ownership::remove(&app_handle, &id); - state - .queue_manager - .rollback_enqueue_reservation(&id, lifecycle_generation, previous_generation) - .await; - results.push(crate::ipc::EnqueueResult { - id, - success: false, - filename: None, - error: Some(error), - }); - continue; - } - }; - let ownership_result = if item.torrent_remove_unselected_file.unwrap_or(false) { - crate::download_ownership::set_owned_paths_with_primary_and_removal( - &app_handle, - &item.id, - &primary, - &paths.selected, - &paths.unselected, - ) - } else { - crate::download_ownership::set_owned_paths_with_primary( - &app_handle, - &item.id, - &primary, - &paths.selected, - ) - }; - if let Err(error) = ownership_result { - let _ = crate::download_ownership::remove(&app_handle, &id); - state - .queue_manager - .rollback_enqueue_reservation(&id, lifecycle_generation, previous_generation) - .await; - results.push(crate::ipc::EnqueueResult { - id, - success: false, - filename: None, - error: Some(error), - }); - continue; - } - } - Ok(None) => {} - Err(error) => { - let _ = crate::download_ownership::remove(&app_handle, &id); - state - .queue_manager - .rollback_enqueue_reservation(&id, lifecycle_generation, previous_generation) - .await; - results.push(crate::ipc::EnqueueResult { - id, - success: false, - filename: None, - error: Some(error), - }); - continue; - } - } - if !item.torrent_remove_unselected_file.unwrap_or(false) { - if let Err(error) = - crate::download_ownership::clear_torrent_removal_paths(&app_handle, &id) - { - let _ = crate::download_ownership::remove(&app_handle, &id); - state - .queue_manager - .rollback_enqueue_reservation(&id, lifecycle_generation, previous_generation) - .await; - results.push(crate::ipc::EnqueueResult { - id, - success: false, - filename: None, - error: Some(error), - }); - continue; - } - } if let Err(error) = state .queue_manager .commit_reserved_enqueue(item.into_task(), lifecycle_generation) @@ -7414,6 +7290,36 @@ fn db_get_all_downloads( crate::db::load_downloads(&connection) } +#[tauri::command] +async fn clear_torrent_removal_paths( + app_handle: tauri::AppHandle, + state: tauri::State<'_, AppState>, + id: String, +) -> Result<(), String> { + // This command is reached from the trusted renderer, but it still owns a + // destructive reservation boundary. Serialize it with terminal/control + // transitions and refuse to clear a lifecycle that the backend still + // owns. The frontend's registration set is only a hint and must not be + // the safety check. + let _control_guard = state.queue_manager.acquire_aria2_control(&id).await; + if state.queue_manager.aria2_gid_for_download(&id).is_some() + || state.queue_manager.is_registered(&id).await + { + return Err( + "cannot clear Torrent removal paths while the backend owns the download".to_string(), + ); + } + crate::download_ownership::clear_torrent_removal_paths(&app_handle, &id) +} + +#[tauri::command] +fn reconcile_torrent_removal_reservations( + state: tauri::State<'_, crate::db::DbState>, +) -> Result { + let connection = state.lock()?; + crate::db::reconcile_torrent_removal_paths_after_restart(&connection) +} + fn retained_torrent_id_from_persisted_record(record: &str) -> Option { let value = serde_json::from_str::(record).ok()?; let object = value.as_object()?; @@ -11433,6 +11339,7 @@ pub fn run() { parity::get_system_proxy, parity::get_file_category, parity::check_for_updates, parity::is_supported_media, parity::get_supported_media_domains, parity::create_category_directories, db_save_settings, db_load_settings, db_get_all_downloads, db_replace_downloads, + clear_torrent_removal_paths, reconcile_torrent_removal_reservations, db_get_all_queues, db_replace_queues, read_logs, export_logs, toggle_log_pause, is_log_paused, clear_logs, set_log_stream_active diff --git a/src-tauri/src/platform.rs b/src-tauri/src/platform.rs index 3f9ea9a..65d7af2 100644 --- a/src-tauri/src/platform.rs +++ b/src-tauri/src/platform.rs @@ -123,10 +123,20 @@ pub fn path_is_within(path: &Path, root: &Path) -> bool { } pub fn paths_equal(left: &Path, right: &Path) -> bool { - #[cfg(any(target_os = "windows", target_os = "macos"))] + #[cfg(target_os = "windows")] { left.to_string_lossy() - .eq_ignore_ascii_case(&right.to_string_lossy()) + .to_lowercase() + == right.to_string_lossy().to_lowercase() + } + #[cfg(target_os = "macos")] + { + use unicode_normalization::UnicodeNormalization; + + let normalize = |path: &Path| { + path.to_string_lossy().to_lowercase().nfc().collect::() + }; + normalize(left) == normalize(right) } #[cfg(not(any(target_os = "windows", target_os = "macos")))] { @@ -155,7 +165,8 @@ fn numbered_windows_device(stem: &str, prefix: &str) -> bool { #[cfg(test)] mod tests { - use super::{engine_binary_name, is_windows_reserved_filename, target_triple}; + use super::{engine_binary_name, is_windows_reserved_filename, paths_equal, target_triple}; + use std::path::Path; #[test] fn target_engine_name_uses_current_rust_target() { @@ -186,4 +197,37 @@ mod tests { assert!(!is_windows_reserved_filename(filename), "{filename}"); } } + + #[test] + fn path_identity_matches_the_host_filesystem_case_contract() { + let left = Path::new("/downloads/Selected/File.bin"); + let right = Path::new("/Downloads/selected/file.BIN"); + if cfg!(any(target_os = "windows", target_os = "macos")) { + assert!(paths_equal(left, right)); + } else { + assert!(!paths_equal(left, right)); + } + } + + #[test] + fn path_identity_handles_non_ascii_case_differences() { + let left = Path::new("/downloads/Ärt/File.bin"); + let right = Path::new("/DOWNLOADS/ärt/file.BIN"); + if cfg!(any(target_os = "windows", target_os = "macos")) { + assert!(paths_equal(left, right)); + } else { + assert!(!paths_equal(left, right)); + } + } + + #[test] + fn path_identity_handles_macos_unicode_normalization() { + let composed = Path::new("/downloads/café/File.bin"); + let decomposed = Path::new("/DOWNLOADS/cafe\u{301}/file.BIN"); + if cfg!(target_os = "macos") { + assert!(paths_equal(composed, decomposed)); + } else { + assert!(!paths_equal(composed, decomposed)); + } + } } diff --git a/src-tauri/src/queue.rs b/src-tauri/src/queue.rs index 8517bb4..a7ca7be 100644 --- a/src-tauri/src/queue.rs +++ b/src-tauri/src/queue.rs @@ -2120,29 +2120,15 @@ impl QueueManager { self.clear_aria2_retry_state(id).await; self.forget_aria2_gid(id).await; if torrent_removal_requested { - match crate::download_ownership::torrent_removal_paths_for_id( + match crate::download_ownership::clear_torrent_removal_paths_if_absent( &self.app_handle, id, ) { - Ok(paths) if paths.iter().all(|path| !path.exists()) => { - if let Err(error) = - crate::download_ownership::clear_torrent_removal_paths( - &self.app_handle, - id, - ) - { - log::warn!( - "aria2 torrent removal reservation [{}]: could not clear after completion: {}", - id, - error - ); - } - } - Ok(paths) => { + Ok(true) => {} + Ok(false) => { log::warn!( - "aria2 torrent removal reservation [{}]: keeping {} path(s) reserved because Aria2 cleanup was not observed", - id, - paths.len() + "aria2 torrent removal reservation [{}]: keeping paths reserved because Aria2 cleanup was not observed", + id ); } Err(error) => { @@ -2175,14 +2161,20 @@ impl QueueManager { self.clear_aria2_retry_state(id).await; self.forget_aria2_gid(id).await; if torrent_removal_requested { - if let Err(clear_error) = - crate::download_ownership::clear_torrent_removal_paths(&self.app_handle, id) - { - log::warn!( - "aria2 torrent removal reservation [{}]: could not clear after terminal failure: {}", + match crate::download_ownership::clear_torrent_removal_paths_if_absent( + &self.app_handle, + id, + ) { + Ok(true) => {} + Ok(false) => log::warn!( + "aria2 torrent removal reservation [{}]: keeping paths reserved because cleanup was not observed after failure", + id + ), + Err(clear_error) => log::warn!( + "aria2 torrent removal reservation [{}]: could not verify cleanup after terminal failure: {}", id, clear_error - ); + ), } } self.release_registered_id(id).await; diff --git a/src/ipc.ts b/src/ipc.ts index 23805e7..c461d91 100644 --- a/src/ipc.ts +++ b/src/ipc.ts @@ -57,6 +57,8 @@ type CommandMap = { remove_download: { args: { id: string; deleteAssets: boolean; preserveResumable?: boolean }; result: void }; get_download_primary_path: { args: { id: string }; result: string | null }; detach_download_for_reconfigure: { args: { id: string }; result: void }; + clear_torrent_removal_paths: { args: { id: string }; result: void }; + reconcile_torrent_removal_reservations: { args: undefined; result: number }; begin_dock_badge_session: { args: undefined; result: number }; update_dock_badge: { args: { count: number; generation: number; session: number }; result: void }; get_platform_info: { args: undefined; result: PlatformInfo }; diff --git a/src/store/useDownloadStore.test.ts b/src/store/useDownloadStore.test.ts index 666c288..a6957b7 100644 --- a/src/store/useDownloadStore.test.ts +++ b/src/store/useDownloadStore.test.ts @@ -135,6 +135,38 @@ describe('useDownloadStore', () => { expect(fileName.endsWith('.mp4')).toBe(true); }); + it('clears a persisted Torrent removal reservation when a paused item disables cleanup', async () => { + useDownloadStore.setState({ + downloads: [{ + id: 'paused-torrent-removal', + url: 'magnet:?xt=urn:btih:abc', + fileName: 'torrent', + status: 'paused', + category: 'Other', + dateAdded: '', + isTorrent: true, + torrentFileIndices: [0], + torrentRemoveUnselectedFile: true + }] as any[], + backendRegisteredIds: new Set(['paused-torrent-removal']) + }); + vi.mocked(ipc.invokeCommand).mockResolvedValue(undefined as never); + + await useDownloadStore.getState().applyProperties('paused-torrent-removal', { + torrentRemoveUnselectedFile: false + }); + + expect(ipc.invokeCommand).toHaveBeenCalledWith( + 'detach_download_for_reconfigure', + { id: 'paused-torrent-removal' } + ); + expect(ipc.invokeCommand).toHaveBeenCalledWith( + 'clear_torrent_removal_paths', + { id: 'paused-torrent-removal' } + ); + expect(useDownloadStore.getState().downloads[0].torrentRemoveUnselectedFile).toBe(false); + }); + it('replaces stale media intent when an appended handoff reuses a URL', () => { useDownloadStore.getState().openAddModalWithUrls( 'https://example.com/file.bin', '', '', '', '', true diff --git a/src/store/useDownloadStore.ts b/src/store/useDownloadStore.ts index cdbdea8..efc3989 100644 --- a/src/store/useDownloadStore.ts +++ b/src/store/useDownloadStore.ts @@ -913,12 +913,18 @@ export const useDownloadStore = create((set, get) => { const normalizedUpdates = updates.fileName === undefined ? updates : { ...updates, fileName: canonicalizeDownloadFileName(updates.fileName) }; + const disablingTorrentRemoval = item.isTorrent === true + && normalizedUpdates.torrentRemoveUnselectedFile === false + && item.torrentRemoveUnselectedFile !== false; if (item.status === 'downloading' || item.status === 'processing' || item.status === 'seeding' || item.status === 'retrying') { throw new Error(i18n.t($ => $.downloadTable.transferActive)); } if (item.status === 'ready' || item.status === 'staged' || item.status === 'completed' || item.status === 'failed') { + if (disablingTorrentRemoval) { + await invoke('clear_torrent_removal_paths', { id }); + } state.updateDownload(id, normalizedUpdates); return; } @@ -932,6 +938,9 @@ export const useDownloadStore = create((set, get) => { state.unregisterBackendIds([id]); set(current => ({ pendingOrder: current.pendingOrder.filter(value => value !== id) })); } + if (disablingTorrentRemoval) { + await invoke('clear_torrent_removal_paths', { id }); + } state.updateDownload(id, normalizedUpdates); if (isRegistered || wasDispatching) { const dispatched = await dispatchItemInternal(id); @@ -951,6 +960,9 @@ export const useDownloadStore = create((set, get) => { } state.unregisterBackendIds([id]); } + if (disablingTorrentRemoval) { + await invoke('clear_torrent_removal_paths', { id }); + } state.updateDownload(id, normalizedUpdates); } }; @@ -2386,6 +2398,16 @@ export const useDownloadStore = create((set, get) => { : state.downloads })); + // A process can die after Aria2 has removed the unselected files but + // before the terminal event clears Firelink's reservation. Reclaim + // only the conservative terminal cases in the backend before queued + // downloads are allowed to claim paths on startup. + try { + await invoke('reconcile_torrent_removal_reservations'); + } catch (error) { + console.warn('Could not reconcile Torrent removal reservations during startup:', error); + } + // The backend dispatcher is live before the frontend finishes startup. // Synchronize the normalized queue policy before any saved download is // allowed to claim a permit.