From 9bf02f2ee2cf242098ae8049a6d66337f67e0a2a Mon Sep 17 00:00:00 2001 From: NimBold Date: Fri, 28 Aug 2026 17:25:52 +0330 Subject: [PATCH] fix(network): make DNS routing TUN-aware (issue #38) - Share literal-target policy and route selection across metadata, Aria2, Torrent, and yt-dlp paths. - Start direct Aria2 resolution with system DNS and fence a bounded alternate resolver fallback. - Validate Torrent trackers/web seeds and harden magnet probe cleanup and lifecycle handling. - Add route, resolver, fallback, and smoke coverage without changing browser IPC payloads. Refs: #38 --- scripts/smoke-aria2-resolver.js | 40 ++- scripts/smoke-torrent.js | 20 +- src-tauri/src/lib.rs | 366 +++++++++---------- src-tauri/src/network.rs | 291 +++++++++++++++ src-tauri/src/queue.rs | 316 ++++++++++++----- src-tauri/src/torrent.rs | 42 +++ src-tauri/src/torrent_probe.rs | 512 +++++++++++++++++++++++++-- src-tauri/tests/queue_manager.rs | 19 +- src/components/AddDownloadsModal.tsx | 4 +- 9 files changed, 1273 insertions(+), 337 deletions(-) create mode 100644 src-tauri/src/network.rs diff --git a/scripts/smoke-aria2-resolver.js b/scripts/smoke-aria2-resolver.js index c9c0aeb..9724dd4 100644 --- a/scripts/smoke-aria2-resolver.js +++ b/scripts/smoke-aria2-resolver.js @@ -69,6 +69,14 @@ async function rpc(port, secret, method, params = []) { return body.result; } +async function forceRemoveIfPresent(port, secret, gid) { + try { + await rpc(port, secret, 'aria2.forceRemove', [gid]); + } catch (error) { + if (!/not found|no such download|active download not found/i.test(error.message)) throw error; + } +} + function childExited(child) { return child.exitCode !== null || child.signalCode !== null; } @@ -218,7 +226,37 @@ try { throw new Error(`aria2.addTorrent did not retain async-dns=false: ${JSON.stringify(torrentOptions)}`); } - console.log('[PASS] Aria2 retained system-resolver mode for normal and Torrent transfers'); + const proxyRoute = 'http://127.0.0.1:9'; + const normalizedProxyRoute = new URL(proxyRoute).toString(); + const proxiedUriResult = await rpc(rpcPort, secret, 'aria2.addUri', [['https://route-owned.invalid/file'], { + 'all-proxy': proxyRoute, + pause: 'true', + out: 'resolver-proxied.bin', + }]); + const proxiedUriOptions = await rpc(rpcPort, secret, 'aria2.getOption', [proxiedUriResult]); + if (proxiedUriOptions['all-proxy'] !== normalizedProxyRoute) { + throw new Error(`aria2.addUri did not retain the configured proxy route: ${JSON.stringify(proxiedUriOptions)}`); + } + if (proxiedUriOptions['async-dns'] === 'false') { + throw new Error(`proxied aria2.addUri unexpectedly forced system DNS resolution: ${JSON.stringify(proxiedUriOptions)}`); + } + + const proxiedTorrentResult = await rpc(rpcPort, secret, 'aria2.addTorrent', [torrent, [], { + 'all-proxy': proxyRoute, + pause: 'true', + dir: tempRoot, + }]); + const proxiedTorrentOptions = await rpc(rpcPort, secret, 'aria2.getOption', [proxiedTorrentResult]); + if (proxiedTorrentOptions['all-proxy'] !== normalizedProxyRoute) { + throw new Error(`aria2.addTorrent did not retain the configured proxy route: ${JSON.stringify(proxiedTorrentOptions)}`); + } + if (proxiedTorrentOptions['async-dns'] === 'false') { + throw new Error(`proxied aria2.addTorrent unexpectedly forced system DNS resolution: ${JSON.stringify(proxiedTorrentOptions)}`); + } + await forceRemoveIfPresent(rpcPort, secret, proxiedUriResult); + await forceRemoveIfPresent(rpcPort, secret, proxiedTorrentResult); + + console.log('[PASS] Aria2 retained system-resolver mode for direct normal/Torrent transfers and configured proxy mode for proxied transfers'); } catch (error) { const detail = stderr.trim(); throw new Error(`${error.message}${detail ? `\n${detail}` : ''}`); diff --git a/scripts/smoke-torrent.js b/scripts/smoke-torrent.js index 8be4529..370040f 100644 --- a/scripts/smoke-torrent.js +++ b/scripts/smoke-torrent.js @@ -790,12 +790,15 @@ async function main() { dir: probeDir, 'bt-metadata-only': 'true', 'bt-save-metadata': 'true', + 'async-dns': 'false', 'max-tries': '3', 'retry-wait': '1', 'connect-timeout': '5', timeout: '15', 'auto-file-renaming': 'false', }]); + const probeOptions = await rpc(client.rpcPort, client.secret, 'aria2.getOption', [probeGid]); + assert(probeOptions['async-dns'] === 'false', 'direct magnet metadata probe did not retain system DNS resolution'); const probeStatus = await waitForTerminal(client, probeGid, 30000); assert(probeStatus.status === 'complete', 'magnet metadata probe did not complete'); const savedTorrentPaths = fs.readdirSync(probeDir) @@ -841,7 +844,22 @@ async function main() { if (probeRemoved) await waitForRemoved(client, probeGid); fs.rmSync(probeDir, { recursive: true, force: true }); fs.mkdirSync(probeDir, { recursive: true }); - console.log('[OK] metadata probe was removed after resolution'); + const proxiedProbeRoute = 'http://127.0.0.1:9'; + const normalizedProxiedProbeRoute = new URL(proxiedProbeRoute).toString(); + const proxiedProbeGid = await rpc(client.rpcPort, client.secret, 'aria2.addUri', [[magnet], { + dir: probeDir, + 'bt-metadata-only': 'true', + 'bt-save-metadata': 'true', + 'all-proxy': proxiedProbeRoute, + pause: 'true', + 'auto-file-renaming': 'false', + }]); + const proxiedProbeOptions = await rpc(client.rpcPort, client.secret, 'aria2.getOption', [proxiedProbeGid]); + assert(proxiedProbeOptions['all-proxy'] === normalizedProxiedProbeRoute, 'proxied magnet metadata probe did not retain the configured proxy route'); + assert(proxiedProbeOptions['async-dns'] !== 'false', 'proxied magnet metadata probe unexpectedly forced system DNS resolution'); + assert(await forceRemoveIfPresent(client, proxiedProbeGid), 'proxied magnet metadata probe was not removable'); + await waitForRemoved(client, proxiedProbeGid); + console.log('[OK] metadata probe was removed after resolution; direct and proxied resolver modes were retained'); const finalGid = await rpc(client.rpcPort, client.secret, 'aria2.addTorrent', [ trackerlessTorrentBytes.toString('base64'), diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index c5586c9..400a720 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -1699,60 +1699,23 @@ fn should_cleanup_media_artifacts_after_failure( !(crate::retry::is_transient_network_error(failure_reason) && strike < max_retries) } -fn is_blocked_network_address(ip: std::net::IpAddr) -> bool { - if ip.is_loopback() || ip.is_multicast() || ip.is_unspecified() { - return true; - } - match ip { - std::net::IpAddr::V4(ipv4) => ipv4.is_private() || ipv4.is_link_local(), - std::net::IpAddr::V6(ipv6) => { - ipv6.to_ipv4().is_some_and(|ipv4| is_blocked_network_address(ipv4.into())) - || (ipv6.segments()[0] & 0xfe00) == 0xfc00 - || (ipv6.segments()[0] & 0xffc0) == 0xfe80 - } - } -} - -async fn resolve_and_validate_url_host( - parsed: &reqwest::Url, -) -> Result<(String, std::net::SocketAddr), String> { - let host = parsed.host_str().ok_or("SSRF blocked: No host")?; - let lookup_host = host.trim_start_matches('[').trim_end_matches(']'); - let port = parsed.port_or_known_default().unwrap_or_else(|| match parsed.scheme() { - "ftp" => 21, - "sftp" => 22, - _ => 80, - }); - - let addrs: Vec<_> = if let Ok(ip) = lookup_host.parse::() { - vec![std::net::SocketAddr::new(ip, port)] - } else { - tokio::time::timeout( - std::time::Duration::from_secs(5), - tokio::net::lookup_host((lookup_host, port)), - ) - .await - .map_err(|_| "SSRF blocked: DNS resolution timed out")? - .map_err(|_| "SSRF blocked: DNS resolution failed")? - .collect() - }; - let addr = addrs.first().copied().ok_or("SSRF blocked: No DNS records")?; - if addrs.iter().any(|candidate| is_blocked_network_address(candidate.ip())) { - return Err("SSRF blocked: Private/local IP not allowed".to_string()); - } - Ok((lookup_host.to_string(), addr)) -} - -async fn validate_url_ssrf(url: &str) -> Result, String> { - let parsed = reqwest::Url::parse(url).map_err(|_| "SSRF blocked: Invalid URL")?; - if parsed.scheme() != "http" && parsed.scheme() != "https" { - return Err("SSRF blocked: Only HTTP/HTTPS schemes allowed".to_string()); - } - resolve_and_validate_url_host(&parsed).await.map(Some) -} - const MAX_REMOTE_TORRENT_REDIRECTS: usize = 5; +fn validate_http_url_route(url: &str) -> Result { + crate::network::parse_and_validate_url( + url, + &["http", "https"], + crate::network::CredentialPolicy::Allow, + ) + .map_err(|error| { + if error == "Unsupported URL scheme" { + "SSRF blocked: Only HTTP/HTTPS schemes allowed".to_string() + } else { + error + } + }) +} + fn is_remote_torrent_source(source: &str, explicitly_torrent: bool) -> bool { if crate::torrent::is_remote_torrent_url(source) { return true; @@ -1780,6 +1743,7 @@ async fn fetch_remote_torrent_bytes( .map(crate::queue::aria2_all_proxy_value) .transpose()? .flatten(); + let route = crate::network::NetworkRoute::from_proxy(proxy.as_deref()); let mut current = reqwest::Url::parse(source) .map_err(|_| "SSRF blocked: Invalid URL".to_string())?; let original_origin = Some(current.clone()); @@ -1799,21 +1763,18 @@ async fn fetch_remote_torrent_bytes( return Err("Torrent metadata URLs must not contain credentials".to_string()); } - let (host, address) = validate_url_ssrf(current.as_str()) - .await? - .ok_or_else(|| "SSRF blocked: No host".to_string())?; + crate::network::validate_url( + ¤t, + &["http", "https"], + crate::network::CredentialPolicy::Reject( + "Torrent metadata URLs must not contain credentials", + ), + )?; let mut builder = reqwest::Client::builder() .redirect(reqwest::redirect::Policy::none()) .timeout(FILE_METADATA_TIMEOUT) .user_agent("Firelink torrent metadata"); - match proxy.as_deref().map(str::trim) { - Some("") => builder = builder.no_proxy(), - Some(proxy) => { - builder = builder.proxy(reqwest::Proxy::all(proxy).map_err(|error| error.to_string())?); - } - None => {} - } - builder = builder.resolve(&host, address); + builder = route.configure_reqwest(builder)?; let same_origin_credentials = should_send_metadata_credentials( original_origin.as_ref(), Some(¤t), @@ -1995,6 +1956,7 @@ async fn fetch_metadata( ) -> Result { properties_window::ensure_main_window(&caller)?; ensure_reqwest_crypto_provider(); + let route = crate::network::NetworkRoute::from_proxy(proxy.as_deref()); let metadata_started = Instant::now(); let mut current_url = url.clone(); @@ -2037,27 +1999,20 @@ async fn fetch_metadata( let mut builder = reqwest::Client::builder() .redirect(reqwest::redirect::Policy::none()) .timeout(FILE_METADATA_TIMEOUT); - if let Some(proxy) = proxy.as_deref().map(str::trim).filter(|s| !s.is_empty()) { - if proxy.eq_ignore_ascii_case("none") { - builder = builder.no_proxy(); - } else { - let proxy = match reqwest::Proxy::all(proxy) { - Ok(proxy) => proxy, - Err(error) => { - log::warn!( - "metadata [stage=metadata operation=client result=failed host={} error_code=proxy_configuration elapsed_ms={}]", - reqwest::Url::parse(¤t_url) - .ok() - .and_then(|value| value.host_str().map(str::to_string)) - .unwrap_or_else(|| "".to_string()), - metadata_started.elapsed().as_millis() - ); - return Err(crate::redact_sensitive_text(&error.to_string())); - } - }; - builder = builder.proxy(proxy); + builder = match route.configure_reqwest(builder) { + Ok(builder) => builder, + Err(error) => { + log::warn!( + "metadata [stage=metadata operation=client result=failed host={} error_code=proxy_configuration elapsed_ms={}]", + reqwest::Url::parse(¤t_url) + .ok() + .and_then(|value| value.host_str().map(str::to_string)) + .unwrap_or_else(|| "".to_string()), + metadata_started.elapsed().as_millis() + ); + return Err(error); } - } + }; if let Some(ref ua) = user_agent { let ua = ua.trim(); @@ -2085,38 +2040,33 @@ async fn fetch_metadata( .host_str() .unwrap_or("") .to_string(); - let dns_started = Instant::now(); - let resolved_addr = match parsed_current_url.scheme() { - "http" | "https" => validate_url_ssrf(¤t_url).await, - "ftp" | "sftp" => resolve_and_validate_url_host(&parsed_current_url) - .await - .map(Some), - _ => Err("Unsupported URL scheme".to_string()), + let allowed_schemes: &[&str] = match parsed_current_url.scheme() { + "http" | "https" | "ftp" | "sftp" => &["http", "https", "ftp", "sftp"], + _ => return Err("Unsupported URL scheme".to_string()), }; - let resolved_addr = match resolved_addr { - Ok(resolved_addr) => { + match crate::network::validate_url( + &parsed_current_url, + allowed_schemes, + crate::network::CredentialPolicy::Allow, + ) { + Ok(()) => { log::info!( - "metadata [stage=dns operation=resolve host={} result=ok elapsed_ms={} total_elapsed_ms={}]", + "metadata [stage=network_policy operation=validate host={} result=ok route={} total_elapsed_ms={}]", metadata_host, - dns_started.elapsed().as_millis(), + route.label(), metadata_started.elapsed().as_millis() ); - resolved_addr } Err(error) => { log::warn!( - "metadata [stage=dns operation=resolve host={} result=failed error_code={} elapsed_ms={} total_elapsed_ms={}]", + "metadata [stage=network_policy operation=validate host={} result=failed error_code={} route={} total_elapsed_ms={}]", metadata_host, metadata_error_code(&error), - dns_started.elapsed().as_millis(), + route.label(), metadata_started.elapsed().as_millis() ); return Err(error); } - }; - - if let Some((host, addr)) = resolved_addr { - builder = builder.resolve(&host, addr); } let current_origin = reqwest::Url::parse(¤t_url).ok(); @@ -2689,7 +2639,7 @@ async fn fetch_media_metadata( proxy: Option, ) -> Result { properties_window::ensure_main_window(&caller)?; - validate_url_ssrf(&url).await?; + validate_http_url_route(&url)?; let cache_key = media_metadata_cache_key( &url, &cookie_browser, @@ -2826,7 +2776,7 @@ async fn fetch_media_playlist_metadata( proxy: Option, ) -> Result { properties_window::ensure_main_window(&caller)?; - validate_url_ssrf(&url).await?; + validate_http_url_route(&url)?; let result = fetch_media_playlist_metadata_uncached( app_handle.clone(), @@ -2917,12 +2867,9 @@ async fn fetch_media_playlist_metadata_uncached( } } - if let Some(proxy) = proxy.as_deref().map(str::trim).filter(|s| !s.is_empty()) { - if proxy.eq_ignore_ascii_case("none") { - cmd = cmd.arg("--proxy").arg(""); - } else { - cmd = cmd.arg("--proxy").arg(proxy); - } + let route = crate::network::NetworkRoute::from_proxy(proxy.as_deref()); + if let Some(proxy) = route.ytdlp_proxy_value() { + cmd = cmd.arg("--proxy").arg(proxy); } if let Some(ua) = user_agent @@ -3031,12 +2978,9 @@ async fn fetch_media_metadata_uncached( } } - if let Some(proxy) = proxy.as_deref().map(str::trim).filter(|s| !s.is_empty()) { - if proxy.eq_ignore_ascii_case("none") { - cmd = cmd.arg("--proxy").arg(""); - } else { - cmd = cmd.arg("--proxy").arg(proxy); - } + let route = crate::network::NetworkRoute::from_proxy(proxy.as_deref()); + if let Some(proxy) = route.ytdlp_proxy_value() { + cmd = cmd.arg("--proxy").arg(proxy); } if let Some(ua) = user_agent @@ -3609,6 +3553,7 @@ mod engines; pub mod error; #[allow(dead_code)] pub mod ipc; +mod network; mod parity; mod power; mod platform; @@ -4803,6 +4748,7 @@ pub(crate) async fn start_media_download_internal( max_tries: Option, cancel_rx: &mut tokio::sync::watch::Receiver, ) -> Result { + validate_http_url_route(&url)?; let cookie_source = normalize_media_cookie_source(cookie_source.as_deref())?; let safe_filename = crate::download_ownership::canonical_download_filename(&filename); @@ -4880,6 +4826,7 @@ pub(crate) async fn start_media_download_internal( let mut strike = 0_usize; let mut effective_cookie_source = cookie_source; let mut browser_cookie_fallback_used = false; + let route = crate::network::NetworkRoute::from_proxy(proxy.as_deref()); while strike <= max_retries { if *cancel_rx.borrow() { @@ -4932,12 +4879,8 @@ pub(crate) async fn start_media_download_internal( cmd = cmd.arg("--limit-rate").arg(limit); } - if let Some(p) = proxy.as_deref().map(str::trim).filter(|s| !s.is_empty()) { - if p.eq_ignore_ascii_case("none") { - cmd = cmd.arg("--proxy").arg(""); - } else { - cmd = cmd.arg("--proxy").arg(p); - } + if let Some(proxy) = route.ytdlp_proxy_value() { + cmd = cmd.arg("--proxy").arg(proxy); } if let Some(cs) = effective_cookie_source.as_ref() { @@ -8099,13 +8042,12 @@ fn enqueue_lifecycle_generation(item: &queue::EnqueueItem) -> Result Result<(), String> { - let parsed = reqwest::Url::parse(url).map_err(|_| "SSRF blocked: Invalid URL".to_string())?; - match parsed.scheme() { - "http" | "https" | "ftp" | "sftp" => { - resolve_and_validate_url_host(&parsed).await.map(|_| ()) - } - _ => Err("Unsupported URL scheme".to_string()), - } + crate::network::parse_and_validate_url( + url, + &["http", "https", "ftp", "sftp"], + crate::network::CredentialPolicy::Allow, + ) + .map(|_| ()) } async fn validate_enqueue_uris(url: &str, mirrors: Option<&str>) -> Result<(), String> { @@ -8127,15 +8069,20 @@ pub(crate) async fn validate_torrent_web_seed_destinations(seeds: &[String]) -> { return Err("Torrent web-seed URI must use HTTP or HTTPS without credentials or fragments".to_string()); } - resolve_and_validate_url_host(&parsed).await?; + crate::network::validate_url( + &parsed, + &["http", "https"], + crate::network::CredentialPolicy::Reject( + "Torrent web-seed URI must use HTTP or HTTPS without credentials or fragments", + ), + )?; } Ok(()) } /// Embedded web seeds are optional Torrent accelerators, not the transfer's -/// source of truth. Keep the peer-only path usable when a stale seed domain is -/// temporarily unavailable, while still rejecting destinations that resolve -/// to local or private addresses before they are offered to Aria2. +/// source of truth. Validate them before they are offered to Aria2; hostnames +/// remain route-owned and are intentionally not resolved by Firelink. pub(crate) async fn filter_torrent_web_seed_destinations( seeds: &[String], ) -> Result, String> { @@ -8151,24 +8098,28 @@ pub(crate) async fn filter_torrent_web_seed_destinations( { return Err("Torrent web-seed URI must use HTTP or HTTPS without credentials or fragments".to_string()); } - match resolve_and_validate_url_host(&parsed).await { - Ok(_) => allowed.push(seed.clone()), - Err(error) - if error == "SSRF blocked: DNS resolution failed" - || error == "SSRF blocked: DNS resolution timed out" - || error == "SSRF blocked: No DNS records" => - { - log::warn!( - "Skipping unavailable embedded Torrent web seed {}", - parsed.host_str().unwrap_or("") - ); - } - Err(error) => return Err(error), - } + crate::network::validate_url( + &parsed, + &["http", "https"], + crate::network::CredentialPolicy::Reject( + "Torrent web-seed URI must use HTTP or HTTPS without credentials or fragments", + ), + )?; + allowed.push(seed.clone()); } Ok(allowed) } +fn validate_torrent_metadata_network_policy(bytes: &[u8]) -> Result<(), String> { + crate::torrent::validate_torrent_tracker_metadata(bytes)?; + let details = crate::torrent::torrent_details_from_bytes(bytes)?; + queue::validate_torrent_tracker_destinations(&details.trackers)?; + for web_seed in &details.web_seeds { + queue::normalize_torrent_web_seed_uri(web_seed)?; + } + Ok(()) +} + async fn validate_torrent_enqueue( app_handle: &tauri::AppHandle, item: &mut queue::EnqueueItem, @@ -8202,6 +8153,7 @@ async fn validate_torrent_enqueue( let path = crate::torrent::validate_managed_torrent_path(app_handle, &item.id, path)?; let bytes = crate::torrent::read_bounded_torrent_bytes_sync(&path) .map_err(|error| format!("could not read cached torrent metadata: {error}"))?; + validate_torrent_metadata_network_policy(&bytes)?; let metadata = crate::torrent::parse_torrent_bytes(&bytes)?; let normalized_user_web_seeds = queue::normalize_torrent_web_seeds( item.torrent_web_seeds.as_deref(), @@ -9191,49 +9143,6 @@ async fn remove_magnet_metadata_probe_dir(path: &std::path::Path) -> Result<(), } } -struct MagnetMetadataProbeTelemetry { - info_hash: String, - started_at: Instant, - outcome: Option<&'static str>, -} - -impl MagnetMetadataProbeTelemetry { - fn new(info_hash: &str, tracker_bearing: bool, proxy_configured: bool) -> Self { - log::debug!( - "magnet metadata probe started: info_hash={info_hash}, tracker_bearing={tracker_bearing}, proxy_configured={proxy_configured}" - ); - Self { - info_hash: info_hash.to_string(), - started_at: Instant::now(), - outcome: None, - } - } - - fn finish(&mut self, outcome: &'static str) { - if self.outcome.is_some() { - return; - } - self.outcome = Some(outcome); - log::debug!( - "magnet metadata probe finished: info_hash={}, outcome={outcome}, elapsed_ms={}", - self.info_hash, - self.started_at.elapsed().as_millis() - ); - } -} - -impl Drop for MagnetMetadataProbeTelemetry { - fn drop(&mut self) { - if self.outcome.is_none() { - log::debug!( - "magnet metadata probe finished: info_hash={}, outcome=canceled, elapsed_ms={}", - self.info_hash, - self.started_at.elapsed().as_millis() - ); - } - } -} - async fn resolve_magnet_metadata( app_handle: &tauri::AppHandle, state: &AppState, @@ -9256,6 +9165,7 @@ async fn resolve_magnet_metadata( .await { Ok(Some(bytes)) => { + validate_torrent_metadata_network_policy(&bytes)?; let parsed = crate::torrent::parse_torrent_bytes(&bytes)?; crate::torrent::validate_info_hash(Some(&expected.info_hash), &parsed.info_hash)?; let torrent_path = @@ -9329,27 +9239,25 @@ async fn resolve_magnet_metadata( let client = std::sync::Arc::new(Aria2RpcClient { port, secret }); let sanitized_source = crate::torrent::sanitize_magnet_uri_for_aria2(source)?; - let mut telemetry = MagnetMetadataProbeTelemetry::new( - &expected.info_hash, - sanitized_source.contains("tr="), - proxy_value.is_some(), - ); - let metadata_result = crate::torrent_probe::run_metadata_probe( + let direct_route = proxy_value + .as_deref() + .is_none_or(|proxy| proxy.trim().is_empty()); + let alternate_resolver_available = + direct_route && state.queue_manager.aria2_async_dns_supported(); + let first_resolver_mode = if direct_route { "system" } else { "configured" }; + let metadata_result = crate::torrent_probe::run_metadata_probe_with_resolver_fallback( client, &sanitized_source, options, &metadata_path, + first_resolver_mode, + alternate_resolver_available, Duration::from_secs(60), + Duration::from_secs(20), Duration::from_millis(250), ) .await; - match &metadata_result { - Ok(_) => telemetry.finish("probe-success"), - Err(crate::torrent_probe::ProbeFailure::Metadata(_)) => telemetry.finish("metadata-failure"), - Err(crate::torrent_probe::ProbeFailure::Cleanup(_)) => telemetry.finish("cleanup-failure"), - } - let bytes = match metadata_result { Ok(bytes) => bytes, Err(crate::torrent_probe::ProbeFailure::Metadata(error)) => { @@ -9376,6 +9284,7 @@ async fn resolve_magnet_metadata( if let Err(error) = remove_magnet_metadata_probe_dir(&probe_dir).await { return Err(error); } + validate_torrent_metadata_network_policy(&bytes)?; let parsed = crate::torrent::parse_torrent_bytes(&bytes)?; crate::torrent::validate_info_hash(Some(&expected.info_hash), &parsed.info_hash)?; let torrent_path = if cache { @@ -9430,6 +9339,7 @@ async fn inspect_torrent( ) .await .map_err(AppError::Internal)?; + validate_torrent_metadata_network_policy(&bytes).map_err(AppError::Internal)?; let parsed = crate::torrent::parse_torrent_bytes(&bytes).map_err(AppError::Internal)?; let torrent_path = if cache != Some(false) { let torrent_path = crate::torrent::cache_torrent_bytes(&app_handle, &id, &bytes) @@ -13913,6 +13823,7 @@ mod tests { normalize_media_connections, normalize_media_cookie_source, validate_enqueue_url, validate_enqueue_uris, validate_keychain_grant_request_id, + validate_torrent_metadata_network_policy, aria2_gid_not_found, aria2_download_state_progress, preflight_download_destination_access, retained_torrent_id_from_persisted_record, @@ -14885,7 +14796,11 @@ mod tests { } #[tokio::test] - async fn enqueue_url_validation_blocks_local_http_but_preserves_ftp() { + async fn enqueue_url_validation_blocks_literal_local_targets_without_dns_preflight() { + assert_eq!( + validate_enqueue_url("https://this-host-does-not-resolve.invalid/file.zip").await, + Ok(()) + ); assert_eq!( validate_enqueue_url("http://127.0.0.1/file.zip").await, Err("SSRF blocked: Private/local IP not allowed".to_string()) @@ -14902,10 +14817,57 @@ mod tests { validate_enqueue_url("http://[::ffff:127.0.0.1]/file.zip").await, Err("SSRF blocked: Private/local IP not allowed".to_string()) ); + for url in [ + "http://localhost/file.zip", + "http://localhost./file.zip", + "http://downloads.localhost/file.zip", + ] { + assert_eq!( + validate_enqueue_url(url).await, + Err("SSRF blocked: Private/local IP not allowed".to_string()), + "{url}" + ); + } assert_eq!( validate_enqueue_url("file:///tmp/file.zip").await, Err("Unsupported URL scheme".to_string()) ); + assert_eq!( + validate_enqueue_url("ftp://this-host-does-not-resolve.invalid/file.zip").await, + Ok(()) + ); + assert_eq!( + validate_enqueue_url("sftp://this-host-does-not-resolve.invalid/file.zip").await, + Ok(()) + ); + } + + #[test] + fn cached_torrent_network_policy_validates_trackers_without_resolving_hostnames() { + assert_eq!( + validate_torrent_metadata_network_policy( + b"d8:announce25:http://127.0.0.1/announce4:infod6:lengthi5e4:name4:test12:piece lengthi2e6:pieces20:01234567890123456789ee" + ), + Err("SSRF blocked: Private/local IP not allowed".to_string()) + ); + assert_eq!( + validate_torrent_metadata_network_policy( + b"d8:announce49:https://tracker-does-not-resolve.invalid/announce4:infod6:lengthi5e4:name4:test12:piece lengthi2e6:pieces20:01234567890123456789ee" + ), + Ok(()) + ); + assert_eq!( + validate_torrent_metadata_network_policy( + b"d4:infod6:lengthi5e4:name4:test12:piece lengthi2e6:pieces20:01234567890123456789e8:url-list21:http://127.0.0.1/filee" + ), + Err("SSRF blocked: Private/local IP not allowed".to_string()) + ); + assert_eq!( + validate_torrent_metadata_network_policy( + b"d4:infod6:lengthi5e4:name4:test12:piece lengthi2e6:pieces20:01234567890123456789e8:url-list19:https://x.invalid/ae" + ), + Ok(()) + ); } #[tokio::test] diff --git a/src-tauri/src/network.rs b/src-tauri/src/network.rs new file mode 100644 index 0000000..ccb3360 --- /dev/null +++ b/src-tauri/src/network.rs @@ -0,0 +1,291 @@ +//! Shared network-target policy and route plumbing. +//! +//! Hostname resolution is deliberately not part of URL policy. The selected +//! route (the OS/TUN resolver, an explicit proxy, or the consumer's own +//! resolver) owns that decision. Only literal local targets and reserved local +//! names are rejected here. + +use std::net::IpAddr; + +use reqwest::{ClientBuilder, Proxy, Url}; + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) enum NetworkRoute { + /// Preserve reqwest's normal environment/OS route selection. + Inherited, + /// Bypass configured/environment proxies and use the direct OS route. + Direct, + /// Route the consumer through this configured proxy. + Proxy(String), +} + +impl NetworkRoute { + pub(crate) fn from_proxy(proxy: Option<&str>) -> Self { + match proxy.map(str::trim) { + None => Self::Inherited, + Some(value) if value.is_empty() || value.eq_ignore_ascii_case("none") => Self::Direct, + Some(value) => Self::Proxy(value.to_string()), + } + } + + pub(crate) fn configure_reqwest( + &self, + builder: ClientBuilder, + ) -> Result { + match self { + Self::Inherited => Ok(builder), + Self::Direct => Ok(builder.no_proxy()), + Self::Proxy(value) => { + let proxy = Proxy::all(value) + .map_err(|error| crate::redact_sensitive_text(&error.to_string()))?; + Ok(builder.proxy(proxy)) + } + } + } + + /// Translate the route into Aria2's all-proxy option without changing the + /// target hostname. Aria2 accepts HTTP-family proxy endpoints for normal + /// transfers; SOCKS remains a yt-dlp-only route in Firelink. + pub(crate) fn aria2_proxy_value(&self) -> Result, String> { + match self { + Self::Inherited => Ok(None), + Self::Direct => Ok(Some(String::new())), + Self::Proxy(value) => { + let is_socks = value.split_once("://").is_some_and(|(scheme, _)| { + scheme.eq_ignore_ascii_case("socks") + || scheme.to_ascii_lowercase().starts_with("socks") + }); + if is_socks { + return Err( + "SOCKS system proxies are not supported for normal file downloads because aria2 only accepts HTTP/HTTPS/FTP proxy URLs. Use an HTTP proxy endpoint for normal downloads, or use media downloads where yt-dlp supports SOCKS." + .to_string(), + ); + } + Ok(Some(value.clone())) + } + } + } + + /// Translate the route into yt-dlp's explicit proxy argument. `None` + /// means inherit the process/OS route; an empty value deliberately disables + /// inherited proxies for an explicit direct route. + pub(crate) fn ytdlp_proxy_value(&self) -> Option<&str> { + match self { + Self::Inherited => None, + Self::Direct => Some(""), + Self::Proxy(value) => Some(value), + } + } + + pub(crate) fn label(&self) -> &'static str { + match self { + Self::Inherited => "inherited", + Self::Direct => "direct", + Self::Proxy(_) => "configured", + } + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum CredentialPolicy { + Allow, + Reject(&'static str), +} + +/// Validate a parsed URL without resolving its hostname. +pub(crate) fn validate_url( + parsed: &Url, + allowed_schemes: &[&str], + credentials: CredentialPolicy, +) -> Result<(), String> { + if !allowed_schemes + .iter() + .any(|scheme| parsed.scheme() == *scheme) + { + return Err("Unsupported URL scheme".to_string()); + } + + let host = parsed + .host_str() + .filter(|host| !host.trim().is_empty()) + .ok_or_else(|| "SSRF blocked: No host".to_string())?; + + if let CredentialPolicy::Reject(message) = credentials { + if !parsed.username().is_empty() || parsed.password().is_some() { + return Err(message.to_string()); + } + } + + let normalized_host = host.trim_matches(['[', ']']); + if is_local_hostname(normalized_host) { + return Err("SSRF blocked: Private/local IP not allowed".to_string()); + } + if parse_literal_ip(normalized_host).is_some_and(is_blocked_network_address) { + return Err("SSRF blocked: Private/local IP not allowed".to_string()); + } + + Ok(()) +} + +pub(crate) fn parse_and_validate_url( + raw: &str, + allowed_schemes: &[&str], + credentials: CredentialPolicy, +) -> Result { + let parsed = Url::parse(raw).map_err(|_| "SSRF blocked: Invalid URL".to_string())?; + validate_url(&parsed, allowed_schemes, credentials)?; + Ok(parsed) +} + +pub(crate) fn is_local_hostname(host: &str) -> bool { + let normalized = host.trim().trim_end_matches('.').to_ascii_lowercase(); + normalized == "localhost" + || normalized.ends_with(".localhost") + || matches!( + normalized.as_str(), + "localhost.localdomain" | "ip6-localhost" | "ip6-loopback" + ) + || normalized.ends_with(".local") +} + +fn parse_literal_ip(host: &str) -> Option { + let host = host.trim_end_matches('.'); + if let Ok(ip) = host.parse::() { + return Some(ip); + } + host.split_once("%25") + .and_then(|(address, _zone)| address.parse::().ok()) +} + +pub(crate) fn is_blocked_network_address(ip: IpAddr) -> bool { + if ip.is_loopback() || ip.is_multicast() || ip.is_unspecified() { + return true; + } + match ip { + IpAddr::V4(ipv4) => ipv4.is_private() || ipv4.is_link_local(), + IpAddr::V6(ipv6) => { + ipv6.to_ipv4() + .is_some_and(|ipv4| is_blocked_network_address(ipv4.into())) + || (ipv6.segments()[0] & 0xfe00) == 0xfc00 + || (ipv6.segments()[0] & 0xffc0) == 0xfe80 + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn validate(raw: &str, schemes: &[&str]) -> Result<(), String> { + parse_and_validate_url(raw, schemes, CredentialPolicy::Allow).map(|_| ()) + } + + #[test] + fn rejects_literal_local_and_mapped_addresses() { + for raw in [ + "http://127.0.0.1/file", + "http://10.0.0.8/file", + "http://169.254.10.2/file", + "http://[::1]/file", + "http://[::ffff:127.0.0.1]/file", + "http://[fc00::1]/file", + "http://[fe80::1]/file", + "http://127.0.0.1./file", + // URL parsers commonly canonicalize these legacy IPv4 literal + // spellings, but keep the policy test explicit so a parser + // upgrade cannot turn them into SSRF bypasses. + "http://127.1/file", + "http://2130706433/file", + "http://0x7f000001/file", + "http://0177.0.0.1/file", + "http://0/file", + ] { + assert_eq!( + validate(raw, &["http", "https"]), + Err("SSRF blocked: Private/local IP not allowed".to_string()), + "{raw}" + ); + } + } + + #[test] + fn rejects_localhost_aliases_without_dns() { + for raw in [ + "http://localhost/file", + "http://localhost./file", + "http://media.localhost/file", + "http://localhost.localdomain/file", + "http://printer.local/file", + ] { + assert_eq!( + validate(raw, &["http", "https"]), + Err("SSRF blocked: Private/local IP not allowed".to_string()), + "{raw}" + ); + } + } + + #[test] + fn rejects_scoped_link_local_literals() { + assert!(matches!( + validate("http://[fe80::1%25en0]/file", &["http", "https"]), + Err(message) if message.contains("SSRF blocked") + )); + } + + #[test] + fn public_hostname_validation_does_not_require_application_dns() { + assert_eq!( + validate( + "https://this-host-does-not-resolve.invalid/file", + &["http", "https"] + ), + Ok(()) + ); + } + + #[test] + fn route_mapping_preserves_direct_and_proxy_choices() { + assert_eq!(NetworkRoute::from_proxy(None), NetworkRoute::Inherited); + assert_eq!(NetworkRoute::from_proxy(Some("none")), NetworkRoute::Direct); + assert_eq!(NetworkRoute::from_proxy(Some(" ")), NetworkRoute::Direct); + assert_eq!( + NetworkRoute::from_proxy(Some("http://proxy.example:8080")), + NetworkRoute::Proxy("http://proxy.example:8080".to_string()) + ); + assert_eq!(NetworkRoute::from_proxy(None).ytdlp_proxy_value(), None); + assert_eq!( + NetworkRoute::from_proxy(Some("none")).ytdlp_proxy_value(), + Some("") + ); + assert_eq!( + NetworkRoute::from_proxy(Some("http://proxy.example:8080")).ytdlp_proxy_value(), + Some("http://proxy.example:8080") + ); + assert_eq!( + NetworkRoute::from_proxy(Some("none")) + .aria2_proxy_value() + .unwrap() + .as_deref(), + Some("") + ); + assert!( + NetworkRoute::from_proxy(Some("socks5://proxy.example:1080")) + .aria2_proxy_value() + .is_err() + ); + } + + #[test] + fn credentials_can_be_rejected_by_the_consumer_policy() { + let url = Url::parse("https://user:pass@example.com/file").unwrap(); + assert_eq!( + validate_url( + &url, + &["http", "https"], + CredentialPolicy::Reject("credentials are not allowed") + ), + Err("credentials are not allowed".to_string()) + ); + } +} diff --git a/src-tauri/src/queue.rs b/src-tauri/src/queue.rs index eaf1110..69e22ab 100644 --- a/src-tauri/src/queue.rs +++ b/src-tauri/src/queue.rs @@ -199,6 +199,13 @@ pub(crate) fn normalize_torrent_web_seed_uri(value: &str) -> Result bool { .is_some_and(|scheme| scheme.eq_ignore_ascii_case("magnet:")) } +fn proxy_uses_direct_network(proxy: Option<&str>) -> bool { + proxy.is_none_or(|proxy| { + let proxy = proxy.trim(); + proxy.is_empty() || proxy.eq_ignore_ascii_case("none") + }) +} + +fn payload_uses_direct_network(payload: &SpawnPayload) -> bool { + proxy_uses_direct_network(payload.proxy.as_deref()) +} + fn torrent_uses_direct_network(payload: &SpawnPayload) -> bool { payload.is_torrent && !payload.torrent_verify_only - && payload.proxy.as_deref().is_none_or(|proxy| { - let proxy = proxy.trim(); - proxy.is_empty() || proxy.eq_ignore_ascii_case("none") - }) + && payload_uses_direct_network(payload) } fn aria2_effective_resolver_mode(payload: &SpawnPayload) -> &'static str { if payload.aria2_resolver_mode == Aria2ResolverMode::System - || torrent_uses_direct_network(payload) + && payload_uses_direct_network(payload) { "system" } else { @@ -1110,10 +1125,12 @@ enum SeedAdmissionOutcome { /// (String/Option) to match the existing command signatures exactly. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum Aria2ResolverMode { - /// Use Aria2's normal resolver configuration. This is the initial mode - /// and deliberately leaves daemon-wide behavior unchanged. + /// Use Aria2's normal asynchronous resolver configuration. This is the + /// bounded alternate mode for a direct transfer after the system route + /// cannot resolve the target. Automatic, - /// Use the host operating system resolver for this transfer. + /// Use the host operating system resolver for this direct transfer. This + /// is the initial mode so TUN/VPN DNS interception remains authoritative. System, } @@ -1476,10 +1493,14 @@ impl QueueManager { .store(supported, Ordering::Relaxed); } - fn aria2_system_resolver_fallback_available(&self) -> bool { + pub(crate) fn aria2_async_dns_supported(&self) -> bool { self.aria2_async_dns_supported.load(Ordering::Relaxed) } + fn aria2_alternate_resolver_available(&self) -> bool { + self.aria2_async_dns_supported() + } + /// Accept one lifecycle-fenced Aria2 status sample and return Firelink's /// monotonic lifetime counters. Poller callers must already have checked /// the mapping; the key and epoch checks here provide a second fence at @@ -6045,9 +6066,9 @@ impl QueueManager { &payload, &error, strike, - self.aria2_system_resolver_fallback_available(), + self.aria2_alternate_resolver_available(), ); - let resolver_fallback = retry_action == Aria2RetryAction::SystemResolverFallback; + let resolver_fallback = retry_action == Aria2RetryAction::AlternateResolverFallback; let requested_connections = self .aria2_requested_connections(&id) .await @@ -6057,7 +6078,7 @@ impl QueueManager { .await .unwrap_or(requested_connections); let action = match retry_action { - Aria2RetryAction::SystemResolverFallback => "system_resolver_fallback", + Aria2RetryAction::AlternateResolverFallback => "alternate_resolver_fallback", Aria2RetryAction::OrdinaryRetry => "ordinary_retry", Aria2RetryAction::Terminal => "terminal", }; @@ -6125,7 +6146,7 @@ impl QueueManager { } if resolver_fallback { - payload.aria2_resolver_mode = Aria2ResolverMode::System; + payload.aria2_resolver_mode = Aria2ResolverMode::Automatic; self.aria2_payloads .lock() .await @@ -6613,19 +6634,20 @@ fn is_retryable_aria2_error_for_payload(payload: &SpawnPayload, error: &str) -> && is_aria2_low_speed_error(error)) } -fn should_use_aria2_system_resolver_fallback( +fn should_use_aria2_alternate_resolver_fallback( payload: &SpawnPayload, error: &str, async_dns_supported: bool, ) -> bool { async_dns_supported - && payload.aria2_resolver_mode == Aria2ResolverMode::Automatic + && payload.aria2_resolver_mode == Aria2ResolverMode::System + && payload_uses_direct_network(payload) && is_aria2_name_resolution_error(error) } #[derive(Debug, Clone, Copy, PartialEq, Eq)] enum Aria2RetryAction { - SystemResolverFallback, + AlternateResolverFallback, OrdinaryRetry, Terminal, } @@ -6636,8 +6658,8 @@ fn aria2_retry_action( strike: usize, async_dns_supported: bool, ) -> Aria2RetryAction { - if should_use_aria2_system_resolver_fallback(payload, error, async_dns_supported) { - return Aria2RetryAction::SystemResolverFallback; + if should_use_aria2_alternate_resolver_fallback(payload, error, async_dns_supported) { + return Aria2RetryAction::AlternateResolverFallback; } if is_retryable_aria2_error_for_payload(payload, error) && strike < automatic_retry_limit(payload.max_tries) @@ -6797,7 +6819,11 @@ async fn prepare_normal_transfer( if !is_http_uri(&uri) { let parsed = reqwest::Url::parse(&uri).map_err(|_| "SSRF blocked: Invalid URL".to_string())?; - crate::resolve_and_validate_url_host(&parsed).await?; + crate::network::validate_url( + &parsed, + &["http", "https", "ftp", "sftp"], + crate::network::CredentialPolicy::Allow, + )?; let uri_credentials_allowed = can_forward_payload_credentials(&credential_origin, &parsed); if index > 0 && payload_has_credential_material(payload) @@ -6994,26 +7020,8 @@ fn uri_host_for_log(uri: &str) -> String { .unwrap_or_else(|| "".to_string()) } -fn proxy_scheme(proxy: &str) -> Option { - proxy - .split_once("://") - .map(|(scheme, _)| scheme.trim().to_ascii_lowercase()) -} - pub(crate) fn aria2_all_proxy_value(proxy: &str) -> Result, String> { - let proxy = proxy.trim(); - if proxy.is_empty() { - return Ok(None); - } - if proxy.eq_ignore_ascii_case("none") { - return Ok(Some(String::new())); - } - if proxy_scheme(proxy).is_some_and(|scheme| scheme.starts_with("socks")) { - return Err( - "SOCKS system proxies are not supported for normal file downloads because aria2 only accepts HTTP/HTTPS/FTP proxy URLs. Use an HTTP proxy endpoint for normal downloads, or use media downloads where yt-dlp supports SOCKS.".to_string(), - ); - } - Ok(Some(proxy.to_string())) + crate::network::NetworkRoute::from_proxy(Some(proxy)).aria2_proxy_value() } pub(crate) fn proxy_route_for_log(proxy: Option<&str>) -> &'static str { @@ -7052,45 +7060,22 @@ async fn probe_bounded_range_support_with_local_override( let mut current = reqwest::Url::parse(uri).map_err(|error| error.to_string())?; let mut credentials_allowed = can_forward_payload_credentials(credential_origin, ¤t); for redirect_count in 0..=5 { - let (host, address) = if allow_localhost { - let host = current - .host_str() - .ok_or_else(|| "range probe test URL has no host".to_string())?; - let ip = host - .parse::() - .map_err(|_| "range probe test URL must use an IP host".to_string())?; - ( - host.to_string(), - std::net::SocketAddr::new( - ip, - current.port_or_known_default().unwrap_or(80), - ), - ) - } else { - crate::resolve_and_validate_url_host(¤t).await? - }; + if !allow_localhost { + crate::network::validate_url( + ¤t, + &["http", "https"], + crate::network::CredentialPolicy::Allow, + )?; + } let mut builder = reqwest::Client::builder() .redirect(reqwest::redirect::Policy::none()) - .timeout(std::time::Duration::from_secs(10)) - .resolve(&host, address); - - if allow_localhost { - builder = builder.no_proxy(); - } - - if let Some(proxy) = payload - .proxy - .as_deref() - .map(str::trim) - .filter(|value| !value.is_empty()) - { - if proxy.eq_ignore_ascii_case("none") { - builder = builder.no_proxy(); - } else { - builder = - builder.proxy(reqwest::Proxy::all(proxy).map_err(|error| error.to_string())?); - } - } + .timeout(std::time::Duration::from_secs(10)); + let route = if allow_localhost { + crate::network::NetworkRoute::Direct + } else { + crate::network::NetworkRoute::from_proxy(payload.proxy.as_deref()) + }; + builder = route.configure_reqwest(builder)?; let client = builder.build().map_err(|error| error.to_string())?; let request = client @@ -7288,11 +7273,15 @@ async fn validate_aria2_transfer_network_policy( message: "Unsupported URL scheme".to_string(), }); } - // This is deliberately repeated immediately before addUri/addTorrent - // so admission-time DNS is not the only policy check. Aria2 still - // resolves independently later; Firelink therefore does not claim - // that this is an IP pinning boundary. - if let Err(error) = crate::resolve_and_validate_url_host(&parsed).await { + // Keep this check immediately before addUri so every retry and every + // mirror is fenced by the same literal-target policy. Hostname DNS + // belongs to the selected consumer route and is intentionally not + // performed here. + if let Err(error) = crate::network::validate_url( + &parsed, + &["http", "https", "ftp", "sftp"], + crate::network::CredentialPolicy::Allow, + ) { return Err(Aria2NetworkPolicyError { host: uri_host_for_log(uri), message: error, @@ -7420,9 +7409,11 @@ fn apply_aria2_normal_reliability_options( fn apply_aria2_resolver_options( options: &mut serde_json::Map, - mode: Aria2ResolverMode, + payload: &SpawnPayload, ) { - if mode == Aria2ResolverMode::System { + if payload.aria2_resolver_mode == Aria2ResolverMode::System + && payload_uses_direct_network(payload) + { options.insert("async-dns".to_string(), serde_json::json!("false")); } } @@ -8011,7 +8002,7 @@ pub(crate) fn normalize_torrent_tracker_uri(value: &str) -> Result Result) -> Result Result<(), String> { + for tracker in trackers { + normalize_torrent_tracker_uri(tracker)?; + } + Ok(()) +} + pub(crate) fn normalize_torrent_exclude_trackers( value: Option<&str>, ) -> Result, String> { @@ -8212,7 +8215,9 @@ fn apply_aria2_torrent_options( options.insert("bt-metadata-only".to_string(), serde_json::json!("false")); options.insert("bt-save-metadata".to_string(), serde_json::json!("false")); options.insert("follow-torrent".to_string(), serde_json::json!("false")); - if torrent_uses_direct_network(payload) { + if torrent_uses_direct_network(payload) + && payload.aria2_resolver_mode == Aria2ResolverMode::System + { // Aria2's async resolver is independent of the system resolver. TUN // clients commonly install DNS interception/routing at the system // resolver boundary, so direct Torrent discovery must use the same @@ -8569,7 +8574,7 @@ impl SidecarSpawner for ProductionSpawner { if let Some(prox) = proxy_value { options.insert("all-proxy".to_string(), serde_json::json!(prox)); } - apply_aria2_resolver_options(&mut options, payload.aria2_resolver_mode); + apply_aria2_resolver_options(&mut options, payload); let retry_strike = state.queue_manager.aria2_retry_strike(id).await; let transfer_host = transfer_uris .first() @@ -8604,6 +8609,8 @@ impl SidecarSpawner for ProductionSpawner { crate::torrent::sanitize_torrent_bytes_for_aria2(&bytes)?; let embedded_web_seeds = crate::filter_torrent_web_seed_destinations(&embedded_web_seeds).await?; + let torrent_details = crate::torrent::torrent_details_from_bytes(&sanitized_bytes)?; + validate_torrent_tracker_destinations(&torrent_details.trackers)?; let metadata = crate::torrent::parse_torrent_bytes(&sanitized_bytes)?; options.insert( "index-out".to_string(), @@ -9251,6 +9258,11 @@ impl EnqueueItem { let mut item = self; item.strip_torrent_credentials(); let media = item.is_media.unwrap_or(false); + let aria2_resolver_mode = if proxy_uses_direct_network(item.proxy.as_deref()) { + Aria2ResolverMode::System + } else { + Aria2ResolverMode::Automatic + }; let kind = if media { TaskKind::Media } else { @@ -9287,7 +9299,7 @@ impl EnqueueItem { retry_not_found_errors: item.retry_not_found_errors.unwrap_or(false), adaptive_mirror_selection: item.adaptive_mirror_selection.unwrap_or(true), proxy: item.proxy, - aria2_resolver_mode: Aria2ResolverMode::Automatic, + aria2_resolver_mode, format_selector: item.format_selector, cookie_source: item.cookie_source, is_media: media, @@ -9889,14 +9901,56 @@ mod tests { #[test] fn aria2_system_resolver_mode_is_per_transfer_and_preserves_automatic_default() { let mut automatic = serde_json::Map::new(); - apply_aria2_resolver_options(&mut automatic, Aria2ResolverMode::Automatic); + apply_aria2_resolver_options( + &mut automatic, + &SpawnPayload { + aria2_resolver_mode: Aria2ResolverMode::Automatic, + ..Default::default() + }, + ); assert!(!automatic.contains_key("async-dns")); let mut system = serde_json::Map::new(); - apply_aria2_resolver_options(&mut system, Aria2ResolverMode::System); + apply_aria2_resolver_options( + &mut system, + &SpawnPayload { + aria2_resolver_mode: Aria2ResolverMode::System, + ..Default::default() + }, + ); assert_eq!(system.get("async-dns"), Some(&serde_json::json!("false"))); } + #[test] + fn enqueue_route_selects_system_resolver_for_direct_transfers() { + let direct: EnqueueItem = serde_json::from_value(serde_json::json!({ + "id": "direct", + "queue_id": "main", + "url": "https://example.com/file", + "destination": "/tmp", + "filename": "file" + })) + .unwrap(); + assert_eq!( + direct.into_task().payload.aria2_resolver_mode, + Aria2ResolverMode::System + ); + + let proxied: EnqueueItem = serde_json::from_value(serde_json::json!({ + "id": "proxied", + "queue_id": "main", + "url": "https://example.com/file", + "destination": "/tmp", + "filename": "file", + "proxy": "http://proxy.example:8080" + })) + .unwrap(); + assert_eq!( + proxied.into_task().payload.aria2_resolver_mode, + Aria2ResolverMode::Automatic + ); + } + #[test] fn resolver_state_events_are_typed_and_redacted() { let event = DownloadStateEvent::failed( @@ -10144,6 +10198,7 @@ mod tests { &mut options, &SpawnPayload { is_torrent: true, + aria2_resolver_mode: Aria2ResolverMode::System, ..Default::default() }, ) @@ -10180,6 +10235,7 @@ mod tests { assert_eq!( aria2_effective_resolver_mode(&SpawnPayload { is_torrent: true, + aria2_resolver_mode: Aria2ResolverMode::System, ..Default::default() }), "system" @@ -10194,6 +10250,38 @@ mod tests { ); } + #[test] + fn resolver_options_never_force_local_dns_for_configured_proxy_routes() { + let mut direct = serde_json::Map::new(); + apply_aria2_resolver_options( + &mut direct, + &SpawnPayload { + aria2_resolver_mode: Aria2ResolverMode::System, + ..Default::default() + }, + ); + assert_eq!(direct.get("async-dns"), Some(&serde_json::json!("false"))); + + let mut proxied = serde_json::Map::new(); + apply_aria2_resolver_options( + &mut proxied, + &SpawnPayload { + aria2_resolver_mode: Aria2ResolverMode::System, + proxy: Some("http://proxy.example:8080".to_string()), + ..Default::default() + }, + ); + assert!(!proxied.contains_key("async-dns")); + assert_eq!( + aria2_effective_resolver_mode(&SpawnPayload { + aria2_resolver_mode: Aria2ResolverMode::System, + proxy: Some("http://proxy.example:8080".to_string()), + ..Default::default() + }), + "automatic" + ); + } + #[test] fn generic_aria2_downloads_disable_followed_child_gids() { let mut options = serde_json::Map::new(); @@ -10571,11 +10659,20 @@ mod tests { "ftp://tracker.example/announce", "https://user:pass@tracker.example/announce", "https://tracker.example/announce#fragment", + "https://127.0.0.1/announce", + "https://localhost/announce", "https://tracker.example/announce,", "https://", ] { assert!(normalize_torrent_trackers(Some(value)).is_err(), "{value}"); } + assert_eq!( + normalize_torrent_trackers(Some( + "https://this-tracker-does-not-resolve.invalid/announce" + )) + .unwrap(), + Some("https://this-tracker-does-not-resolve.invalid/announce".to_string()) + ); let too_many = (0..=MAX_TORRENT_TRACKERS) .map(|index| format!("https://tracker{index}.example/announce")) .collect::>() @@ -10600,6 +10697,8 @@ mod tests { "sftp://mirror.example/file", "https://user:pass@mirror.example/file", "https://mirror.example/file#fragment", + "https://127.0.0.1/file", + "https://mirror.localhost/file", ] { assert!( normalize_torrent_mirror_uris(Some(value)).is_err(), @@ -10633,6 +10732,25 @@ mod tests { ); } + #[test] + fn torrent_tracker_destination_validation_keeps_dns_route_owned() { + assert!(validate_torrent_tracker_destinations(&[ + "https://tracker-does-not-resolve.invalid/announce".to_string(), + "udp://tracker-does-not-resolve.invalid:6969/announce".to_string(), + ]) + .is_ok()); + for tracker in [ + "https://127.0.0.1/announce", + "https://[::1]/announce", + "udp://localhost:6969/announce", + ] { + assert!( + validate_torrent_tracker_destinations(&[tracker.to_string()]).is_err(), + "{tracker}" + ); + } + } + #[test] fn aria2_web_seed_readback_keeps_file_ownership_separate() { let files = vec![ @@ -12268,24 +12386,28 @@ mod tests { fn aria2_name_resolution_error_is_retryable_for_resolver_recovery() { let error = "aria2 error code 19: Name resolution for example.test failed: Could not contact DNS servers."; assert!(is_retryable_aria2_error(error)); - assert!(should_use_aria2_system_resolver_fallback( - &SpawnPayload::default(), + let direct_initial = SpawnPayload { + aria2_resolver_mode: Aria2ResolverMode::System, + ..Default::default() + }; + assert!(should_use_aria2_alternate_resolver_fallback( + &direct_initial, error, true, )); assert_eq!( - aria2_retry_action(&SpawnPayload::default(), error, 0, true), - Aria2RetryAction::SystemResolverFallback + aria2_retry_action(&direct_initial, error, 0, true), + Aria2RetryAction::AlternateResolverFallback ); - assert!(!should_use_aria2_system_resolver_fallback( + assert!(!should_use_aria2_alternate_resolver_fallback( &SpawnPayload { - aria2_resolver_mode: Aria2ResolverMode::System, + aria2_resolver_mode: Aria2ResolverMode::Automatic, ..Default::default() }, error, true, )); - assert!(!should_use_aria2_system_resolver_fallback( + assert!(!should_use_aria2_alternate_resolver_fallback( &SpawnPayload::default(), error, false, @@ -12301,12 +12423,12 @@ mod tests { 0, true, ), - Aria2RetryAction::Terminal + Aria2RetryAction::AlternateResolverFallback ); assert_eq!( aria2_retry_action( &SpawnPayload { - aria2_resolver_mode: Aria2ResolverMode::System, + aria2_resolver_mode: Aria2ResolverMode::Automatic, max_tries: Some(1), ..Default::default() }, diff --git a/src-tauri/src/torrent.rs b/src-tauri/src/torrent.rs index 93b87dc..51dd778 100644 --- a/src-tauri/src/torrent.rs +++ b/src-tauri/src/torrent.rs @@ -382,6 +382,7 @@ pub fn sanitize_torrent_bytes_for_aria2(bytes: &[u8]) -> Result<(Vec, Vec value, _ => return Err("torrent root is not a dictionary".to_string()), }; + validate_torrent_tracker_metadata(bytes)?; let web_seeds = parse_torrent_web_seeds(root.get(b"url-list".as_slice()))?; let mut sanitized = root; sanitized.remove(b"url-list".as_slice()); @@ -486,6 +487,27 @@ fn torrent_tracker_metadata_is_safe(root: &BTreeMap, BencodeValue>) -> b true } +/// Validate the tracker fields before handing original metainfo to Aria2. +/// `torrent_details_from_bytes` intentionally omits malformed tracker values +/// from its display projection, but Aria2 consumes the original bencode and +/// would otherwise still see those values. +pub fn validate_torrent_tracker_metadata(bytes: &[u8]) -> Result<(), String> { + if bytes.is_empty() || bytes.len() > MAX_TORRENT_BYTES { + return Err(format!( + "torrent metadata must be between 1 byte and {MAX_TORRENT_BYTES} bytes" + )); + } + let root = match Parser::new(bytes).parse()? { + BencodeValue::Dict(value) => value, + _ => return Err("torrent root is not a dictionary".to_string()), + }; + if torrent_tracker_metadata_is_safe(&root) { + Ok(()) + } else { + Err("torrent metadata contains an invalid tracker URI".to_string()) + } +} + fn parse_torrent_web_seeds(value: Option<&BencodeValue>) -> Result, String> { let Some(value) = value else { return Ok(Vec::new()); @@ -1529,6 +1551,26 @@ mod tests { .expect("web-seed-bearing torrent metadata should parse")); } + #[test] + fn tracker_metadata_syntax_rejects_malformed_values_before_aria2() { + assert!(validate_torrent_tracker_metadata( + b"d8:announce30:ftp://tracker.example/announce4:infod6:lengthi5e4:name4:testee" + ) + .is_err()); + assert!(validate_torrent_tracker_metadata( + b"d8:announce25:http://127.0.0.1/announce4:infod6:lengthi5e4:name4:testee" + ) + .is_ok()); + assert!(validate_torrent_tracker_metadata( + b"d8:announce32:https://tracker.example/announce4:infod6:lengthi5e4:name4:testee" + ) + .is_ok()); + assert!(sanitize_torrent_bytes_for_aria2( + b"d8:announce30:ftp://tracker.example/announce4:infod6:lengthi5e4:name4:testee" + ) + .is_err()); + } + #[test] fn canonical_cache_temporary_names_are_strictly_recognized() { assert!(is_canonical_torrent_temp_file( diff --git a/src-tauri/src/torrent_probe.rs b/src-tauri/src/torrent_probe.rs index 5f453fe..2e77d13 100644 --- a/src-tauri/src/torrent_probe.rs +++ b/src-tauri/src/torrent_probe.rs @@ -20,12 +20,40 @@ pub(crate) enum ProbeFailure { Cleanup(String), } +#[allow(dead_code)] pub(crate) async fn run_metadata_probe( + client: Arc, + source: &str, + options: Map, + metadata_path: &Path, + timeout: Duration, + poll_interval: Duration, +) -> Result, ProbeFailure> { + let deadline = Instant::now() + timeout; + run_metadata_probe_with_deadlines( + client, + source, + options, + metadata_path, + deadline, + deadline, + poll_interval, + ) + .await +} + +/// Run one metadata probe with separate metadata and cleanup deadlines. +/// +/// The split is important for resolver fallback: the first resolver gets its +/// own bounded attempt, while cleanup of its GID consumes only the shared +/// operation deadline before a second resolver may be started. +pub(crate) async fn run_metadata_probe_with_deadlines( client: Arc, source: &str, mut options: Map, metadata_path: &Path, - timeout: Duration, + metadata_deadline: Instant, + cleanup_deadline: Instant, poll_interval: Duration, ) -> Result, ProbeFailure> { // This probe only resolves magnet metadata. It must never allow Aria2 to @@ -34,18 +62,27 @@ pub(crate) async fn run_metadata_probe( options.insert("follow-torrent".to_string(), json!("false")); options.insert("follow-metalink".to_string(), json!("false")); let mut cleanup_guard = ProbeCleanupGuard::new(Arc::clone(&client), metadata_path); - let result = match client - .call("aria2.addUri", json!([[source], options])) - .await - { - Ok(result) => result, - Err(error) => { + let add_result = tokio::time::timeout( + metadata_deadline.saturating_duration_since(Instant::now()), + client.call("aria2.addUri", json!([[source], options])), + ) + .await; + let result = match add_result { + Err(_) => { + cleanup_guard.disarm(); + return Err(ProbeFailure::Metadata( + "Aria2 could not start magnet metadata resolution before the resolver attempt deadline" + .to_string(), + )); + } + Ok(Err(error)) => { cleanup_guard.disarm(); return Err(ProbeFailure::Metadata(format!( "Aria2 could not start magnet metadata resolution: {}", crate::redact_sensitive_text(&error) ))); } + Ok(Ok(result)) => result, }; let gid = match result.as_str().filter(|value| !value.is_empty()) { Some(gid) => gid.to_string(), @@ -59,33 +96,42 @@ pub(crate) async fn run_metadata_probe( cleanup_guard.set_gid(gid.clone()); let metadata_result = async { - let deadline = Instant::now() + timeout; loop { - let status = match client - .call( + let status = match tokio::time::timeout( + metadata_deadline.saturating_duration_since(Instant::now()), + client.call( "aria2.tellStatus", json!([&gid, ["status", "errorCode", "errorMessage"]]), - ) - .await + ), + ) + .await { - Ok(status) => status, - Err(error) if crate::aria2_gid_not_found(&error) => { + Err(_) => { + return Err(ProbeFailure::Metadata( + "Aria2 magnet metadata resolution timed out".to_string(), + )); + } + Ok(Ok(status)) => status, + Ok(Err(error)) if crate::aria2_gid_not_found(&error) => { return Err(ProbeFailure::Metadata( "Aria2 removed the magnet metadata probe before metadata was saved" .to_string(), )); } - Err(error) if crate::retry::is_transient_network_error(&error) => { - if Instant::now() >= deadline { + Ok(Err(error)) if crate::retry::is_transient_network_error(&error) => { + if Instant::now() >= metadata_deadline { return Err(ProbeFailure::Metadata(format!( "Aria2 metadata resolution status failed: {}", crate::redact_sensitive_text(&error) ))); } - tokio::time::sleep(poll_interval).await; + tokio::time::sleep( + poll_interval.min(metadata_deadline.saturating_duration_since(Instant::now())), + ) + .await; continue; } - Err(error) => { + Ok(Err(error)) => { return Err(ProbeFailure::Metadata(format!( "Aria2 metadata resolution status failed: {}", crate::redact_sensitive_text(&error) @@ -125,24 +171,42 @@ pub(crate) async fn run_metadata_probe( ))); } } - if Instant::now() >= deadline { + if Instant::now() >= metadata_deadline { return Err(ProbeFailure::Metadata( "Aria2 magnet metadata resolution timed out".to_string(), )); } - tokio::time::sleep(poll_interval).await; + tokio::time::sleep( + poll_interval.min(metadata_deadline.saturating_duration_since(Instant::now())), + ) + .await; } - tokio::fs::read(metadata_path).await.map_err(|error| { - ProbeFailure::Metadata(format!( + let remaining = metadata_deadline.saturating_duration_since(Instant::now()); + match tokio::time::timeout(remaining, tokio::fs::read(metadata_path)).await { + Err(_) => Err(ProbeFailure::Metadata( + "Aria2 magnet metadata file read timed out".to_string(), + )), + Ok(Ok(bytes)) => Ok(bytes), + Ok(Err(error)) => Err(ProbeFailure::Metadata(format!( "Aria2 did not save magnet metadata ({:?})", error.kind() - )) - }) + ))), + } } .await; - let cleanup_result = cleanup_metadata_probe(client.as_ref(), &gid).await; + let cleanup_result = match tokio::time::timeout( + cleanup_deadline.saturating_duration_since(Instant::now()), + cleanup_metadata_probe(client.as_ref(), &gid), + ) + .await + { + Ok(result) => result, + Err(_) => Err(format!( + "failed to remove aria2 gid {gid} before the metadata operation deadline" + )), + }; if let Err(error) = cleanup_result { return Err(ProbeFailure::Cleanup(error)); } @@ -150,6 +214,139 @@ pub(crate) async fn run_metadata_probe( metadata_result } +/// Run direct magnet metadata discovery through the system resolver first, +/// then use Aria2's alternate resolver only after the first probe has failed +/// and its GID has been cleaned up. Both attempts share one absolute deadline. +pub(crate) async fn run_metadata_probe_with_resolver_fallback( + client: Arc, + source: &str, + initial_options: Map, + metadata_path: &Path, + initial_resolver_mode: &'static str, + fallback_to_automatic: bool, + total_timeout: Duration, + initial_timeout: Duration, + poll_interval: Duration, +) -> Result, ProbeFailure> { + let operation_started = Instant::now(); + let operation_deadline = operation_started + total_timeout; + let initial_deadline = (operation_started + initial_timeout).min(operation_deadline); + + log::debug!( + "magnet metadata probe [resolver_mode={initial_resolver_mode} outcome=started error_class=none elapsed_ms=0]" + ); + let first_started = Instant::now(); + let first_result = run_metadata_probe_with_deadlines( + Arc::clone(&client), + source, + initial_options.clone(), + metadata_path, + initial_deadline, + operation_deadline, + poll_interval, + ) + .await; + log_probe_result(initial_resolver_mode, first_started, &first_result); + + let should_try_alternate = initial_resolver_mode == "system" + && fallback_to_automatic + && probe_failure_allows_resolver_fallback(&first_result) + && Instant::now() < operation_deadline; + if !should_try_alternate { + return first_result; + } + + // The first probe has completed its lifecycle cleanup. Remove any + // partial metadata before reusing the path so a stale result can never be + // adopted by the alternate resolver. + let clear_result = match tokio::time::timeout( + operation_deadline.saturating_duration_since(Instant::now()), + tokio::fs::remove_file(metadata_path), + ) + .await + { + Ok(Ok(())) => Ok(()), + Ok(Err(error)) if error.kind() == std::io::ErrorKind::NotFound => Ok(()), + Ok(Err(error)) => Err(format!("filesystem error ({:?})", error.kind())), + Err(_) => Err("timed out before the metadata operation deadline".to_string()), + }; + if let Err(error) = clear_result { + let cleanup_failure = ProbeFailure::Cleanup(format!( + "could not clear stale magnet metadata before resolver fallback: {error}" + )); + log::debug!( + "magnet metadata probe [resolver_mode=automatic outcome=cleanup-failure error_class=cleanup elapsed_ms={}]", + Instant::now().duration_since(operation_started).as_millis() + ); + return Err(cleanup_failure); + } + + let mut alternate_options = initial_options; + alternate_options.remove("async-dns"); + log::debug!( + "magnet metadata probe [resolver_mode=automatic outcome=started error_class=none elapsed_ms={}]", + operation_started.elapsed().as_millis() + ); + let alternate_started = Instant::now(); + let alternate_result = run_metadata_probe_with_deadlines( + client, + source, + alternate_options, + metadata_path, + operation_deadline, + operation_deadline, + poll_interval, + ) + .await; + log_probe_result("automatic", alternate_started, &alternate_result); + alternate_result +} + +fn probe_failure_allows_resolver_fallback(result: &Result, ProbeFailure>) -> bool { + let Err(ProbeFailure::Metadata(error)) = result else { + return false; + }; + // An alternate resolver can only repair a resolver-owned failure. Keep + // malformed metadata, missing output, daemon/status errors, cancellation, + // and every cleanup uncertainty on the original result. The generic + // operation timeout is emitted only by the bounded add/status polling + // path; file-read and addUri-start deadlines use distinct messages so an + // uncertain GID is never duplicated by a fallback attempt. + crate::retry::is_aria2_name_resolution_error(error) + || error == "Aria2 magnet metadata resolution timed out" +} + +fn log_probe_result( + resolver_mode: &str, + started: Instant, + result: &Result, ProbeFailure>, +) { + let (outcome, error_class) = match result { + Ok(_) => ("probe-success", "none"), + Err(ProbeFailure::Metadata(error)) => ("metadata-failure", probe_error_class(error)), + Err(ProbeFailure::Cleanup(error)) => ("cleanup-failure", probe_error_class(error)), + }; + log::debug!( + "magnet metadata probe [resolver_mode={resolver_mode} outcome={outcome} error_class={error_class} elapsed_ms={}]", + started.elapsed().as_millis() + ); +} + +fn probe_error_class(error: &str) -> &'static str { + let lower = error.to_ascii_lowercase(); + if lower.contains("cleanup") || lower.contains("remove aria2 gid") { + "cleanup" + } else if lower.contains("timed out") || lower.contains("timeout") { + "timeout" + } else if lower.contains("dns") || lower.contains("name resolution") { + "name_resolution" + } else if lower.contains("metadata") || lower.contains("torrent") { + "metadata" + } else { + "probe" + } +} + struct ProbeCleanupGuard { client: Arc, gid: Option, @@ -943,6 +1140,271 @@ mod tests { ScriptedReply::Result(json!({ "status": status })) } + #[tokio::test(flavor = "current_thread")] + async fn resolver_fallback_cleans_the_first_gid_and_rejects_stale_metadata() { + let server = ScriptedRpcServer::start(scripts([ + ( + "aria2.addUri", + vec![ + ScriptedReply::Result(json!("gid-1")), + ScriptedReply::Delay( + Duration::from_millis(50), + Box::new(ScriptedReply::Result(json!("gid-2"))), + ), + ], + ), + ( + "aria2.tellStatus", + vec![ + ScriptedReply::Result(json!({ + "status": "error", + "errorCode": "19", + "errorMessage": "Name resolution failed", + })), + status_reply("removed"), + status_reply("complete"), + status_reply("removed"), + ], + ), + ( + "aria2.forceRemove", + vec![ + ScriptedReply::Result(json!("gid-1")), + ScriptedReply::Result(json!("gid-2")), + ], + ), + ])) + .await; + let (_temporary, probe_dir, metadata_path) = probe_fixture().await; + let probe_path = metadata_path.clone(); + let client = server.client(); + let task = tokio::spawn(async move { + let mut options = Map::new(); + options.insert("async-dns".to_string(), json!("false")); + run_metadata_probe_with_resolver_fallback( + client, + "magnet:?xt=urn:btih:0123456789abcdef0123456789abcdef01234567", + options, + &probe_path, + "system", + true, + Duration::from_secs(2), + Duration::from_millis(200), + Duration::ZERO, + ) + .await + }); + + server.wait_for_method("aria2.addUri", 2).await; + assert!( + !metadata_path.exists(), + "the first probe's stale metadata must be removed before fallback" + ); + tokio::fs::write(&metadata_path, b"metadata from alternate resolver") + .await + .expect("alternate metadata fixture should be writable"); + let bytes = tokio::time::timeout(Duration::from_secs(1), task) + .await + .expect("resolver fallback should remain bounded") + .expect("resolver fallback task should not panic") + .expect("alternate resolver should complete the probe"); + assert_eq!(bytes, b"metadata from alternate resolver"); + + let calls = server.calls(); + let add_calls = calls + .iter() + .filter(|(method, _)| method == "aria2.addUri") + .collect::>(); + assert_eq!(add_calls.len(), 2); + let first_options = add_calls[0] + .1 + .as_array() + .and_then(|params| params.get(2)) + .and_then(Value::as_object) + .expect("first addUri options should be recorded"); + assert_eq!(first_options.get("async-dns"), Some(&json!("false"))); + let alternate_options = add_calls[1] + .1 + .as_array() + .and_then(|params| params.get(2)) + .and_then(Value::as_object) + .expect("alternate addUri options should be recorded"); + assert!(!alternate_options.contains_key("async-dns")); + + let first_force_remove = calls + .iter() + .position(|(method, _)| method == "aria2.forceRemove") + .expect("first probe must be cleaned up"); + let second_add = calls + .iter() + .rposition(|(method, _)| method == "aria2.addUri") + .expect("alternate probe must be started"); + assert!( + first_force_remove < second_add, + "alternate probing must wait for first-GID cleanup" + ); + assert_eq!( + calls + .iter() + .filter(|(method, _)| method == "aria2.forceRemove") + .count(), + 2 + ); + + tokio::fs::remove_dir_all(&probe_dir) + .await + .expect("successful fallback fixture should be removable"); + server.shutdown().await; + } + + #[tokio::test(flavor = "current_thread")] + async fn resolver_fallback_does_not_repeat_invalid_metadata_failures() { + let server = ScriptedRpcServer::start(scripts([ + ("aria2.addUri", vec![ScriptedReply::Result(json!("gid-1"))]), + ( + "aria2.tellStatus", + vec![ + ScriptedReply::Result(json!({ + "status": "error", + "errorCode": "1", + "errorMessage": "tracker rejected metadata", + })), + status_reply("removed"), + ], + ), + ( + "aria2.forceRemove", + vec![ScriptedReply::Result(json!("gid-1"))], + ), + ])) + .await; + let (_temporary, probe_dir, metadata_path) = probe_fixture().await; + let error = run_metadata_probe_with_resolver_fallback( + server.client(), + "magnet:?xt=urn:btih:0123456789abcdef0123456789abcdef01234567", + Map::new(), + &metadata_path, + "system", + true, + Duration::from_secs(1), + Duration::from_millis(100), + Duration::ZERO, + ) + .await + .expect_err("invalid metadata failure should remain on the initial route"); + assert!(matches!(error, ProbeFailure::Metadata(message) if message.contains("could not resolve"))); + assert_eq!( + server + .calls() + .iter() + .filter(|(method, _)| method == "aria2.addUri") + .count(), + 1 + ); + tokio::fs::remove_dir_all(&probe_dir) + .await + .expect("failed probe fixture should be removable"); + server.shutdown().await; + } + + #[tokio::test(flavor = "current_thread")] + async fn resolver_fallback_does_not_run_after_cleanup_uncertainty() { + let server = ScriptedRpcServer::start(scripts([ + ("aria2.addUri", vec![ScriptedReply::Result(json!("gid-1"))]), + ( + "aria2.tellStatus", + vec![ + ScriptedReply::Result(json!({ + "status": "error", + "errorCode": "19", + "errorMessage": "Name resolution failed", + })), + status_reply("active"), + ], + ), + ( + "aria2.forceRemove", + vec![ScriptedReply::RpcError("temporary cleanup failure".to_string())], + ), + ])) + .await; + let (_temporary, probe_dir, metadata_path) = probe_fixture().await; + let error = run_metadata_probe_with_resolver_fallback( + server.client(), + "magnet:?xt=urn:btih:0123456789abcdef0123456789abcdef01234567", + Map::new(), + &metadata_path, + "system", + true, + Duration::from_secs(1), + Duration::from_millis(100), + Duration::ZERO, + ) + .await + .expect_err("cleanup uncertainty must stop resolver fallback"); + assert!(matches!(error, ProbeFailure::Cleanup(_))); + assert_eq!( + server + .calls() + .iter() + .filter(|(method, _)| method == "aria2.addUri") + .count(), + 1 + ); + tokio::fs::remove_dir_all(&probe_dir) + .await + .expect("uncertain probe fixture should be removable by the test"); + server.shutdown().await; + } + + #[tokio::test(flavor = "current_thread")] + async fn resolver_fallback_is_not_started_after_cancellation() { + let server = ScriptedRpcServer::start(scripts([ + ("aria2.addUri", vec![ScriptedReply::Result(json!("gid-1"))]), + ( + "aria2.tellStatus", + vec![ScriptedReply::Hang, status_reply("removed")], + ), + ( + "aria2.forceRemove", + vec![ScriptedReply::Result(json!("gid-1"))], + ), + ])) + .await; + let (_temporary, probe_dir, metadata_path) = probe_fixture().await; + let probe_path = metadata_path.clone(); + let client = server.client(); + let task = tokio::spawn(async move { + run_metadata_probe_with_resolver_fallback( + client, + "magnet:?xt=urn:btih:0123456789abcdef0123456789abcdef01234567", + Map::new(), + &probe_path, + "system", + true, + Duration::from_secs(60), + Duration::from_secs(20), + Duration::from_secs(60), + ) + .await + }); + server.wait_for_method("aria2.tellStatus", 1).await; + task.abort(); + let _ = task.await; + server.wait_for_method("aria2.forceRemove", 1).await; + assert_eq!( + server + .calls() + .iter() + .filter(|(method, _)| method == "aria2.addUri") + .count(), + 1, + "cancellation must not start the alternate resolver" + ); + wait_for_path(&probe_dir, false).await; + server.terminate().await; + } + #[tokio::test(flavor = "current_thread")] async fn http_rpc_harness_exercises_production_client_and_status_order() { let server = ScriptedRpcServer::start(scripts([ diff --git a/src-tauri/tests/queue_manager.rs b/src-tauri/tests/queue_manager.rs index 35c5d79..3548272 100644 --- a/src-tauri/tests/queue_manager.rs +++ b/src-tauri/tests/queue_manager.rs @@ -2304,9 +2304,9 @@ async fn resolver_failure_uses_one_system_fallback_without_retry_budget() { let manager = Arc::new(mgr); manager.set_aria2_async_dns_supported(true); let mut task = aria2_task("resolver-fallback"); + task.payload.aria2_resolver_mode = Aria2ResolverMode::System; task.payload.max_tries = Some(0); task.payload.headers = Some("X-Test: retained".to_string()); - task.payload.proxy = Some("http://127.0.0.1:8123".to_string()); manager.push(task).await.unwrap(); let dispatcher = { @@ -2341,32 +2341,33 @@ async fn resolver_failure_uses_one_system_fallback_without_retry_budget() { } }) .await - .expect("resolver failure should re-add once with the system resolver"); + .expect("resolver failure should re-add once with Aria2's alternate resolver"); assert_eq!( *spawner.add_resolver_modes.lock().unwrap(), - vec![Aria2ResolverMode::Automatic, Aria2ResolverMode::System] + vec![Aria2ResolverMode::System, Aria2ResolverMode::Automatic] ); assert_eq!( *spawner.add_transfer_context.lock().unwrap(), vec![ ( - Aria2ResolverMode::Automatic, + Aria2ResolverMode::System, Some("X-Test: retained".to_string()), - Some("http://127.0.0.1:8123".to_string()), + None, None, ), ( - Aria2ResolverMode::System, + Aria2ResolverMode::Automatic, Some("X-Test: retained".to_string()), - Some("http://127.0.0.1:8123".to_string()), + None, None, ), ] ); assert_eq!(spawner.add_uri_calls.load(Ordering::SeqCst), 2); - // A second resolver failure is now on the system mode. With max_tries=0 - // it must terminate instead of switching back or consuming another add. + // A second resolver failure is now on the alternate mode. With + // max_tries=0 it must terminate instead of switching back or consuming + // another add. manager .handle_aria2_event( "gid-2", diff --git a/src/components/AddDownloadsModal.tsx b/src/components/AddDownloadsModal.tsx index 2144c87..de4380e 100644 --- a/src/components/AddDownloadsModal.tsx +++ b/src/components/AddDownloadsModal.tsx @@ -877,8 +877,6 @@ export const AddDownloadsModal = () => { const metadataBlockedReason = [ 'SSRF blocked: Invalid URL', 'SSRF blocked: No host', - 'SSRF blocked: DNS resolution failed', - 'SSRF blocked: No DNS records', 'SSRF blocked: Private/local IP not allowed' ].some(prefix => errorMessage.startsWith(prefix)) ? 'unsafe-url' as const @@ -1625,10 +1623,12 @@ export const AddDownloadsModal = () => { } else if (!isMagnetUrl(item.sourceUrl)) { // Keep a safe fallback for rows restored from an older draft // shape that did not retain the preview cache identity. + const proxy = await getProxyArgs(useSettingsStore.getState()); const torrentData = await invoke('inspect_torrent', { source: item.sourceUrl, id, cache: true, + proxy: proxy ?? undefined, headers: headersForRow(contextUrl) || undefined, cookies: cookiesForRow(contextUrl, item.sourceUrl) || undefined, cookieScopes: requestContextForUrl(contextUrl)?.cookieScopes || undefined,