mirror of
https://github.com/nimbold/Firelink.git
synced 2026-08-07 01:44:01 +00:00
fix(torrents): harden removal reservation recovery
This commit is contained in:
+12
-10
@@ -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.
|
||||
|
||||
Generated
+10
@@ -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"
|
||||
|
||||
@@ -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"
|
||||
|
||||
+274
-5
@@ -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<usize, String> {
|
||||
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<String> = 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::<Vec<String>>(&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
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -110,16 +110,6 @@ pub fn expected_primary_path<R: tauri::Runtime>(
|
||||
.ok_or_else(|| "Download path could not be canonicalized".to_string())
|
||||
}
|
||||
|
||||
pub fn register_expected<R: tauri::Runtime>(
|
||||
app_handle: &tauri::AppHandle<R>,
|
||||
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<R: tauri::Runtime>(
|
||||
app_handle: &tauri::AppHandle<R>,
|
||||
id: &str,
|
||||
@@ -292,6 +282,27 @@ pub fn clear_torrent_removal_paths<R: tauri::Runtime>(
|
||||
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<R: tauri::Runtime>(
|
||||
app_handle: &tauri::AppHandle<R>,
|
||||
id: &str,
|
||||
) -> Result<bool, String> {
|
||||
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<R: tauri::Runtime>(
|
||||
app_handle: &tauri::AppHandle<R>,
|
||||
id: &str,
|
||||
|
||||
+70
-163
@@ -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<usize, String> {
|
||||
let connection = state.lock()?;
|
||||
crate::db::reconcile_torrent_removal_paths_after_restart(&connection)
|
||||
}
|
||||
|
||||
fn retained_torrent_id_from_persisted_record(record: &str) -> Option<String> {
|
||||
let value = serde_json::from_str::<serde_json::Value>(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
|
||||
|
||||
@@ -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::<String>()
|
||||
};
|
||||
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));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+17
-25
@@ -2120,29 +2120,15 @@ impl<R: tauri::Runtime> QueueManager<R> {
|
||||
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<R: tauri::Runtime> QueueManager<R> {
|
||||
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;
|
||||
|
||||
@@ -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 };
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -913,12 +913,18 @@ export const useDownloadStore = create<DownloadState>((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<DownloadState>((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<DownloadState>((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<DownloadState>((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.
|
||||
|
||||
Reference in New Issue
Block a user