diff --git a/src/download.rs b/src/download.rs index 0fc29bd9..38a539c0 100644 --- a/src/download.rs +++ b/src/download.rs @@ -3031,6 +3031,15 @@ impl DownloadManager { pub fn remove(self: &Rc, id: u64) { self.cancel_inner(id, true); self.epoch.borrow_mut().remove(&id); + // Video rows stage split parts under staging_dir(id): drop them + // with the row. Pause/cancel keep them for resume; remove and + // delete never resume (an Undo'd row re-extracts fresh URLs and + // restarts), so orphaned parts would otherwise sit in /tmp until + // reboot. Live rows are exempt: their worker was only signaled, + // not aborted, and its finalize path cleans up after itself. + if !self.live_rows.borrow().contains(&id) && self.video_sources.borrow().contains_key(&id) { + crate::video::clean_staging(&crate::video::staging_dir(id)); + } // The snapshot carries the source for Undo; the live map drops it // with the row (cancel keeps it, remove doesn't). self.video_sources.borrow_mut().remove(&id); diff --git a/src/download_tests.rs b/src/download_tests.rs index 09d2d543..fc85af95 100644 --- a/src/download_tests.rs +++ b/src/download_tests.rs @@ -3073,3 +3073,79 @@ fn piece_bitmap_prefers_segment_then_torrent_then_bytes() { manager.torrent_pieces.borrow_mut().insert(8, haves.clone()); assert_eq!(manager.piece_bitmap(8), haves); } + +#[test] +fn remove_cleans_video_staging() { + let (_q, _l) = test_locks(); + let _qf = test_queue_file("remove-staging"); + let settings = test_settings(); + let manager = DownloadManager::new(gio::ListStore::new::(), settings); + let id = 910_000 + std::process::id() as u64; + let dir = crate::video::staging_dir(id); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join("video.mp4"), b"partial").unwrap(); + let item = DownloadItem::new(id, "https://x.com/u/status/1", "v.mp4", "/tmp/dl"); + manager.store().append(&item); + manager.video_sources.borrow_mut().insert( + id, + crate::video::VideoSource::Page { + page_url: "https://x.com/u/status/1".to_string(), + media_url: None, + expires_at: None, + quality: "1080p".to_string(), + audio_only: false, + is_live: false, + video_format_id: None, + }, + ); + manager.remove(id); + assert!(!dir.exists(), "staged parts must go with the row"); +} + +#[test] +fn remove_keeps_live_staging_for_finalize() { + // Live rows are only signaled on remove, not aborted: the worker's + // own finalize path owns staging cleanup, so remove must not race it. + let (_q, _l) = test_locks(); + let _qf = test_queue_file("remove-live-staging"); + let settings = test_settings(); + let manager = DownloadManager::new(gio::ListStore::new::(), settings); + let id = 920_000 + std::process::id() as u64; + let dir = crate::video::staging_dir(id); + std::fs::create_dir_all(&dir).unwrap(); + let item = DownloadItem::new(id, "https://x.com/u/status/1", "v.mp4", "/tmp/dl"); + manager.store().append(&item); + manager.video_sources.borrow_mut().insert( + id, + crate::video::VideoSource::Page { + page_url: "https://x.com/u/status/1".to_string(), + media_url: None, + expires_at: None, + quality: "1080p".to_string(), + audio_only: false, + is_live: true, + video_format_id: None, + }, + ); + manager.live_rows.borrow_mut().insert(id); + manager.remove(id); + assert!(dir.exists(), "live finalize still owns staging"); + let _ = std::fs::remove_dir_all(&dir); +} + +#[test] +fn remove_leaves_plain_rows_staging_alone() { + // Non-video rows never stage: no video source, no cleanup attempt. + let (_q, _l) = test_locks(); + let _qf = test_queue_file("remove-plain"); + let settings = test_settings(); + let manager = DownloadManager::new(gio::ListStore::new::(), settings); + let id = 930_000 + std::process::id() as u64; + let dir = crate::video::staging_dir(id); + std::fs::create_dir_all(&dir).unwrap(); + let item = DownloadItem::new(id, "https://example.com/a.bin", "a.bin", "/tmp/dl"); + manager.store().append(&item); + manager.remove(id); + assert!(dir.exists(), "plain rows must not trigger staging cleanup"); + let _ = std::fs::remove_dir_all(&dir); +} diff --git a/src/video.rs b/src/video.rs index bb743452..320b212e 100644 --- a/src/video.rs +++ b/src/video.rs @@ -1059,6 +1059,9 @@ async fn fetch_video_page( if let Some(spec) = cookies_browser_spec(cookies_browser) { args.push(format!("--cookies-from-browser={spec}")); } + // `--` before the page URL: option parsing ends here, so a hostile + // or malformed URL can never be read as a flag. + args.push("--".to_string()); args.push(url.to_string()); let executor = yt_dlp::executor::Executor::new(youtube_bin, args, timeout); let output = executor.execute().await.map_err(VideoError::fetch)?; @@ -1374,7 +1377,10 @@ fn quality_height(value: &str) -> Option { /// cap, else the tallest available. Mirrors the crate's /// closest-at-or-above preset semantics. fn select_hls_format(formats: &[Format], want: Option) -> Option { - let cands: Vec = formats.iter().filter_map(HlsSel::from_format).collect(); + let mut cands: Vec = formats.iter().filter_map(HlsSel::from_format).collect(); + // Extractor order is arbitrary: sort ascending so the capped match is + // genuinely the smallest height at or above it. + cands.sort_by_key(|s| s.height.unwrap_or(0)); match want { Some(h) => cands .iter() @@ -3020,6 +3026,9 @@ async fn run_hls_ytdlp( if !job.user_agent.is_empty() { cmd.arg("--user-agent").arg(&job.user_agent); } + // `--` before the page URL: option parsing ends here, so a hostile + // or malformed URL can never be read as a flag. + cmd.arg("--"); cmd.arg(&job.page_url); cmd.stdin(std::process::Stdio::null()) .stdout(std::process::Stdio::piped()) diff --git a/src/video_tests.rs b/src/video_tests.rs index c1b504b0..e4482463 100644 --- a/src/video_tests.rs +++ b/src/video_tests.rs @@ -1955,3 +1955,55 @@ fn quality_for_height_buckets() { ); } } + +// ── HLS candidate ordering ─────────────────────────────────────────── + +#[test] +fn hls_selection_ignores_extractor_order() { + // Tallest first (the order some extractors emit): the cap must still + // resolve to the smallest height at or above it, not the first + // qualifying entry. + let formats: Vec = serde_json::from_value(serde_json::json!([ + test_format_full( + "h1080", + "avc1", + "mp4a.40.2", + Some(1080), + None, + "m3u8_native", + false + ), + test_format_full( + "h720", + "avc1", + "mp4a.40.2", + Some(720), + None, + "m3u8_native", + false + ), + test_format_full( + "h480", + "avc1", + "mp4a.40.2", + Some(480), + None, + "m3u8_native", + false + ), + ])) + .unwrap(); + assert_eq!( + select_hls_format(&formats, Some(720)).expect("hls").url, + "https://cdn.example/h720" + ); + assert_eq!( + select_hls_format(&formats, Some(480)).expect("hls").url, + "https://cdn.example/h480" + ); + // Above every variant still takes the tallest. + assert_eq!( + select_hls_format(&formats, Some(2160)).expect("hls").url, + "https://cdn.example/h1080" + ); +}