From 99f73eaae2b25c4e33f367c2e65293349b81967e Mon Sep 17 00:00:00 2001 From: NimBold Date: Mon, 6 Jul 2026 20:19:33 +0330 Subject: [PATCH] fix(downloads): handle root category folders and cookie fallback Allow explicit empty category subfolders to resolve to the base download folder across settings, UI, and backend ownership paths. Retry yt-dlp metadata and media downloads without browser cookies when the selected browser cookie database cannot be copied, while preserving real auth failures. Fixes #6 Fixes #7 --- src-tauri/src/download_ownership.rs | 11 +++-- src-tauri/src/lib.rs | 75 +++++++++++++++++++++++++---- src-tauri/src/settings.rs | 36 +++++++++++--- src/components/SettingsView.tsx | 8 ++- src/utils/downloadLocations.test.ts | 12 +++++ src/utils/downloadLocations.ts | 40 ++++++++++----- 6 files changed, 147 insertions(+), 35 deletions(-) diff --git a/src-tauri/src/download_ownership.rs b/src-tauri/src/download_ownership.rs index d4b40d4..cac598a 100644 --- a/src-tauri/src/download_ownership.rs +++ b/src-tauri/src/download_ownership.rs @@ -178,10 +178,13 @@ fn legacy_download_queue_paths(app_handle: &tauri::AppHandle) -> Result bool { + let lower = message.to_ascii_lowercase(); + lower.contains("could not copy") && lower.contains("cookie database") + || lower.contains("could not access browser cookie database") + || lower.contains("failed to read browser cookie") + || lower.contains("failed to decrypt with dpapi") +} + fn should_cleanup_media_artifacts_after_failure( failure_reason: &str, strike: usize, @@ -1332,15 +1340,28 @@ async fn fetch_media_metadata( drop(cache_guard); let result = fetch_media_metadata_uncached( - app_handle, - url, - cookie_browser, - username, - password, - proxy, + app_handle.clone(), + url.clone(), + cookie_browser.clone(), + username.clone(), + password.clone(), + proxy.clone(), ) .await; + let result = match (result, cookie_browser.as_deref()) { + (Err(error), Some(browser)) + if !browser.trim().is_empty() && is_browser_cookie_extraction_error(&error) => + { + log::warn!( + "yt-dlp could not read browser cookies from {}; retrying media metadata without browser cookies", + browser + ); + fetch_media_metadata_uncached(app_handle, url, None, username, password, proxy).await + } + (result, _) => result, + }; + let result = match result { Ok(metadata) if metadata.formats.is_empty() => { Err("yt-dlp returned no usable media formats for this URL".to_string()) @@ -2667,6 +2688,8 @@ pub(crate) async fn start_media_download_internal( let max_retries = max_tries.unwrap_or(0).max(0) as usize; let mut strike = 0_usize; let mut processing_started = false; + let mut effective_cookie_source = cookie_source.clone(); + let mut browser_cookie_fallback_used = false; while strike <= max_retries { let ytdlp_path = resolve_bundled_binary_path(&app_handle, "yt-dlp")?; @@ -2714,7 +2737,7 @@ pub(crate) async fn start_media_download_internal( } } - if let Some(cs) = cookie_source.as_ref() { + if let Some(cs) = effective_cookie_source.as_ref() { let mut cs = cs.clone(); if !cs.is_empty() && cs != "none" { if cs == "safari" { @@ -2754,7 +2777,7 @@ pub(crate) async fn start_media_download_internal( let mut temp_info_path = None; if let Ok(mut cache) = get_metadata_cache().lock() { let info_cache_key = - metadata_info_cache_key(&url, cookie_source.as_deref(), proxy.as_deref()); + metadata_info_cache_key(&url, effective_cookie_source.as_deref(), proxy.as_deref()); if let Some(json_str) = cache.remove(&info_cache_key) { let temp_dir = std::env::temp_dir(); let path = temp_dir.join(format!("firelink_ytdlp_{}.info.json", id)); @@ -2941,6 +2964,21 @@ pub(crate) async fn start_media_download_internal( if should_cleanup_media_artifacts_after_failure(&failure_reason, strike, max_retries) { cleanup_media_artifacts(&out_path, false).await; } + if !browser_cookie_fallback_used + && effective_cookie_source + .as_deref() + .is_some_and(|source| !source.trim().is_empty() && source != "none") + && is_browser_cookie_extraction_error(&failure_reason) + { + let source = effective_cookie_source.clone().unwrap_or_default(); + log::warn!( + "yt-dlp could not read browser cookies from {}; retrying media download without browser cookies", + source + ); + effective_cookie_source = None; + browser_cookie_fallback_used = true; + continue; + } if !(transient && strikes_left) { return Err(failure_reason); } @@ -4423,8 +4461,9 @@ mod tests { aggregate_media_fraction, append_ytdlp_http_headers, build_media_format_options, collect_download_uris, filename_from_content_disposition, filename_from_url_disposition_query, filename_from_url_path, is_excluded_yt_dlp_format, - json_lower, media_output_template, media_progress_speed, normalize_speed_limit_for_aria2, - parse_firelink_deep_link, parse_ffmpeg_version, parse_media_progress_line, redact_log_line, + is_browser_cookie_extraction_error, json_lower, media_output_template, + media_progress_speed, normalize_speed_limit_for_aria2, parse_firelink_deep_link, + parse_ffmpeg_version, parse_media_progress_line, redact_log_line, sanitize_ytdlp_config_value, should_cleanup_media_artifacts_after_failure, FirelinkDeepLink, MediaProgress, MEDIA_PROGRESS_PREFIX, }; @@ -4714,6 +4753,22 @@ mod tests { assert_eq!(option.filesize_approx, Some(1_468_000_000)); } + #[test] + fn classifies_browser_cookie_database_errors_for_fallback() { + assert!(is_browser_cookie_extraction_error( + "ERROR: Could not copy Chrome cookie database. See https://github.com/yt-dlp/yt-dlp/issues/7271" + )); + assert!(is_browser_cookie_extraction_error( + "failed to read browser cookie data" + )); + assert!(!is_browser_cookie_extraction_error( + "ERROR: Sign in to confirm you are not a bot" + )); + assert!(!is_browser_cookie_extraction_error( + "ERROR: requested format is not available" + )); + } + #[test] #[ignore = "requires network and a local yt-dlp executable"] fn filters_live_youtube_metadata_from_env() { diff --git a/src-tauri/src/settings.rs b/src-tauri/src/settings.rs index 6c167e4..b296007 100644 --- a/src-tauri/src/settings.rs +++ b/src-tauri/src/settings.rs @@ -150,6 +150,10 @@ fn default_category_subfolders() -> HashMap { } fn normalize_category_subfolder(value: &str, fallback: &str) -> String { + if value.trim().is_empty() { + return String::new(); + } + let parts = value .split(['/', '\\']) .filter(|part| !part.is_empty() && *part != "." && *part != ".." && !part.ends_with(':')) @@ -181,7 +185,7 @@ fn migrate_location_settings(state: &mut Value) -> Result<(), String> { let mut subfolders = default_category_subfolders(); if let Some(persisted) = state.get("categorySubfolders").and_then(Value::as_object) { for (category, value) in persisted { - if let Some(folder) = value.as_str().filter(|folder| !folder.trim().is_empty()) { + if let Some(folder) = value.as_str() { let fallback = subfolders .get(category) .cloned() @@ -251,11 +255,13 @@ fn normalize_location_path(path: &str) -> String { } fn derived_location_path(base: &str, subfolder: &str) -> String { - format!( - "{}/{}", - normalize_location_path(base), - subfolder.trim_matches(|character| character == '/' || character == '\\') - ) + let base = normalize_location_path(base); + let subfolder = subfolder.trim_matches(|character| character == '/' || character == '\\'); + if subfolder.is_empty() { + base + } else { + format!("{base}/{subfolder}") + } } fn default_settings() -> PersistedSettings { @@ -451,6 +457,24 @@ mod tests { assert_eq!(settings.category_subfolders["Documents"], "Documents"); } + #[test] + fn preserves_empty_category_subfolder_as_base_folder() { + let stored = json!({ + "state": { + "baseDownloadFolder": "/Users/test/Downloads", + "categorySubfolders": { + "Movies": "" + } + }, + "version": 3 + }); + + let settings = decode_stored_settings(&Value::String(stored.to_string())).unwrap(); + + assert_eq!(settings.category_subfolders["Movies"], ""); + assert_eq!(settings.category_subfolders["Documents"], "Documents"); + } + #[test] fn replaces_zero_concurrency_with_the_safe_default() { let stored = json!({"state": {"maxConcurrentDownloads": 0}, "version": 0}); diff --git a/src/components/SettingsView.tsx b/src/components/SettingsView.tsx index 588d0bb..b919711 100644 --- a/src/components/SettingsView.tsx +++ b/src/components/SettingsView.tsx @@ -175,7 +175,9 @@ const CategoryFolderInput = ({ onBrowse: () => void; }) => { const base = settings.baseDownloadFolder || '~/Downloads'; - const sub = settings.categorySubfolders[category] || DEFAULT_CATEGORY_SUBFOLDERS[category as keyof typeof DEFAULT_CATEGORY_SUBFOLDERS]; + const sub = Object.prototype.hasOwnProperty.call(settings.categorySubfolders, category) + ? settings.categorySubfolders[category] + : DEFAULT_CATEGORY_SUBFOLDERS[category as keyof typeof DEFAULT_CATEGORY_SUBFOLDERS]; const override = settings.categoryDirectoryOverrides[category]; const displayPath = override ?? formatDerivedCategoryPath(base, sub); @@ -480,7 +482,9 @@ runEngineChecks(false); DOWNLOAD_CATEGORIES.map(category => [ category, normalizeCategorySubfolder( - settings.categorySubfolders[category] || '', + Object.prototype.hasOwnProperty.call(settings.categorySubfolders, category) + ? settings.categorySubfolders[category] + : DEFAULT_CATEGORY_SUBFOLDERS[category], DEFAULT_CATEGORY_SUBFOLDERS[category] ) ]) diff --git a/src/utils/downloadLocations.test.ts b/src/utils/downloadLocations.test.ts index 9e96917..43ca640 100644 --- a/src/utils/downloadLocations.test.ts +++ b/src/utils/downloadLocations.test.ts @@ -65,6 +65,18 @@ describe('download locations', () => { expect(await resolveCategoryDestination(automatic, 'Movies')).toBe('/Volumes/Media'); }); + it('keeps an explicit empty category subfolder as the base folder', async () => { + const settings = normalizeDownloadLocationSettings({ + baseDownloadFolder: '/Users/test/Downloads', + categorySubfolders: { Movies: '' } + }); + + expect(settings.categorySubfolders.Movies).toBe(''); + expect(settings.categorySubfolders.Documents).toBe('Documents'); + expect(formatDerivedCategoryPath('/Users/test/Downloads', '')).toBe('/Users/test/Downloads'); + expect(await resolveCategoryDestination(settings, 'Movies')).toBe('/Users/test/Downloads'); + }); + it('keeps category subfolders relative and permits nested folders', () => { expect(normalizeCategorySubfolder('../Media/./Movies', 'Movies')).toBe('Media/Movies'); expect(normalizeCategorySubfolder('C:\\Media\\Movies', 'Movies')).toBe('Media/Movies'); diff --git a/src/utils/downloadLocations.ts b/src/utils/downloadLocations.ts index 40b2655..c6cc5e5 100644 --- a/src/utils/downloadLocations.ts +++ b/src/utils/downloadLocations.ts @@ -55,11 +55,16 @@ const stringRecord = (value: unknown): Record => { ); }; +const hasOwn = (value: Record, key: string): boolean => + Object.prototype.hasOwnProperty.call(value, key); + const normalizedForComparison = (value: string): string => value.replace(/\\/g, '/').replace(/\/+$/, ''); const legacyDerivedPath = (base: string, subfolder: string): string => - `${normalizedForComparison(base)}/${subfolder.replace(/^[\\/]+|[\\/]+$/g, '')}`; + [normalizedForComparison(base), subfolder.replace(/^[\\/]+|[\\/]+$/g, '')] + .filter(Boolean) + .join('/'); const isWindowsLikePath = (value: string): boolean => /^[a-z]:[\\/]/i.test(value) || value.startsWith('\\\\') || value.includes('\\'); @@ -67,6 +72,7 @@ const isWindowsLikePath = (value: string): boolean => export const formatDerivedCategoryPath = (base: string, subfolder: string): string => { const trimmedBase = base.trim() || '~/Downloads'; const relative = subfolder.replace(/^[\\/]+|[\\/]+$/g, ''); + if (!relative) return trimmedBase.replace(/[\\/]+$/, ''); const separator = isWindowsLikePath(trimmedBase) ? '\\' : '/'; return `${trimmedBase.replace(/[\\/]+$/, '')}${separator}${relative}`; }; @@ -90,6 +96,9 @@ export const normalizeCategorySubfolder = ( value: string, fallback: string ): string => { + const trimmed = value.trim(); + if (!trimmed) return ''; + const parts = value .trim() .replace(/\\/g, '/') @@ -107,13 +116,15 @@ export const normalizeDownloadLocationSettings = ( '~/Downloads'; const persistedSubfolders = stringRecord(value.categorySubfolders); const categorySubfolders = Object.fromEntries( - DOWNLOAD_CATEGORIES.map(category => [ - category, - normalizeCategorySubfolder( - persistedSubfolders[category] || '', - DEFAULT_CATEGORY_SUBFOLDERS[category] - ) - ]) + DOWNLOAD_CATEGORIES.map(category => { + const persistedValue = hasOwn(persistedSubfolders, category) + ? persistedSubfolders[category] + : DEFAULT_CATEGORY_SUBFOLDERS[category]; + return [ + category, + normalizeCategorySubfolder(persistedValue, DEFAULT_CATEGORY_SUBFOLDERS[category]) + ]; + }) ); const categoryDirectoryOverrides = stringRecord(value.categoryDirectoryOverrides); const legacyDirectories = stringRecord(value.downloadDirectories); @@ -153,11 +164,14 @@ export const resolveCategoryDestination = async ( const base = settings.baseDownloadFolder.trim() || '~/Downloads'; const expandedBase = await expandTilde(base); - const subfolder = - normalizeCategorySubfolder( - settings.categorySubfolders[category] || '', - DEFAULT_CATEGORY_SUBFOLDERS[category] - ); + const persistedValue = hasOwn(settings.categorySubfolders, category) + ? settings.categorySubfolders[category] + : DEFAULT_CATEGORY_SUBFOLDERS[category]; + const subfolder = normalizeCategorySubfolder( + persistedValue, + DEFAULT_CATEGORY_SUBFOLDERS[category] + ); + if (!subfolder) return expandedBase; return join(expandedBase, subfolder); };