mirror of
https://github.com/nimbold/Firelink.git
synced 2026-09-09 17:25:42 +00:00
fix(app): enforce media and scheduler lifecycle boundaries
- keep magnet probe cleanup and resolver fallback inside absolute deadlines - route provider media and playlists only through HTTP(S) - enforce scheduler ID, time-format, and running-state invariants across persistence - isolate Companion release tag checks from ambient Git configuration
This commit is contained in:
@@ -22,7 +22,7 @@ export function exactVersionTag(extensionRoot, expectedTag) {
|
||||
stdio: ['ignore', 'pipe', 'ignore'],
|
||||
env: {
|
||||
...process.env,
|
||||
GIT_CONFIG_GLOBAL: process.env.GIT_CONFIG_GLOBAL || (process.platform === 'win32' ? 'NUL' : '/dev/null'),
|
||||
GIT_CONFIG_GLOBAL: process.platform === 'win32' ? 'NUL' : '/dev/null',
|
||||
GIT_CONFIG_NOSYSTEM: '1',
|
||||
},
|
||||
}
|
||||
|
||||
@@ -120,6 +120,7 @@ test('rejects a Companion tag for another version', () => {
|
||||
|
||||
test('exactVersionTag resolves tag on HEAD with isolated git environment', () => {
|
||||
const root = fs.mkdtempSync(path.join(os.tmpdir(), 'firelink-git-test-'));
|
||||
const previousGlobalConfig = process.env.GIT_CONFIG_GLOBAL;
|
||||
try {
|
||||
const gitEnv = {
|
||||
...process.env,
|
||||
@@ -134,9 +135,17 @@ test('exactVersionTag resolves tag on HEAD with isolated git environment', () =>
|
||||
execFileSync('git', ['-C', root, 'commit', '--allow-empty', '-m', 'test'], { env: gitEnv, stdio: 'ignore' });
|
||||
execFileSync('git', ['-C', root, 'tag', 'v2.0.7'], { env: gitEnv, stdio: 'ignore' });
|
||||
|
||||
const globalConfig = path.join(root, 'global.gitconfig');
|
||||
fs.writeFileSync(globalConfig, '[alias]\n\ttag = !printf "v2.0.8\\n"\n');
|
||||
process.env.GIT_CONFIG_GLOBAL = globalConfig;
|
||||
assert.equal(exactVersionTag(root, 'v2.0.7'), 'v2.0.7');
|
||||
assert.equal(exactVersionTag(root, 'v2.0.8'), null);
|
||||
} finally {
|
||||
if (previousGlobalConfig === undefined) {
|
||||
delete process.env.GIT_CONFIG_GLOBAL;
|
||||
} else {
|
||||
process.env.GIT_CONFIG_GLOBAL = previousGlobalConfig;
|
||||
}
|
||||
fs.rmSync(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
@@ -9368,9 +9368,7 @@ async fn resolve_magnet_metadata(
|
||||
.err()
|
||||
.is_some_and(crate::torrent_probe::allows_resolver_fallback) =>
|
||||
{
|
||||
let cleanup_budget = operation_deadline
|
||||
.saturating_duration_since(Instant::now())
|
||||
.max(MAGNET_PROBE_CLEANUP_RESERVE);
|
||||
let cleanup_budget = operation_deadline.saturating_duration_since(Instant::now());
|
||||
match tokio::time::timeout(cleanup_budget, remove_magnet_metadata_probe_dir(&probe_dir))
|
||||
.await
|
||||
{
|
||||
@@ -9387,9 +9385,7 @@ async fn resolve_magnet_metadata(
|
||||
);
|
||||
}
|
||||
}
|
||||
let create_budget = operation_deadline
|
||||
.saturating_duration_since(Instant::now())
|
||||
.max(MAGNET_PROBE_CLEANUP_RESERVE);
|
||||
let create_budget = operation_deadline.saturating_duration_since(Instant::now());
|
||||
match tokio::time::timeout(create_budget, tokio::fs::create_dir_all(&probe_dir)).await {
|
||||
Ok(Ok(())) => {}
|
||||
Ok(Err(error)) => {
|
||||
|
||||
+13
-1
@@ -680,6 +680,9 @@ pub fn get_supported_media_domains() -> Vec<String> {
|
||||
#[tauri::command]
|
||||
pub fn is_supported_media(url: String) -> bool {
|
||||
if let Ok(parsed_url) = reqwest::Url::parse(&url) {
|
||||
if !matches!(parsed_url.scheme(), "http" | "https") {
|
||||
return false;
|
||||
}
|
||||
if let Some(host) = parsed_url.host_str() {
|
||||
let host_lower = host.to_lowercase();
|
||||
for domain in SUPPORTED_DOMAINS.iter() {
|
||||
@@ -694,7 +697,7 @@ pub fn is_supported_media(url: String) -> bool {
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::get_file_category;
|
||||
use super::{get_file_category, is_supported_media};
|
||||
use crate::ipc::DownloadCategory;
|
||||
|
||||
#[test]
|
||||
@@ -708,4 +711,13 @@ mod tests {
|
||||
DownloadCategory::Movies
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn only_http_urls_are_supported_media_routes() {
|
||||
assert!(is_supported_media(
|
||||
"https://youtube.com/watch?v=video".to_string()
|
||||
));
|
||||
assert!(!is_supported_media("ftp://youtube.com/video".to_string()));
|
||||
assert!(!is_supported_media("sftp://youtube.com/video".to_string()));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -5,9 +5,18 @@ use std::time::Duration;
|
||||
use tauri::Emitter;
|
||||
|
||||
fn minute_of_day(value: &str) -> Option<u32> {
|
||||
let (hour, minute) = value.split_once(':')?;
|
||||
let hour = hour.parse::<u32>().ok()?;
|
||||
let minute = minute.parse::<u32>().ok()?;
|
||||
let bytes = value.as_bytes();
|
||||
if bytes.len() != 5
|
||||
|| bytes[2] != b':'
|
||||
|| !bytes[0].is_ascii_digit()
|
||||
|| !bytes[1].is_ascii_digit()
|
||||
|| !bytes[3].is_ascii_digit()
|
||||
|| !bytes[4].is_ascii_digit()
|
||||
{
|
||||
return None;
|
||||
}
|
||||
let hour = u32::from(bytes[0] - b'0') * 10 + u32::from(bytes[1] - b'0');
|
||||
let minute = u32::from(bytes[3] - b'0') * 10 + u32::from(bytes[4] - b'0');
|
||||
(hour < 24 && minute < 60).then_some(hour * 60 + minute)
|
||||
}
|
||||
|
||||
@@ -254,6 +263,10 @@ mod tests {
|
||||
fn rejects_invalid_scheduler_times() {
|
||||
assert_eq!(minute_of_day("24:00"), None);
|
||||
assert_eq!(minute_of_day("12:60"), None);
|
||||
assert_eq!(minute_of_day("1:02"), None);
|
||||
assert_eq!(minute_of_day("01:2"), None);
|
||||
assert_eq!(minute_of_day(" 01:02"), None);
|
||||
assert_eq!(minute_of_day("01:02 "), None);
|
||||
assert_eq!(minute_of_day("bad"), None);
|
||||
}
|
||||
|
||||
|
||||
@@ -545,9 +545,16 @@ fn sanitize_persisted_setting_values(state: &mut Value) {
|
||||
if !active_ids.is_array() {
|
||||
state.remove("schedulerActiveDownloadIds");
|
||||
} else if let Some(ids_arr) = state.get_mut("schedulerActiveDownloadIds").and_then(Value::as_array_mut) {
|
||||
ids_arr.retain(|v| v.as_str().is_some());
|
||||
ids_arr.retain(|v| v.as_str().is_some_and(|id| !id.trim().is_empty()));
|
||||
}
|
||||
}
|
||||
if !state
|
||||
.get("schedulerActiveDownloadIds")
|
||||
.and_then(Value::as_array)
|
||||
.is_some_and(|ids| !ids.is_empty())
|
||||
{
|
||||
state.insert("schedulerRunning".to_string(), Value::Bool(false));
|
||||
}
|
||||
if let Some(overrides) = state.get("categoryDirectoryOverrides") {
|
||||
if !overrides.is_object() {
|
||||
state.remove("categoryDirectoryOverrides");
|
||||
@@ -1533,6 +1540,39 @@ mod tests {
|
||||
assert!(settings.scheduler_active_download_ids.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn filters_empty_scheduler_active_download_ids() {
|
||||
let stored = json!({
|
||||
"state": {
|
||||
"schedulerRunning": true,
|
||||
"schedulerActiveDownloadIds": ["", " ", "download-1", 42]
|
||||
}
|
||||
});
|
||||
|
||||
let settings = decode_stored_settings(&Value::String(stored.to_string())).unwrap();
|
||||
|
||||
assert_eq!(
|
||||
settings.scheduler_active_download_ids,
|
||||
vec!["download-1".to_string()]
|
||||
);
|
||||
assert!(settings.scheduler_running);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn does_not_restore_a_running_scheduler_without_active_download_ids() {
|
||||
let stored = json!({
|
||||
"state": {
|
||||
"schedulerRunning": true,
|
||||
"schedulerActiveDownloadIds": ["", " ", 42]
|
||||
}
|
||||
});
|
||||
|
||||
let settings = decode_stored_settings(&Value::String(stored.to_string())).unwrap();
|
||||
|
||||
assert!(!settings.scheduler_running);
|
||||
assert!(settings.scheduler_active_download_ids.is_empty());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn preserves_valid_torrent_network_settings() {
|
||||
let stored = json!({
|
||||
|
||||
@@ -250,9 +250,7 @@ pub(crate) async fn run_metadata_probe_with_deadlines<C: RpcClient + 'static>(
|
||||
}
|
||||
.await;
|
||||
|
||||
let cleanup_budget = cleanup_deadline
|
||||
.saturating_duration_since(Instant::now())
|
||||
.max(Duration::from_secs(5));
|
||||
let cleanup_budget = cleanup_deadline.saturating_duration_since(Instant::now());
|
||||
let cleanup_result = match tokio::time::timeout(
|
||||
cleanup_budget,
|
||||
cleanup_metadata_probe(client.as_ref(), &gid),
|
||||
@@ -500,10 +498,8 @@ impl<C: RpcClient + 'static> ProbeCleanupGuard<C> {
|
||||
}
|
||||
|
||||
let mut first_error = None;
|
||||
let cleanup_budget = deadline
|
||||
.saturating_duration_since(Instant::now())
|
||||
.max(Duration::from_secs(5));
|
||||
for gid in self.gids.clone() {
|
||||
let cleanup_budget = deadline.saturating_duration_since(Instant::now());
|
||||
match tokio::time::timeout(
|
||||
cleanup_budget,
|
||||
cleanup_metadata_probe(self.client.as_ref(), &gid),
|
||||
@@ -1601,6 +1597,44 @@ mod tests {
|
||||
server.shutdown().await;
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
async fn cleanup_never_extends_absolute_probe_deadline() {
|
||||
let server = ScriptedRpcServer::start(scripts([
|
||||
("aria2.addUri", vec![ScriptedReply::Result(json!("gid-1"))]),
|
||||
("aria2.tellStatus", vec![ScriptedReply::Hang]),
|
||||
("aria2.forceRemove", vec![ScriptedReply::Hang]),
|
||||
]))
|
||||
.await;
|
||||
let (_temporary, probe_dir, metadata_path) = probe_fixture().await;
|
||||
let error = tokio::time::timeout(
|
||||
Duration::from_millis(500),
|
||||
run_bounded_metadata_probe(
|
||||
server.client(),
|
||||
"magnet:?xt=urn:btih:0123456789abcdef0123456789abcdef01234567",
|
||||
Map::new(),
|
||||
&metadata_path,
|
||||
"automatic",
|
||||
MetadataProbeSchedule {
|
||||
total_timeout: Duration::from_millis(100),
|
||||
metadata_timeout: Duration::from_millis(20),
|
||||
cleanup_reserve: Duration::from_millis(80),
|
||||
poll_interval: Duration::ZERO,
|
||||
},
|
||||
),
|
||||
)
|
||||
.await
|
||||
.expect("cleanup must not extend the absolute probe deadline")
|
||||
.expect_err("a cleanup timeout should be reported");
|
||||
assert!(matches!(
|
||||
error,
|
||||
ProbeFailure::Cleanup(message) if message.contains("failed to remove aria2 gid")
|
||||
));
|
||||
server.terminate().await;
|
||||
tokio::fs::remove_dir_all(&probe_dir)
|
||||
.await
|
||||
.expect("the probe fixture should be removable after the server stops");
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "current_thread")]
|
||||
async fn generic_metadata_timeout_does_not_enter_blocking_system_resolver() {
|
||||
// Use a deterministic pending RPC rather than a loopback HTTP server.
|
||||
|
||||
@@ -132,6 +132,49 @@ describe('durable main-window and sidebar preferences', () => {
|
||||
expect(fallbackResult?.scheduler.selectedQueueIds).toEqual(current.scheduler.selectedQueueIds);
|
||||
});
|
||||
|
||||
it('filters empty scheduler active download IDs during hydration', () => {
|
||||
const merge = useSettingsStore.persist.getOptions().merge;
|
||||
expect(merge).toBeTypeOf('function');
|
||||
const current = useSettingsStore.getState();
|
||||
|
||||
const result = merge?.({
|
||||
schedulerActiveDownloadIds: ['', ' ', 'download-1', 42] as any
|
||||
}, current);
|
||||
|
||||
expect(result?.schedulerActiveDownloadIds).toEqual(['download-1']);
|
||||
});
|
||||
|
||||
it('does not restore a running scheduler without active download IDs', () => {
|
||||
const merge = useSettingsStore.persist.getOptions().merge;
|
||||
expect(merge).toBeTypeOf('function');
|
||||
const current = useSettingsStore.getState();
|
||||
|
||||
const result = merge?.({
|
||||
schedulerRunning: true,
|
||||
schedulerActiveDownloadIds: ['', ' ', 42] as any
|
||||
}, current);
|
||||
|
||||
expect(result?.schedulerRunning).toBe(false);
|
||||
expect(result?.schedulerActiveDownloadIds).toEqual([]);
|
||||
});
|
||||
|
||||
it('does not persist a running scheduler without active download IDs', () => {
|
||||
const partialize = useSettingsStore.persist.getOptions().partialize;
|
||||
expect(partialize).toBeTypeOf('function');
|
||||
const current = useSettingsStore.getState();
|
||||
|
||||
const snapshot = partialize?.({
|
||||
...current,
|
||||
schedulerRunning: true,
|
||||
schedulerActiveDownloadIds: []
|
||||
});
|
||||
|
||||
expect(snapshot).toMatchObject({
|
||||
schedulerRunning: false,
|
||||
schedulerActiveDownloadIds: []
|
||||
});
|
||||
});
|
||||
|
||||
it('sanitizes setter calls for enum and numeric settings', () => {
|
||||
useSettingsStore.getState().setMediaCookieSource('invalid-browser' as any);
|
||||
expect(useSettingsStore.getState().mediaCookieSource).toBe('none');
|
||||
|
||||
@@ -166,6 +166,13 @@ const persistedBoolean = (value: unknown, fallback: boolean) =>
|
||||
const persistedString = (value: unknown, fallback: string): string =>
|
||||
typeof value === 'string' ? value : fallback;
|
||||
|
||||
const sanitizeSchedulerActiveDownloadIds = (
|
||||
value: unknown,
|
||||
fallback: string[]
|
||||
): string[] => Array.isArray(value)
|
||||
? value.filter((id): id is string => typeof id === 'string' && id.trim().length > 0)
|
||||
: fallback;
|
||||
|
||||
const persistedFiniteInteger = (
|
||||
value: unknown,
|
||||
minimum: number,
|
||||
@@ -865,7 +872,12 @@ export const useSettingsStore = create<SettingsState>()(
|
||||
shouldPersistLegacyFoldersFallback = false;
|
||||
state.setFoldersCollapsed(state.isFoldersCollapsed);
|
||||
},
|
||||
partialize: (state): PersistedSettingsSnapshot => ({
|
||||
partialize: (state): PersistedSettingsSnapshot => {
|
||||
const schedulerActiveDownloadIds = sanitizeSchedulerActiveDownloadIds(
|
||||
state.schedulerActiveDownloadIds,
|
||||
[]
|
||||
);
|
||||
return ({
|
||||
theme: state.theme,
|
||||
fontFamily: state.fontFamily,
|
||||
windowControlStyle: state.windowControlStyle,
|
||||
@@ -888,8 +900,8 @@ export const useSettingsStore = create<SettingsState>()(
|
||||
sidebarPosition: state.sidebarPosition,
|
||||
activeSettingsTab: state.activeSettingsTab,
|
||||
scheduler: state.scheduler,
|
||||
schedulerRunning: state.schedulerRunning,
|
||||
schedulerActiveDownloadIds: state.schedulerActiveDownloadIds,
|
||||
schedulerRunning: state.schedulerRunning && schedulerActiveDownloadIds.length > 0,
|
||||
schedulerActiveDownloadIds,
|
||||
schedulerLastStartKey: state.schedulerLastStartKey,
|
||||
schedulerLastStopKey: state.schedulerLastStopKey,
|
||||
lastCustomSpeedLimitKiB: state.lastCustomSpeedLimitKiB,
|
||||
@@ -940,7 +952,8 @@ export const useSettingsStore = create<SettingsState>()(
|
||||
keychainAccessVersion: state.keychainAccessVersion,
|
||||
keychainPromptDismissed: state.keychainPromptDismissed,
|
||||
autoCheckUpdates: state.autoCheckUpdates
|
||||
}),
|
||||
});
|
||||
},
|
||||
merge: (persistedState: unknown, currentState) => {
|
||||
const persisted = persistedState && typeof persistedState === 'object'
|
||||
? persistedState as Partial<SettingsState>
|
||||
@@ -953,6 +966,10 @@ export const useSettingsStore = create<SettingsState>()(
|
||||
const foldersCollapsedFallback = legacyFoldersCollapsed
|
||||
?? currentState.isFoldersCollapsed;
|
||||
const locations = normalizeDownloadLocationSettings(persisted);
|
||||
const schedulerActiveDownloadIds = sanitizeSchedulerActiveDownloadIds(
|
||||
persisted.schedulerActiveDownloadIds,
|
||||
currentState.schedulerActiveDownloadIds
|
||||
);
|
||||
return ({
|
||||
...currentState,
|
||||
...persisted,
|
||||
@@ -1180,10 +1197,9 @@ export const useSettingsStore = create<SettingsState>()(
|
||||
? persisted.scheduler.postQueueAction
|
||||
: currentState.scheduler.postQueueAction
|
||||
},
|
||||
schedulerRunning: persistedBoolean(persisted.schedulerRunning, currentState.schedulerRunning),
|
||||
schedulerActiveDownloadIds: Array.isArray(persisted.schedulerActiveDownloadIds)
|
||||
? persisted.schedulerActiveDownloadIds.filter((id): id is string => typeof id === 'string')
|
||||
: currentState.schedulerActiveDownloadIds,
|
||||
schedulerRunning: persistedBoolean(persisted.schedulerRunning, currentState.schedulerRunning)
|
||||
&& schedulerActiveDownloadIds.length > 0,
|
||||
schedulerActiveDownloadIds,
|
||||
schedulerLastStartKey: persistedString(persisted.schedulerLastStartKey, currentState.schedulerLastStartKey),
|
||||
schedulerLastStopKey: persistedString(persisted.schedulerLastStopKey, currentState.schedulerLastStopKey),
|
||||
siteLogins: Array.isArray(persisted.siteLogins)
|
||||
|
||||
@@ -87,6 +87,8 @@ describe('add download metadata workflow', () => {
|
||||
expect(isYouTubePlaylistUrl('https://music.youtube.com/playlist?list=PL123')).toBe(true);
|
||||
expect(isYouTubePlaylistUrl('https://www.youtube.com/watch?v=video&list=PL123')).toBe(false);
|
||||
expect(isYouTubePlaylistUrl('https://example.com/playlist?list=PL123')).toBe(false);
|
||||
expect(isYouTubePlaylistUrl('ftp://youtube.com/playlist?list=PL123')).toBe(false);
|
||||
expect(isYouTubePlaylistUrl('sftp://youtube.com/playlist?list=PL123')).toBe(false);
|
||||
});
|
||||
|
||||
it('admits magnets and local torrent files through the Add window metadata path', () => {
|
||||
|
||||
@@ -153,6 +153,7 @@ type ParsedInput = {
|
||||
export const isYouTubePlaylistUrl = (rawUrl: string): boolean => {
|
||||
try {
|
||||
const url = new URL(rawUrl);
|
||||
if (url.protocol !== 'http:' && url.protocol !== 'https:') return false;
|
||||
const hostname = url.hostname.toLowerCase();
|
||||
const isYouTube = hostname === 'youtube.com' || hostname.endsWith('.youtube.com');
|
||||
const pathname = url.pathname.replace(/\/+$/, '') || '/';
|
||||
|
||||
@@ -9,6 +9,7 @@ import {
|
||||
canonicalizeDownloadFileName,
|
||||
categoryForDownload,
|
||||
categoryForFileName,
|
||||
isMediaUrl,
|
||||
isAllocationPhaseVisible,
|
||||
isAllocationPhaseEligible,
|
||||
isValidTorrentExcludeTrackerList,
|
||||
@@ -49,6 +50,14 @@ describe('download category detection', () => {
|
||||
expect(categoryForDownload('Renamed', true, 'Other')).toBe('Other');
|
||||
expect(categoryForDownload('Renamed', true, 'Torrents')).toBe('Torrents');
|
||||
});
|
||||
|
||||
it('only classifies HTTP(S) provider URLs as media', () => {
|
||||
expect(isMediaUrl('https://www.youtube.com/watch?v=video')).toBe(true);
|
||||
expect(isMediaUrl('http://youtu.be/video')).toBe(true);
|
||||
expect(isMediaUrl('ftp://youtube.com/video.mp4')).toBe(false);
|
||||
expect(isMediaUrl('sftp://youtube.com/video.mp4')).toBe(false);
|
||||
expect(isMediaUrl('magnet://youtube.com/video')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('download names from URLs', () => {
|
||||
|
||||
@@ -595,6 +595,7 @@ export const downloadFileNamesMatch = (left: string, right: string): boolean =>
|
||||
export const isMediaUrl = (rawUrl: string): boolean => {
|
||||
try {
|
||||
const url = new URL(rawUrl);
|
||||
if (url.protocol !== 'http:' && url.protocol !== 'https:') return false;
|
||||
return MEDIA_DOMAINS.some(domain =>
|
||||
url.hostname === domain || url.hostname.endsWith(`.${domain}`)
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user