diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 54c336e..8a8e69a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -71,6 +71,10 @@ jobs: run: | cargo test --tests --target ${{ matrix.target }} cargo test --lib --no-run --target ${{ matrix.target }} + - name: Verify Windows atomic Torrent storage + if: runner.os == 'Windows' + working-directory: src-tauri + run: cargo test --test atomic_file --target ${{ matrix.target }} -- --nocapture - name: Provision locked engines if: runner.os != 'macOS' run: node scripts/provision-engines.js --target ${{ matrix.target }} diff --git a/src-tauri/Cargo.lock b/src-tauri/Cargo.lock index 09aa467..8606045 100644 --- a/src-tauri/Cargo.lock +++ b/src-tauri/Cargo.lock @@ -1441,6 +1441,7 @@ dependencies = [ "url", "uuid", "windows-native-keyring-store", + "windows-sys 0.61.2", "zbus-secret-service-keyring-store", ] diff --git a/src-tauri/Cargo.toml b/src-tauri/Cargo.toml index 237cbfd..6ece027 100644 --- a/src-tauri/Cargo.toml +++ b/src-tauri/Cargo.toml @@ -69,6 +69,7 @@ unicode-normalization = "0.1.25" [target.'cfg(target_os = "windows")'.dependencies] windows-native-keyring-store = "1.1.0" +windows-sys = { version = "0.61.2", features = ["Win32_Foundation", "Win32_Storage_FileSystem"] } [target.'cfg(target_os = "linux")'.dependencies] zbus-secret-service-keyring-store = { version = "1.0.0", features = ["crypto-rust"] } diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 6f089e1..e620bf6 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -3567,6 +3567,8 @@ pub mod ipc; mod parity; mod power; mod platform; +#[doc(hidden)] +pub use platform::atomic_write_replace; mod properties_window; pub mod queue; pub mod process; @@ -6797,7 +6799,7 @@ async fn validate_enqueue_uris(url: &str, mirrors: Option<&str>) -> Result<(), S Ok(()) } -async fn validate_torrent_web_seed_destinations(seeds: &[String]) -> Result<(), String> { +pub(crate) async fn validate_torrent_web_seed_destinations(seeds: &[String]) -> Result<(), String> { for uri in seeds { let parsed = reqwest::Url::parse(uri) .map_err(|_| "Torrent web-seed URI is invalid".to_string())?; @@ -6877,7 +6879,9 @@ async fn validate_torrent_enqueue( item.torrent_encryption_policy = queue::normalize_torrent_encryption_policy( item.torrent_encryption_policy.as_deref(), )?; - validate_enqueue_uris("", item.mirrors.as_deref()).await?; + let legacy_web_seeds = queue::normalize_torrent_mirror_uris(item.mirrors.as_deref())?; + validate_torrent_web_seed_destinations(&legacy_web_seeds).await?; + item.mirrors = (!legacy_web_seeds.is_empty()).then_some(legacy_web_seeds.join("\n")); if let Some(path) = item.torrent_path.as_deref() { let path = crate::torrent::validate_managed_torrent_path(app_handle, &item.id, path)?; let bytes = std::fs::read(path) @@ -8443,11 +8447,7 @@ async fn write_torrent_move_journal( }); let bytes = serde_json::to_vec_pretty(&data) .map_err(|_| "could not encode Torrent move journal".to_string())?; - let temporary = path.with_extension("json.tmp"); - tokio::fs::write(&temporary, bytes) - .await - .map_err(|_| "could not write Torrent move journal".to_string())?; - tokio::fs::rename(&temporary, path) + crate::platform::atomic_write_replace(path, &bytes) .await .map_err(|_| "could not commit Torrent move journal".to_string()) } @@ -8671,6 +8671,18 @@ fn recover_torrent_move_journals( Err(_) => continue, }; let path = entry.path(); + let name = entry.file_name(); + let name = name.to_string_lossy(); + let file_type = match entry.file_type() { + Ok(file_type) => file_type, + Err(_) => continue, + }; + if crate::platform::is_atomic_temp_file_name(&name) { + if file_type.is_file() || file_type.is_symlink() { + let _ = std::fs::remove_file(&path); + } + continue; + } if path.extension().and_then(|value| value.to_str()) != Some("json") { continue; } @@ -11728,7 +11740,7 @@ mod tests { #[test] fn renderer_download_snapshots_cannot_roll_back_native_web_seed_changes() { - let native_seeds = json!([{ "fileIndex": 0, "uri": "https://mirror.example/file" }]); + let native_seeds = json!([{ "fileIndex": 1, "uri": "https://mirror.example/file" }]); let existing = vec![ json!({ "id": "torrent-1", diff --git a/src-tauri/src/platform.rs b/src-tauri/src/platform.rs index 65d7af2..5e26273 100644 --- a/src-tauri/src/platform.rs +++ b/src-tauri/src/platform.rs @@ -1,6 +1,141 @@ use std::ffi::OsString; +use std::io; use std::path::{Path, PathBuf}; +const ATOMIC_TEMP_PREFIX: &str = ".firelink-atomic-"; + +/// Write bytes to a same-directory temporary file, synchronize them, and +/// replace the destination without ever opening the destination for writing. +/// +/// The destination is checked with `symlink_metadata` so managed callers fail +/// closed when an attacker or another process has substituted a link or a +/// non-file. The final rename is atomic on Unix and uses Windows replace +/// semantics rather than the non-replacing `std::fs::rename` behavior. +pub async fn atomic_write_replace(path: &Path, bytes: &[u8]) -> io::Result<()> { + let parent = path + .parent() + .ok_or_else(|| io::Error::new(io::ErrorKind::InvalidInput, "atomic path has no parent"))?; + + match tokio::fs::symlink_metadata(path).await { + Ok(metadata) if metadata.file_type().is_symlink() => { + return Err(io::Error::new( + io::ErrorKind::PermissionDenied, + "atomic destination cannot be a symbolic link", + )); + } + Ok(metadata) if !metadata.file_type().is_file() => { + return Err(io::Error::new( + io::ErrorKind::AlreadyExists, + "atomic destination is not a regular file", + )); + } + Ok(_) => {} + Err(error) if error.kind() == io::ErrorKind::NotFound => {} + Err(error) => return Err(error), + } + + let temporary = parent.join(format!( + "{ATOMIC_TEMP_PREFIX}{}.tmp", + uuid::Uuid::new_v4().simple() + )); + let write_result = async { + use tokio::io::AsyncWriteExt; + + let mut file = tokio::fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(&temporary) + .await?; + file.write_all(bytes).await?; + file.sync_all().await + } + .await; + + if let Err(error) = write_result { + let _ = tokio::fs::remove_file(&temporary).await; + return Err(error); + } + + if let Err(error) = replace_staged_file(&temporary, path) { + let _ = tokio::fs::remove_file(&temporary).await; + return Err(error); + } + + #[cfg(unix)] + { + // A directory sync makes the rename durable across a power loss on + // platforms that support opening directories as file descriptors. + std::fs::File::open(parent)?.sync_all()?; + } + + Ok(()) +} + +pub fn is_atomic_temp_file_name(name: &str) -> bool { + let Some(suffix) = name.strip_prefix(ATOMIC_TEMP_PREFIX) else { + return false; + }; + let Some(identifier) = suffix.strip_suffix(".tmp") else { + return false; + }; + identifier.len() == 32 && identifier.bytes().all(|byte| byte.is_ascii_hexdigit()) +} + +fn replace_staged_file(temporary: &Path, destination: &Path) -> io::Result<()> { + #[cfg(not(target_os = "windows"))] + { + std::fs::rename(temporary, destination) + } + + #[cfg(target_os = "windows")] + { + use std::os::windows::ffi::OsStrExt; + use std::thread; + use std::time::Duration; + use windows_sys::Win32::Foundation::{ + GetLastError, ERROR_LOCK_VIOLATION, ERROR_SHARING_VIOLATION, + }; + use windows_sys::Win32::Storage::FileSystem::{ + MoveFileExW, MOVEFILE_REPLACE_EXISTING, MOVEFILE_WRITE_THROUGH, + }; + + let temporary = temporary + .as_os_str() + .encode_wide() + .chain(std::iter::once(0)) + .collect::>(); + let destination = destination + .as_os_str() + .encode_wide() + .chain(std::iter::once(0)) + .collect::>(); + + for attempt in 0..5 { + // SAFETY: both paths are NUL-terminated UTF-16 buffers owned for + // the duration of the call, and the flags request same-volume + // replacement with write-through semantics. + let replaced = unsafe { + MoveFileExW( + temporary.as_ptr(), + destination.as_ptr(), + MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH, + ) + }; + if replaced != 0 { + return Ok(()); + } + + let error = unsafe { GetLastError() }; + if !matches!(error, ERROR_LOCK_VIOLATION | ERROR_SHARING_VIOLATION) || attempt == 4 { + return Err(io::Error::from_raw_os_error(error as i32)); + } + thread::sleep(Duration::from_millis(25 * (attempt + 1) as u64)); + } + + unreachable!("atomic Windows replacement loop always returns"); + } +} + pub fn target_arch() -> &'static str { if cfg!(target_arch = "aarch64") { "aarch64" diff --git a/src-tauri/src/queue.rs b/src-tauri/src/queue.rs index ba62cdf..f92a76c 100644 --- a/src-tauri/src/queue.rs +++ b/src-tauri/src/queue.rs @@ -172,7 +172,7 @@ pub fn normalize_torrent_dht_message_timeout(value: u32) -> Result Ok(value) } -fn normalize_torrent_web_seed_uri(value: &str) -> Result { +pub(crate) fn normalize_torrent_web_seed_uri(value: &str) -> Result { let value = value.trim(); if value.is_empty() || value.len() > MAX_TORRENT_WEB_SEED_URI_LENGTH { return Err(format!( @@ -223,6 +223,24 @@ pub fn normalize_torrent_web_seeds( Ok(normalized) } +pub(crate) fn normalize_torrent_mirror_uris( + mirrors: Option<&str>, +) -> Result, String> { + let mut normalized = Vec::new(); + for uri in crate::collect_download_uris("", mirrors) { + let uri = normalize_torrent_web_seed_uri(&uri)?; + if !normalized.iter().any(|existing| existing == &uri) { + if normalized.len() >= MAX_TORRENT_WEB_SEEDS { + return Err(format!( + "a Torrent may have at most {MAX_TORRENT_WEB_SEEDS} fallback web seeds" + )); + } + normalized.push(uri); + } + } + Ok(normalized) +} + fn expand_torrent_web_seed_uri( seed: &crate::ipc::TorrentWebSeed, files: &[crate::ipc::TorrentFile], @@ -262,6 +280,17 @@ pub fn expand_torrent_web_seeds( .collect() } +fn expected_initial_torrent_web_seed_uris( + current: &[String], + explicit: &[(u32, String)], +) -> HashSet { + current + .iter() + .cloned() + .chain(explicit.iter().map(|(_, uri)| uri.clone())) + .collect() +} + fn parse_aria2_web_seed_uris(value: &serde_json::Value) -> Result, String> { let entries = value .as_array() @@ -2425,6 +2454,7 @@ impl QueueManager { return Err("Torrent lifecycle changed while reading initial web seeds".to_string()); } let expanded = expand_torrent_web_seeds(&desired, &files)?; + let expected = expected_initial_torrent_web_seed_uris(¤t, &expanded); let mut current_set = current.into_iter().collect::>(); let mut changes = Vec::<(u32, Vec, Vec)>::new(); for (file_index, uri) in &expanded { @@ -2473,7 +2503,6 @@ impl QueueManager { .await; return Err("Torrent lifecycle changed while reading initial web seeds".to_string()); } - let expected = expanded.into_iter().map(|(_, uri)| uri).collect::>(); if readback.into_iter().collect::>() != expected { self.rollback_torrent_web_seed_changes(&id, &gid, &mapping, &changes) .await; @@ -5970,8 +5999,8 @@ const MAX_TORRENT_MAX_PEERS: u32 = 1000; pub(crate) const MAX_TORRENT_STOP_TIMEOUT: u32 = 7 * 24 * 60 * 60; pub(crate) const MAX_TORRENT_PEER_DIAGNOSTICS: usize = 128; const MAX_TORRENT_PEER_RESPONSE: usize = 4096; -const MAX_TORRENT_TRACKERS: usize = 64; -const MAX_TORRENT_TRACKER_BYTES: usize = 16 * 1024; +pub(crate) const MAX_TORRENT_TRACKERS: usize = 64; +pub(crate) const MAX_TORRENT_TRACKER_BYTES: usize = 16 * 1024; fn apply_aria2_connection_options( options: &mut serde_json::Map, @@ -6620,6 +6649,30 @@ pub(crate) fn parse_torrent_piece_progress( }) } +pub(crate) fn normalize_torrent_tracker_uri(value: &str) -> Result { + let token = value.trim(); + if token.is_empty() { + return Err("torrent tracker URI is empty".to_string()); + } + if token.chars().any(char::is_control) { + return Err("torrent tracker URI contains a control character".to_string()); + } + let parsed = url::Url::parse(token).map_err(|_| "torrent tracker URI is invalid".to_string())?; + if !matches!(parsed.scheme(), "http" | "https" | "udp") { + return Err("torrent tracker URI must use http, https, or udp".to_string()); + } + if parsed.host_str().is_none_or(str::is_empty) { + return Err("torrent tracker URI must include a host".to_string()); + } + if !parsed.username().is_empty() || parsed.password().is_some() { + return Err("torrent tracker URI must not contain credentials".to_string()); + } + if parsed.fragment().is_some() { + return Err("torrent tracker URI must not contain a fragment".to_string()); + } + Ok(parsed.to_string()) +} + fn normalize_torrent_tracker_list( value: Option<&str>, allow_wildcard: bool, @@ -6646,9 +6699,6 @@ fn normalize_torrent_tracker_list( if token.is_empty() { return Err("torrent tracker list contains an empty entry".to_string()); } - if token.chars().any(char::is_control) { - return Err("torrent tracker URI contains a control character".to_string()); - } if allow_wildcard && token == "*" { if !trackers.is_empty() { return Err( @@ -6665,22 +6715,7 @@ fn normalize_torrent_tracker_list( .to_string(), ); } - let parsed = url::Url::parse(token) - .map_err(|_| "torrent tracker URI is invalid".to_string())?; - if !matches!(parsed.scheme(), "http" | "https" | "udp") { - return Err("torrent tracker URI must use http, https, or udp".to_string()); - } - if parsed.host_str().is_none_or(str::is_empty) { - return Err("torrent tracker URI must include a host".to_string()); - } - if !parsed.username().is_empty() || parsed.password().is_some() { - return Err("torrent tracker URI must not contain credentials".to_string()); - } - if parsed.fragment().is_some() { - return Err("torrent tracker URI must not contain a fragment".to_string()); - } - - let normalized = parsed.to_string(); + let normalized = normalize_torrent_tracker_uri(token)?; if trackers.iter().any(|tracker| tracker == &normalized) { continue; } @@ -7219,18 +7254,13 @@ impl SidecarSpawner for ProductionSpawner { ); } let encoded = base64::engine::general_purpose::STANDARD.encode(sanitized_bytes); + let fallback_web_seeds = + normalize_torrent_mirror_uris(payload.mirrors.as_deref())?; + crate::validate_torrent_web_seed_destinations(&fallback_web_seeds).await?; let mut uris = embedded_web_seeds; - if payload.torrent_web_seeds.is_none() { - uris.extend( - payload - .mirrors - .as_deref() - .map(|mirrors| crate::collect_download_uris("", Some(mirrors))) - .unwrap_or_default(), - ); - uris.sort(); - uris.dedup(); - } + uris.extend(fallback_web_seeds); + uris.sort(); + uris.dedup(); ("aria2.addTorrent", serde_json::json!([encoded, uris, options])) } else { let parsed = url::Url::parse(&payload.url) @@ -8601,6 +8631,46 @@ mod tests { assert!(normalize_torrent_trackers(Some(&too_many)).is_err()); } + #[test] + fn torrent_fallback_mirrors_use_the_dedicated_http_policy() { + assert_eq!( + normalize_torrent_mirror_uris(Some( + " https://mirror.example/one\nhttps://mirror.example/one\nhttps://mirror.example/two " + )) + .unwrap(), + vec![ + "https://mirror.example/one".to_string(), + "https://mirror.example/two".to_string() + ] + ); + for value in [ + "ftp://mirror.example/file", + "sftp://mirror.example/file", + "https://user:pass@mirror.example/file", + "https://mirror.example/file#fragment", + ] { + assert!(normalize_torrent_mirror_uris(Some(value)).is_err(), "{value}"); + } + } + + #[test] + fn initial_torrent_web_seed_readback_keeps_existing_and_explicit_uris() { + let current = vec![ + "https://embedded.example/file".to_string(), + "https://legacy.example/file".to_string(), + ]; + let explicit = vec![(1, "https://explicit.example/file".to_string())]; + + assert_eq!( + expected_initial_torrent_web_seed_uris(¤t, &explicit), + HashSet::from([ + "https://embedded.example/file".to_string(), + "https://legacy.example/file".to_string(), + "https://explicit.example/file".to_string(), + ]) + ); + } + #[test] fn torrent_exclude_trackers_support_wildcard_and_normalized_uris() { assert_eq!(normalize_torrent_exclude_trackers(Some("*")).unwrap(), Some("*".to_string())); diff --git a/src-tauri/src/torrent.rs b/src-tauri/src/torrent.rs index a100021..0697d2f 100644 --- a/src-tauri/src/torrent.rs +++ b/src-tauri/src/torrent.rs @@ -632,9 +632,8 @@ pub fn torrent_metadata_is_safe_for_magnet_reuse(bytes: &[u8]) -> Result Result { let parsed = url::Url::parse(source).map_err(|_| "invalid magnet URI".to_string())?; - if parsed.scheme() != "magnet" { - return Err("unsupported torrent source".to_string()); - } + validate_magnet_authority(&parsed)?; + let _ = normalized_magnet_trackers(&parsed)?; let info_hash = parsed .query_pairs() .find_map(|(key, value)| { @@ -654,15 +653,11 @@ fn magnet_metadata(source: &str) -> Result { Ok(ParsedTorrent { name, total_bytes: 0, files: Vec::new(), info_hash, web_seeds: Vec::new() }) } -/// Return the Magnet URI form that Firelink may hand to Aria2. Direct source -/// parameters can make Aria2 fetch arbitrary HTTP/FTP/SFTP resources during -/// metadata resolution, so keep the peer/tracker identity parameters but -/// remove `ws`, `as`, and `xs` sources. Users can add validated web seeds after -/// metadata is available through the transactional per-file path. -pub fn sanitize_magnet_uri_for_aria2(source: &str) -> Result { - let mut parsed = url::Url::parse(source.trim()).map_err(|_| "invalid magnet URI".to_string())?; - if parsed.scheme() != "magnet" - || !parsed.username().is_empty() +fn validate_magnet_authority(parsed: &url::Url) -> Result<(), String> { + if parsed.scheme() != "magnet" { + return Err("unsupported torrent source".to_string()); + } + if !parsed.username().is_empty() || parsed.password().is_some() || parsed.fragment().is_some() || parsed.host_str().is_some() @@ -670,6 +665,47 @@ pub fn sanitize_magnet_uri_for_aria2(source: &str) -> Result { { return Err("magnet URI contains an invalid authority or fragment".to_string()); } + Ok(()) +} + +fn normalized_magnet_trackers(parsed: &url::Url) -> Result, String> { + let mut trackers = Vec::new(); + let mut serialized_bytes = 0usize; + for (key, value) in parsed.query_pairs() { + if key != "tr" { + continue; + } + if trackers.len() >= crate::queue::MAX_TORRENT_TRACKERS { + return Err(format!( + "magnet tracker list must contain at most {} trackers", + crate::queue::MAX_TORRENT_TRACKERS + )); + } + let normalized = crate::queue::normalize_torrent_tracker_uri(&value)?; + serialized_bytes = serialized_bytes + .checked_add(normalized.len()) + .ok_or_else(|| "magnet tracker list is too large".to_string())?; + if serialized_bytes > crate::queue::MAX_TORRENT_TRACKER_BYTES { + return Err(format!( + "magnet tracker list must be at most {} bytes", + crate::queue::MAX_TORRENT_TRACKER_BYTES + )); + } + trackers.push(normalized); + } + Ok(trackers) +} + +/// Return the Magnet URI form that Firelink may hand to Aria2. Direct source +/// parameters can make Aria2 fetch arbitrary HTTP/FTP/SFTP resources during +/// metadata resolution, so keep the peer/tracker identity parameters but +/// remove `ws`, `as`, and `xs` sources. Users can add validated web seeds after +/// metadata is available through the transactional per-file path. +pub fn sanitize_magnet_uri_for_aria2(source: &str) -> Result { + let mut parsed = url::Url::parse(source.trim()).map_err(|_| "invalid magnet URI".to_string())?; + validate_magnet_authority(&parsed)?; + let normalized_trackers = normalized_magnet_trackers(&parsed)?; + let mut tracker_index = 0; let mut has_info_hash = false; let mut query = url::form_urlencoded::Serializer::new(String::new()); @@ -684,6 +720,12 @@ pub fn sanitize_magnet_uri_for_aria2(source: &str) -> Result { .ok_or_else(|| "magnet URI has no valid BitTorrent info hash".to_string())?; query.append_pair("xt", &format!("urn:btih:{hash}")); has_info_hash = true; + } else if key == "tr" { + let tracker = normalized_trackers + .get(tracker_index) + .ok_or_else(|| "magnet tracker normalization lost an entry".to_string())?; + tracker_index += 1; + query.append_pair("tr", tracker); } else { query.append_pair(&key, &value); } @@ -700,7 +742,7 @@ pub fn magnet_allows_cached_metadata(source: &str) -> bool { let Ok(parsed) = url::Url::parse(source.trim()) else { return false; }; - if parsed.scheme() != "magnet" { + if validate_magnet_authority(&parsed).is_err() || normalized_magnet_trackers(&parsed).is_err() { return false; } @@ -930,7 +972,23 @@ fn remove_orphaned_cached_torrents_at( let Some(name) = entry.file_name().to_str().map(ToOwned::to_owned) else { continue; }; - if file_type.is_file() && is_canonical_torrent_temp_file(&name) { + if crate::platform::is_atomic_temp_file_name(&name) { + if file_type.is_file() || file_type.is_symlink() { + match std::fs::remove_file(entry.path()) { + Ok(()) => removed += 1, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} + Err(error) => { + return Err(format!( + "could not remove orphaned torrent metadata temporary file: {error}" + )); + } + } + } + continue; + } + if (file_type.is_file() || file_type.is_symlink()) + && is_canonical_torrent_temp_file(&name) + { match std::fs::remove_file(entry.path()) { Ok(()) => removed += 1, Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} @@ -963,9 +1021,7 @@ fn remove_orphaned_cached_torrents_at( } continue; } - if !file_type.is_file() - || entry.path().extension().and_then(|ext| ext.to_str()) != Some("torrent") - { + if entry.path().extension().and_then(|ext| ext.to_str()) != Some("torrent") { continue; } let path = entry.path(); @@ -975,6 +1031,9 @@ fn remove_orphaned_cached_torrents_at( if retained_ids.contains(id) { continue; } + if !file_type.is_file() && !file_type.is_symlink() { + continue; + } match std::fs::remove_file(path) { Ok(()) => removed += 1, Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} @@ -1047,7 +1106,7 @@ pub async fn cache_torrent_bytes( .await .map_err(|error| format!("could not create torrent storage: {error}"))?; } - tokio::fs::write(&destination, bytes) + crate::platform::atomic_write_replace(&destination, bytes) .await .map_err(|error| format!("could not cache torrent metadata: {error}"))?; Ok(destination.to_string_lossy().to_string()) @@ -1170,32 +1229,10 @@ pub async fn cache_torrent_info_hash( tokio::fs::create_dir_all(parent) .await .map_err(|error| format!("could not create torrent storage: {error}"))?; - let temporary = parent.join(format!( - ".cache-{info_hash}.torrent.{}.tmp", - uuid::Uuid::new_v4().simple() - )); - if let Err(error) = tokio::fs::write(&temporary, bytes).await { - let _ = tokio::fs::remove_file(&temporary).await; - return Err(format!("could not stage canonical torrent metadata: {error}")); - } - match tokio::fs::rename(&temporary, &destination).await { - Ok(()) => Ok(Some(destination.to_string_lossy().to_string())), - Err(error) if error.kind() == std::io::ErrorKind::AlreadyExists => { - let _ = tokio::fs::remove_file(&temporary).await; - if read_cached_torrent_by_info_hash_unlocked(app_handle, &info_hash) - .await? - .is_some() - { - Ok(Some(destination.to_string_lossy().to_string())) - } else { - Err("canonical torrent metadata already exists but is invalid".to_string()) - } - } - Err(error) => { - let _ = tokio::fs::remove_file(&temporary).await; - Err(format!("could not commit canonical torrent metadata: {error}")) - } - } + crate::platform::atomic_write_replace(&destination, bytes) + .await + .map_err(|error| format!("could not commit canonical torrent metadata: {error}"))?; + Ok(Some(destination.to_string_lossy().to_string())) } pub fn validate_managed_torrent_path( @@ -1298,6 +1335,51 @@ mod tests { assert!(!sanitized.contains("as=")); } + #[test] + fn magnet_trackers_use_the_shared_bounded_tracker_policy() { + let valid = "magnet:?xt=urn:btih:0123456789012345678901234567890123456789&tr=https%3A%2F%2Ftracker.example%2Fannounce"; + assert!(magnet_allows_cached_metadata(valid)); + assert!(sanitize_magnet_uri_for_aria2(valid).is_ok()); + + for suffix in [ + "&tr=ftp%3A%2F%2Ftracker.example%2Fannounce", + "&tr=https%3A%2F%2Fuser%3Apass%40tracker.example%2Fannounce", + "&tr=https%3A%2F%2Ftracker.example%2Fannounce%23fragment", + "&tr=https%3A%2F%2F", + ] { + let magnet = format!( + "magnet:?xt=urn:btih:0123456789012345678901234567890123456789{suffix}" + ); + assert!(!magnet_allows_cached_metadata(&magnet), "{magnet}"); + assert!(sanitize_magnet_uri_for_aria2(&magnet).is_err(), "{magnet}"); + } + + assert!(!magnet_allows_cached_metadata( + "magnet://tracker.example/?xt=urn:btih:0123456789012345678901234567890123456789" + )); + assert!(sanitize_magnet_uri_for_aria2( + "magnet://tracker.example/?xt=urn:btih:0123456789012345678901234567890123456789" + ) + .is_err()); + + let too_many = (0..=crate::queue::MAX_TORRENT_TRACKERS) + .map(|index| format!("tr=https%3A%2F%2Ftracker{index}.example%2Fannounce")) + .collect::>() + .join("&"); + let magnet = format!( + "magnet:?xt=urn:btih:0123456789012345678901234567890123456789&{too_many}" + ); + assert!(sanitize_magnet_uri_for_aria2(&magnet).is_err()); + assert!(!magnet_allows_cached_metadata(&magnet)); + + let oversized = format!( + "magnet:?xt=urn:btih:0123456789012345678901234567890123456789&tr=https://tracker.example/{}", + "a".repeat(crate::queue::MAX_TORRENT_TRACKER_BYTES) + ); + assert!(sanitize_magnet_uri_for_aria2(&oversized).is_err()); + assert!(!magnet_allows_cached_metadata(&oversized)); + } + #[test] fn validates_torrent_output_names_as_single_safe_components() { for name in ["test", "My Torrent (1)", "archive.tar"] { @@ -1612,6 +1694,11 @@ mod tests { b"orphan temporary", ) .expect("orphan temporary metadata should exist"); + std::fs::write( + root.join(".firelink-atomic-0123456789abcdef0123456789abcdef.tmp"), + b"interrupted atomic staging", + ) + .expect("interrupted atomic staging should exist"); std::fs::write(root.join("notes.txt"), b"unrelated") .expect("unrelated file should exist"); let retained = HashSet::from(["keep-id".to_string()]); @@ -1619,7 +1706,7 @@ mod tests { assert_eq!( remove_orphaned_cached_torrents_at(root, &retained, &retained_hashes).unwrap(), - 4 + 5 ); assert!(root.join("keep-id.torrent").is_file()); assert!(!root.join("orphan-id.torrent").exists()); @@ -1633,6 +1720,34 @@ mod tests { ".cache-{retained_hash}.torrent.0123456789abcdef0123456789abcdef.tmp" )) .exists()); + assert!(!root + .join(".firelink-atomic-0123456789abcdef0123456789abcdef.tmp") + .exists()); assert!(root.join("notes.txt").is_file()); } + + #[cfg(unix)] + #[test] + fn removes_unretained_torrent_symlinks_without_following_targets() { + use std::os::unix::fs::symlink; + + let temporary = tempfile::tempdir().expect("temporary torrent storage should exist"); + let root = temporary.path(); + let target = root.join("target.bin"); + let link = root.join("orphan-link.torrent"); + let temporary_link = root.join( + ".cache-0123456789abcdef0123456789abcdef01234567.torrent.0123456789abcdef0123456789abcdef.tmp", + ); + std::fs::write(&target, b"target should remain").expect("target should exist"); + symlink(&target, &link).expect("orphan symlink should exist"); + symlink(&target, &temporary_link).expect("orphan temporary symlink should exist"); + + assert_eq!( + remove_orphaned_cached_torrents_at(root, &HashSet::new(), &HashSet::new()).unwrap(), + 2 + ); + assert!(!link.exists()); + assert!(!temporary_link.exists()); + assert!(target.is_file()); + } } diff --git a/src-tauri/tests/atomic_file.rs b/src-tauri/tests/atomic_file.rs new file mode 100644 index 0000000..5cce5e4 --- /dev/null +++ b/src-tauri/tests/atomic_file.rs @@ -0,0 +1,64 @@ +use firelink_lib::atomic_write_replace; +use std::fs; +use tempfile::tempdir; + +#[tokio::test] +async fn atomic_replacement_replaces_existing_file_repeatedly() { + let directory = tempdir().expect("temporary directory should be created"); + let destination = directory.path().join("download.torrent"); + + for value in [ + b"reserved".as_slice(), + b"copied".as_slice(), + b"databaseCommitted".as_slice(), + b"sourceCleanupPending".as_slice(), + ] { + atomic_write_replace(&destination, value) + .await + .expect("atomic replacement should succeed"); + assert_eq!( + fs::read(&destination).expect("destination should exist"), + value + ); + } +} + +#[cfg(unix)] +#[tokio::test] +async fn atomic_replacement_rejects_symbolic_link_destinations() { + use std::os::unix::fs::symlink; + + let directory = tempdir().expect("temporary directory should be created"); + let target = directory.path().join("target"); + let destination = directory.path().join("download.torrent"); + fs::write(&target, b"protected").expect("target should be written"); + symlink(&target, &destination).expect("symbolic link should be created"); + + assert!(atomic_write_replace(&destination, b"replacement") + .await + .is_err()); + assert_eq!( + fs::read(&target).expect("target should remain readable"), + b"protected" + ); +} + +#[tokio::test] +async fn atomic_replacement_recovers_after_non_regular_destination_failure() { + let directory = tempdir().expect("temporary directory should be created"); + let destination = directory.path().join("download.torrent"); + fs::create_dir(&destination).expect("non-regular destination should be created"); + + assert!(atomic_write_replace(&destination, b"replacement") + .await + .is_err()); + fs::remove_dir(&destination).expect("failed destination should be removable"); + + atomic_write_replace(&destination, b"recovered") + .await + .expect("atomic replacement should recover after the failed attempt"); + assert_eq!( + fs::read(&destination).expect("destination should exist"), + b"recovered" + ); +} diff --git a/src/components/PropertiesWindowApp.tsx b/src/components/PropertiesWindowApp.tsx index dd8f91d..5cbc801 100644 --- a/src/components/PropertiesWindowApp.tsx +++ b/src/components/PropertiesWindowApp.tsx @@ -1420,7 +1420,7 @@ export const PropertiesWindowApp = () => { {activeTab === 'files' && isTorrent &&
-
{fileProgress?.files.map(file => { const checked = selectedFiles === null ? file.selected : selectedFiles.includes(file.index); return ; })}
{t($ => $.properties.torrentFileProgressSelected)}#{t($ => $.properties.torrentFileProgressPath)}{t($ => $.properties.size)}{t($ => $.properties.torrentFileProgressCompleted)}
{ const current = selectedFiles ?? fileProgress.files.filter(candidate => candidate.selected).map(candidate => candidate.index); const next = checked ? current.filter(index => index !== file.index) : [...current, file.index]; setSelectedFiles(next); setDraftTab('files'); }} aria-label={`${file.index + 1} ${file.relativePath}`} />{file.index + 1}{file.relativePath}{formatDownloadBytes(file.length)}{formatDownloadBytes(file.completedLength)} ({file.length ? Math.round(file.completedLength / file.length * 100) : 0}%)
+
{fileProgress?.files.map(file => { const checked = selectedFiles === null ? file.selected : selectedFiles.includes(file.index); return ; })}
{t($ => $.properties.torrentFileProgressSelected)}#{t($ => $.properties.torrentFileProgressPath)}{t($ => $.properties.size)}{t($ => $.properties.torrentFileProgressCompleted)}
{ const current = selectedFiles ?? fileProgress.files.filter(candidate => candidate.selected).map(candidate => candidate.index); const next = checked ? current.filter(index => index !== file.index) : [...current, file.index]; setSelectedFiles(next); setDraftTab('files'); }} aria-label={`${file.index} ${file.relativePath}`} />{file.index}{file.relativePath}{formatDownloadBytes(file.length)}{formatDownloadBytes(file.completedLength)} ({file.length ? Math.round(file.completedLength / file.length * 100) : 0}%)
{diagnosticPhase === 'initial' && diagnosticsLoading && !fileProgress &&

{t($ => $.properties.torrentFileProgressLoading)}

} {diagnosticPhase === 'unavailable' && !fileProgress && !diagnosticError &&

{t($ => $.properties.torrentFileProgressUnavailable)}

} {diagnosticError &&

{diagnosticError}

} diff --git a/src/components/TorrentWebSeedEditor.test.tsx b/src/components/TorrentWebSeedEditor.test.tsx new file mode 100644 index 0000000..bb776c6 --- /dev/null +++ b/src/components/TorrentWebSeedEditor.test.tsx @@ -0,0 +1,39 @@ +import { renderToStaticMarkup } from 'react-dom/server'; +import { describe, expect, it, vi } from 'vitest'; + +vi.mock('react-i18next', () => ({ + useTranslation: () => ({ + t: (selector: (catalog: { properties: Record }) => string) => selector({ + properties: { + torrentWebSeedsFile: 'File', + torrentWebSeedsUri: 'URI', + torrentWebSeedsRemove: 'Remove', + torrentWebSeedsAdd: 'Add', + torrentWebSeedsInvalid: 'Invalid', + torrentWebSeedsEmpty: 'Empty', + }, + }), + }), +})); + +import { TorrentWebSeedEditor } from './TorrentWebSeedEditor'; + +describe('TorrentWebSeedEditor file indices', () => { + it('renders the native one-based file indices without adding an offset', () => { + const markup = renderToStaticMarkup( + undefined} + idPrefix="test" + />, + ); + + expect(markup).toContain('1: first.bin'); + expect(markup).toContain('2: second.bin'); + expect(markup).not.toContain('3: second.bin'); + }); +}); diff --git a/src/components/TorrentWebSeedEditor.tsx b/src/components/TorrentWebSeedEditor.tsx index 849ee9d..7b9d1ba 100644 --- a/src/components/TorrentWebSeedEditor.tsx +++ b/src/components/TorrentWebSeedEditor.tsx @@ -66,7 +66,7 @@ export const TorrentWebSeedEditor = ({ files, rows, onChange, disabled = false, aria-invalid={!rowIsValid} className="app-control w-full min-h-[30px] px-2 py-1.5 text-xs disabled:opacity-70" > - + ) : ( diff --git a/src/store/useDownloadStore.test.ts b/src/store/useDownloadStore.test.ts index 9106f69..8fcdd0a 100644 --- a/src/store/useDownloadStore.test.ts +++ b/src/store/useDownloadStore.test.ts @@ -993,6 +993,33 @@ describe('useDownloadStore', () => { expect(normalized.torrentEncryptionPolicy).toBeUndefined(); }); + it('drops zero-based persisted Torrent web-seed indices', () => { + const normalized = normalizePersistedDownloadProgress({ + id: 'torrent-web-seed-indexes', + url: 'magnet:?xt=urn:btih:bad', + fileName: 'payload', + status: 'queued', + category: 'Other', + dateAdded: '', + isTorrent: true, + torrentWebSeeds: [ + { fileIndex: 0, uri: 'https://mirror.example/zero' }, + { fileIndex: 1, uri: 'https://mirror.example/one' }, + ], + torrentWebSeedsNative: [ + { fileIndex: 0, uri: 'https://mirror.example/native-zero' }, + { fileIndex: 1, uri: 'https://mirror.example/native-one' }, + ], + }); + + expect(normalized.torrentWebSeeds).toEqual([ + { fileIndex: 1, uri: 'https://mirror.example/one' }, + ]); + expect(normalized.torrentWebSeedsNative).toEqual([ + { fileIndex: 1, uri: 'https://mirror.example/native-one' }, + ]); + }); + it('migrates legacy Torrent credential context before restart resume', () => { const normalized = normalizePersistedDownloadProgress({ id: 'legacy-torrent-credentials', diff --git a/src/store/useDownloadStore.ts b/src/store/useDownloadStore.ts index dd2b962..bb0a38a 100644 --- a/src/store/useDownloadStore.ts +++ b/src/store/useDownloadStore.ts @@ -751,7 +751,7 @@ export const normalizePersistedDownloadProgress = (download: DownloadItem): Down !!seed && typeof seed === 'object' && typeof (seed as { fileIndex?: unknown }).fileIndex === 'number' && Number.isInteger((seed as { fileIndex: number }).fileIndex) && - (seed as { fileIndex: number }).fileIndex >= 0 && + (seed as { fileIndex: number }).fileIndex >= 1 && typeof (seed as { uri?: unknown }).uri === 'string' && (seed as { uri: string }).uri.length <= 2048 ).slice(0, 256) @@ -762,7 +762,7 @@ export const normalizePersistedDownloadProgress = (download: DownloadItem): Down !!seed && typeof seed === 'object' && typeof (seed as { fileIndex?: unknown }).fileIndex === 'number' && Number.isInteger((seed as { fileIndex: number }).fileIndex) && - (seed as { fileIndex: number }).fileIndex >= 0 && + (seed as { fileIndex: number }).fileIndex >= 1 && typeof (seed as { uri?: unknown }).uri === 'string' && (seed as { uri: string }).uri.length <= 2048 ).slice(0, 256)