mirror of
https://github.com/nimbold/Firelink.git
synced 2026-09-02 14:07:57 +00:00
fix(persistence): harden cross-layer ownership
- Validate and sanitize renderer event payloads before UI projection - Fence stale enqueue cleanup by native lifecycle generation - Canonicalize empty startup hydration and harden SQLite backup durability - Add real-postcondition IPC, restart, storage, and queue regressions
This commit is contained in:
+9
-1
@@ -70,7 +70,15 @@ type CommandMap = {
|
||||
open_downloaded_file: { args: { path: string }; result: void };
|
||||
pause_download: { args: { id: string }; result: void };
|
||||
resume_download: { args: { id: string; queueId: string }; result: boolean };
|
||||
remove_download: { args: { id: string; deleteAssets: boolean; preserveResumable?: boolean }; result: void };
|
||||
remove_download: {
|
||||
args: {
|
||||
id: string;
|
||||
deleteAssets: boolean;
|
||||
preserveResumable?: boolean;
|
||||
expectedLifecycleGeneration?: string;
|
||||
};
|
||||
result: void;
|
||||
};
|
||||
get_download_primary_path: { args: { id: string }; result: string | null };
|
||||
detach_download_for_reconfigure: { args: { id: string }; result: void };
|
||||
clear_torrent_removal_paths: { args: { id: string }; result: void };
|
||||
|
||||
@@ -92,6 +92,38 @@ describe('useDownloadProgressStore', () => {
|
||||
release();
|
||||
});
|
||||
|
||||
it('ignores malformed state events instead of projecting an unknown status', async () => {
|
||||
const handlers: Record<string, (event: any) => void> = {};
|
||||
vi.mocked(ipc.listenEvent).mockImplementation((event, handler) => {
|
||||
handlers[event] = handler as (event: any) => void;
|
||||
return Promise.resolve(vi.fn());
|
||||
});
|
||||
useDownloadStore.setState({
|
||||
downloads: [{
|
||||
id: 'malformed-state',
|
||||
url: 'https://example.com/file',
|
||||
fileName: 'file.bin',
|
||||
status: 'downloading',
|
||||
category: 'Other',
|
||||
dateAdded: ''
|
||||
}]
|
||||
});
|
||||
|
||||
const release = await initDownloadListener();
|
||||
handlers['download-state']({ payload: {
|
||||
id: 'malformed-state',
|
||||
status: 'not-a-download-status',
|
||||
error: { secret: 'must not enter the store' }
|
||||
} });
|
||||
|
||||
expect(useDownloadStore.getState().downloads[0]).toMatchObject({
|
||||
id: 'malformed-state',
|
||||
status: 'downloading'
|
||||
});
|
||||
expect(useDownloadStore.getState().downloads[0]).not.toHaveProperty('lastError');
|
||||
release();
|
||||
});
|
||||
|
||||
it('removes a row from backend pending order when its lifecycle becomes active or retrying', async () => {
|
||||
const handlers: Record<string, (event: any) => void> = {};
|
||||
vi.mocked(ipc.listenEvent).mockImplementation((event, handler) => {
|
||||
@@ -167,6 +199,37 @@ describe('useDownloadProgressStore', () => {
|
||||
release();
|
||||
});
|
||||
|
||||
it('accepts a progress frame that omits the optional size value', async () => {
|
||||
const handlers: Record<string, (event: any) => void> = {};
|
||||
vi.mocked(ipc.listenEvent).mockImplementation((event, handler) => {
|
||||
handlers[event] = handler as (event: any) => void;
|
||||
return Promise.resolve(vi.fn());
|
||||
});
|
||||
useDownloadStore.setState({
|
||||
downloads: [{
|
||||
id: 'omitted-size',
|
||||
url: 'https://example.com/file.bin',
|
||||
fileName: 'file.bin',
|
||||
status: 'downloading',
|
||||
category: 'Other',
|
||||
dateAdded: ''
|
||||
}]
|
||||
});
|
||||
|
||||
const release = await initDownloadListener();
|
||||
handlers['download-progress']({ payload: {
|
||||
id: 'omitted-size',
|
||||
fraction: 0.25,
|
||||
speed: '1 MB/s',
|
||||
eta: '1s',
|
||||
size_is_final: false
|
||||
} });
|
||||
|
||||
expect(useDownloadProgressStore.getState().progressMap['omitted-size'])
|
||||
.toMatchObject({ id: 'omitted-size', fraction: 0.25, size: null });
|
||||
release();
|
||||
});
|
||||
|
||||
it('keeps the last valid live frame when a malformed fraction arrives', async () => {
|
||||
const handlers: Record<string, (event: any) => void> = {};
|
||||
vi.mocked(ipc.listenEvent).mockImplementation((event, handler) => {
|
||||
@@ -231,6 +294,13 @@ describe('useDownloadProgressStore', () => {
|
||||
});
|
||||
|
||||
const release = await initDownloadListener();
|
||||
handlers['download-allocation']({ payload: {
|
||||
id: 'native-allocation',
|
||||
pending: true,
|
||||
lifecycleGeneration: 'not-a-generation'
|
||||
} });
|
||||
expect(useDownloadStore.getState().allocationPendingIds.has('native-allocation')).toBe(false);
|
||||
|
||||
handlers['download-allocation']({ payload: {
|
||||
id: 'native-allocation',
|
||||
pending: true,
|
||||
|
||||
@@ -1,9 +1,11 @@
|
||||
import type { UnlistenFn } from '@tauri-apps/api/event';
|
||||
import type { DownloadStatus } from '../bindings/DownloadStatus';
|
||||
import type { DownloadStateEvent } from '../bindings/DownloadStateEvent';
|
||||
import type { DownloadErrorKind } from '../bindings/DownloadErrorKind';
|
||||
import { listenEvent as listen } from '../ipc';
|
||||
import type { DownloadItem } from '../bindings/DownloadItem';
|
||||
import type { DownloadProgressEvent } from '../bindings/DownloadProgressEvent';
|
||||
import { categoryForDownload } from '../utils/downloads';
|
||||
import { categoryForDownload, isDownloadStatus } from '../utils/downloads';
|
||||
import { useDownloadProgressStore } from './downloadProgressStore';
|
||||
|
||||
import {
|
||||
@@ -35,9 +37,53 @@ type ProgressFields = {
|
||||
const finiteNonNegative = (value: unknown): value is number =>
|
||||
typeof value === 'number' && Number.isFinite(value) && value >= 0;
|
||||
|
||||
const isRecord = (value: unknown): value is Record<string, unknown> =>
|
||||
Boolean(value) && typeof value === 'object' && !Array.isArray(value);
|
||||
|
||||
const isDownloadErrorKind = (value: unknown): value is DownloadErrorKind =>
|
||||
value === 'nameResolution' || value === 'destinationAccess';
|
||||
|
||||
const isLifecycleGeneration = (value: unknown): value is string =>
|
||||
typeof value === 'string' && /^\d+$/.test(value);
|
||||
|
||||
type SanitizedDownloadStateEvent = Omit<DownloadStateEvent, 'status'> & {
|
||||
status: DownloadStatus;
|
||||
};
|
||||
|
||||
const sanitizeStatePayload = (value: unknown): SanitizedDownloadStateEvent | null => {
|
||||
if (!isRecord(value) || typeof value.id !== 'string' || !isDownloadStatus(value.status)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
return {
|
||||
id: value.id,
|
||||
status: value.status,
|
||||
error: typeof value.error === 'string' ? value.error : null,
|
||||
...(isDownloadErrorKind(value.errorKind) ? { errorKind: value.errorKind } : {}),
|
||||
...(typeof value.resolverFallback === 'boolean'
|
||||
? { resolverFallback: value.resolverFallback }
|
||||
: {}),
|
||||
...(typeof value.fileName === 'string' ? { fileName: value.fileName } : {}),
|
||||
...(typeof value.destination === 'string' ? { destination: value.destination } : {}),
|
||||
...(finiteNonNegative(value.torrentSeedRemaining)
|
||||
? { torrentSeedRemaining: value.torrentSeedRemaining }
|
||||
: {}),
|
||||
...(value.progress !== undefined ? { progress: value.progress as DownloadStateEvent['progress'] } : {})
|
||||
};
|
||||
};
|
||||
|
||||
const sanitizeProgressPayload = (
|
||||
payload: DownloadProgressEvent,
|
||||
value: unknown,
|
||||
): DownloadProgressEvent | null => {
|
||||
if (!isRecord(value)
|
||||
|| typeof value.id !== 'string'
|
||||
|| typeof value.speed !== 'string'
|
||||
|| typeof value.eta !== 'string'
|
||||
|| (value.size !== undefined && value.size !== null && typeof value.size !== 'string')
|
||||
|| typeof value.size_is_final !== 'boolean') {
|
||||
return null;
|
||||
}
|
||||
const payload = { ...value, size: value.size ?? null } as unknown as DownloadProgressEvent;
|
||||
if (typeof payload.fraction !== 'number'
|
||||
|| !Number.isFinite(payload.fraction)
|
||||
|| payload.fraction < 0
|
||||
@@ -65,6 +111,9 @@ const sanitizeProgressPayload = (
|
||||
&& typeof sanitized.total_is_estimate !== 'boolean') {
|
||||
delete sanitized.total_is_estimate;
|
||||
}
|
||||
if (sanitized.upload_speed !== undefined && typeof sanitized.upload_speed !== 'string') {
|
||||
delete sanitized.upload_speed;
|
||||
}
|
||||
return sanitized;
|
||||
};
|
||||
|
||||
@@ -151,7 +200,8 @@ const disposeDownloadListeners = () => {
|
||||
const startDownloadListeners = async () => {
|
||||
const registrations = await Promise.allSettled([
|
||||
listen('download-progress', (event) => {
|
||||
const payload = event.payload;
|
||||
const payload = sanitizeProgressPayload(event.payload);
|
||||
if (!payload) return;
|
||||
const mainStore = useDownloadStore.getState();
|
||||
const current = mainStore.downloads.find(d => d.id === payload.id);
|
||||
if (!current) {
|
||||
@@ -167,10 +217,7 @@ const startDownloadListeners = async () => {
|
||||
useDownloadProgressStore.getState().clearDownloadProgress(payload.id);
|
||||
return;
|
||||
}
|
||||
const sanitizedPayload = sanitizeProgressPayload(payload);
|
||||
if (!sanitizedPayload) {
|
||||
return;
|
||||
}
|
||||
const sanitizedPayload = payload;
|
||||
useDownloadProgressStore.getState().updateDownloadProgress(payload.id, sanitizedPayload);
|
||||
const shouldUpdateSize = Boolean(sanitizedPayload.size && (!current.isMedia || sanitizedPayload.size_is_final));
|
||||
const updates: Partial<DownloadItem> = {};
|
||||
@@ -232,6 +279,12 @@ const startDownloadListeners = async () => {
|
||||
}),
|
||||
listen('download-allocation', (event) => {
|
||||
const payload = event.payload;
|
||||
if (!isRecord(payload)
|
||||
|| typeof payload.id !== 'string'
|
||||
|| typeof payload.pending !== 'boolean'
|
||||
|| !isLifecycleGeneration(payload.lifecycleGeneration)) {
|
||||
return;
|
||||
}
|
||||
const mainStore = useDownloadStore.getState();
|
||||
const current = mainStore.downloads.find(download => download.id === payload.id);
|
||||
if (!current) {
|
||||
@@ -258,14 +311,15 @@ const startDownloadListeners = async () => {
|
||||
);
|
||||
}),
|
||||
listen('download-state', async (event) => {
|
||||
const payload = event.payload;
|
||||
const payload = sanitizeStatePayload(event.payload);
|
||||
if (!payload) return;
|
||||
const mainStore = useDownloadStore.getState();
|
||||
const current = mainStore.downloads.find(d => d.id === payload.id);
|
||||
if (!current) {
|
||||
useDownloadProgressStore.getState().resetDownloadProgress(payload.id);
|
||||
return;
|
||||
}
|
||||
const status = payload.status as DownloadStatus;
|
||||
const status = payload.status;
|
||||
// A move terminal event carries its authoritative destination. Older
|
||||
// lifecycle events do not, so they must not overwrite an active move or
|
||||
// clear its progress while the native relocation still owns the row.
|
||||
@@ -462,6 +516,7 @@ const startDownloadListeners = async () => {
|
||||
}),
|
||||
listen('torrent-move-progress', (event) => {
|
||||
const payload = event.payload;
|
||||
if (!isRecord(payload) || typeof payload.id !== 'string') return;
|
||||
const current = useDownloadStore.getState().downloads.find(d => d.id === payload.id);
|
||||
if (!current || current.status !== 'moving') {
|
||||
useDownloadProgressStore.getState().clearMoveProgress(payload.id);
|
||||
|
||||
@@ -772,6 +772,28 @@ describe('useDownloadStore', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('replaces stale in-memory downloads when startup loads an empty persisted snapshot', async () => {
|
||||
useDownloadStore.setState({
|
||||
downloads: [{
|
||||
id: 'stale-memory-row',
|
||||
url: 'https://example.com/stale.bin',
|
||||
fileName: 'stale.bin',
|
||||
status: 'completed',
|
||||
category: 'Other',
|
||||
dateAdded: ''
|
||||
}] as any[]
|
||||
});
|
||||
vi.mocked(ipc.invokeCommand).mockImplementation(async (cmd: string) => {
|
||||
if (cmd === 'db_get_all_queues') return [];
|
||||
if (cmd === 'db_get_all_downloads') return [];
|
||||
return undefined;
|
||||
});
|
||||
|
||||
await useDownloadStore.getState().initDB();
|
||||
|
||||
expect(useDownloadStore.getState().downloads).toEqual([]);
|
||||
});
|
||||
|
||||
it('remaps persisted downloads when queue records are malformed or missing', async () => {
|
||||
vi.mocked(ipc.invokeCommand).mockImplementation(async (cmd: string) => {
|
||||
if (cmd === 'db_get_all_queues') {
|
||||
@@ -1336,6 +1358,12 @@ describe('useDownloadStore', () => {
|
||||
expect(
|
||||
vi.mocked(ipc.invokeCommand).mock.calls.filter(([command]) => command === 'remove_download')
|
||||
).toHaveLength(2);
|
||||
expect(
|
||||
vi.mocked(ipc.invokeCommand).mock.calls.some(([command, args]) =>
|
||||
command === 'remove_download'
|
||||
&& (args as { expectedLifecycleGeneration?: string })?.expectedLifecycleGeneration === '0'
|
||||
)
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it('does not expose allocation while admission is merely blocked', async () => {
|
||||
|
||||
@@ -256,9 +256,13 @@ const isCurrentDownloadLifecycle = (id: string, generation: bigint): boolean =>
|
||||
currentDownloadLifecycle(id) === generation &&
|
||||
useDownloadStore.getState().downloads.some(download => download.id === id);
|
||||
|
||||
const removeStaleBackendDispatch = async (id: string): Promise<void> => {
|
||||
const removeStaleBackendDispatch = async (id: string, lifecycleGeneration: bigint): Promise<void> => {
|
||||
try {
|
||||
await invoke('remove_download', { id, deleteAssets: false });
|
||||
await invoke('remove_download', {
|
||||
id,
|
||||
deleteAssets: false,
|
||||
expectedLifecycleGeneration: lifecycleGeneration.toString()
|
||||
});
|
||||
} catch (error) {
|
||||
// The original remove request may already have won this race. Either way,
|
||||
// never allow a stale enqueue to make the deleted row live again.
|
||||
@@ -452,7 +456,7 @@ async function dispatchItemInternal(id: string, proxyOverride?: string | null):
|
||||
const accepted = await invoke('enqueue_download', { item: enqueueItem });
|
||||
backendAccepted = true;
|
||||
if (!isCurrentDownloadLifecycle(id, lifecycleGeneration)) {
|
||||
await removeStaleBackendDispatch(id);
|
||||
await removeStaleBackendDispatch(id, lifecycleGeneration);
|
||||
return false;
|
||||
}
|
||||
|
||||
@@ -469,7 +473,7 @@ async function dispatchItemInternal(id: string, proxyOverride?: string | null):
|
||||
}
|
||||
const order = await invoke('get_pending_order', { queueId: item.queueId || MAIN_QUEUE_ID });
|
||||
if (!isCurrentDownloadLifecycle(id, lifecycleGeneration)) {
|
||||
await removeStaleBackendDispatch(id);
|
||||
await removeStaleBackendDispatch(id, lifecycleGeneration);
|
||||
return false;
|
||||
}
|
||||
|
||||
@@ -483,7 +487,7 @@ async function dispatchItemInternal(id: string, proxyOverride?: string | null):
|
||||
} catch (e) {
|
||||
console.error(`Failed to dispatch ${id}:`, e);
|
||||
if (backendAccepted && lifecycleGeneration !== null) {
|
||||
await removeStaleBackendDispatch(id);
|
||||
await removeStaleBackendDispatch(id, lifecycleGeneration);
|
||||
}
|
||||
if (lifecycleGeneration !== null && isCurrentDownloadLifecycle(id, lifecycleGeneration)) {
|
||||
const proxyBlocked = isSystemProxyConfigurationError(e);
|
||||
@@ -2940,11 +2944,9 @@ export const useDownloadStore = create<DownloadState>((set, get) => {
|
||||
return normalizePersistedDownloadProgress({ ...download, queueId });
|
||||
});
|
||||
|
||||
set(state => ({
|
||||
set(() => ({
|
||||
queues,
|
||||
downloads: downloads.length > 0
|
||||
? normalizeQueuePositions(downloads)
|
||||
: state.downloads
|
||||
downloads: normalizeQueuePositions(downloads)
|
||||
}));
|
||||
|
||||
// A process can die after Aria2 has removed the unselected files but
|
||||
|
||||
@@ -38,6 +38,26 @@ const ACTIVE_DOWNLOAD_STATUSES: ReadonlySet<DownloadStatus> = new Set([
|
||||
'moving',
|
||||
]);
|
||||
|
||||
const DOWNLOAD_STATUSES: ReadonlySet<string> = new Set([
|
||||
'ready',
|
||||
'staged',
|
||||
'downloading',
|
||||
'processing',
|
||||
'seeding',
|
||||
'waitingToSeed',
|
||||
'paused',
|
||||
'completed',
|
||||
'failed',
|
||||
'queued',
|
||||
'retrying',
|
||||
'verifying',
|
||||
'moving',
|
||||
]);
|
||||
|
||||
/** Runtime guard for values arriving from the untyped Tauri event channel. */
|
||||
export const isDownloadStatus = (status: unknown): status is DownloadStatus =>
|
||||
typeof status === 'string' && DOWNLOAD_STATUSES.has(status);
|
||||
|
||||
export const isActiveDownloadStatus = (status: DownloadStatus): boolean =>
|
||||
ACTIVE_DOWNLOAD_STATUSES.has(status);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user