mirror of
https://github.com/nimbold/Firelink.git
synced 2026-07-26 12:08:27 +00:00
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
This commit is contained in:
@@ -178,10 +178,13 @@ fn legacy_download_queue_paths(app_handle: &tauri::AppHandle) -> Result<Vec<Path
|
||||
.get(&category)
|
||||
.cloned()
|
||||
.unwrap_or_else(|| category.clone());
|
||||
std::path::PathBuf::from(&settings.base_download_folder)
|
||||
.join(subfolder)
|
||||
.to_string_lossy()
|
||||
.to_string()
|
||||
let base = std::path::PathBuf::from(&settings.base_download_folder);
|
||||
let destination = if subfolder.is_empty() {
|
||||
base
|
||||
} else {
|
||||
base.join(subfolder)
|
||||
};
|
||||
destination.to_string_lossy().to_string()
|
||||
})
|
||||
});
|
||||
let default_destination = settings
|
||||
|
||||
+65
-10
@@ -1007,6 +1007,14 @@ fn append_ytdlp_http_headers(
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn is_browser_cookie_extraction_error(message: &str) -> 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() {
|
||||
|
||||
@@ -150,6 +150,10 @@ fn default_category_subfolders() -> HashMap<String, String> {
|
||||
}
|
||||
|
||||
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});
|
||||
|
||||
@@ -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]
|
||||
)
|
||||
])
|
||||
|
||||
@@ -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');
|
||||
|
||||
@@ -55,11 +55,16 @@ const stringRecord = (value: unknown): Record<string, string> => {
|
||||
);
|
||||
};
|
||||
|
||||
const hasOwn = (value: Record<string, string>, 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);
|
||||
};
|
||||
|
||||
|
||||
Reference in New Issue
Block a user