mirror of
https://github.com/nimbold/Firelink.git
synced 2026-07-26 12:08:27 +00:00
fix(downloads): harden Add Window handoffs
This commit is contained in:
@@ -316,7 +316,17 @@ fn normalize_download(payload: ExtensionRequest) -> Option<ExtensionDownload> {
|
||||
matches!(url.scheme(), "http" | "https").then(|| url.to_string())
|
||||
});
|
||||
let filename = payload.filename.and_then(|value| sanitize_filename(&value));
|
||||
let headers = normalize_headers(payload.headers, payload.media);
|
||||
// A multi-URL handoff has no per-URL cookie scope. Keep ordinary
|
||||
// request headers, but drop Cookie headers and the dedicated cookie field
|
||||
// so a legacy or untrusted caller cannot reuse one session across hosts.
|
||||
let headers = normalize_headers(payload.headers, payload.media || urls.len() > 1);
|
||||
let cookies = if !payload.media && urls.len() == 1 {
|
||||
payload
|
||||
.cookies
|
||||
.filter(|value| !value.trim().is_empty())
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
||||
Some(ExtensionDownload {
|
||||
urls,
|
||||
@@ -329,10 +339,7 @@ fn normalize_download(payload: ExtensionRequest) -> Option<ExtensionDownload> {
|
||||
// Cookie header can exceed upstream limits and makes old extension
|
||||
// builds pay for a doomed metadata request before retrying. Ordinary
|
||||
// captured downloads still need their exact request cookies.
|
||||
cookies: (!payload.media)
|
||||
.then_some(payload.cookies)
|
||||
.flatten()
|
||||
.filter(|value| !value.trim().is_empty()),
|
||||
cookies,
|
||||
media: payload.media,
|
||||
})
|
||||
}
|
||||
@@ -568,6 +575,26 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn multi_url_capture_drops_cookie_scope_but_preserves_safe_headers() {
|
||||
let download = normalize_download(ExtensionRequest {
|
||||
urls: vec![
|
||||
"https://one.example/private.zip".to_string(),
|
||||
"https://two.example/file.zip".to_string(),
|
||||
],
|
||||
referer: None,
|
||||
silent: true,
|
||||
filename: None,
|
||||
headers: Some("Cookie: session=secret\nUser-Agent: Firefox".to_string()),
|
||||
cookies: Some("session=secret".to_string()),
|
||||
media: false,
|
||||
})
|
||||
.expect("valid multi-url handoff");
|
||||
|
||||
assert_eq!(download.cookies, None);
|
||||
assert_eq!(download.headers.as_deref(), Some("User-Agent: Firefox"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn signs_server_proof_with_timestamp_nonce_and_bound_port() {
|
||||
let token = Arc::new(RwLock::new("pairing-token".to_string()));
|
||||
|
||||
@@ -539,6 +539,7 @@ export const AddDownloadsModal = () => {
|
||||
setResolvedLocation(finalLocation);
|
||||
const store = useDownloadStore.getState();
|
||||
const newConflicts: DuplicateConflict[] = [];
|
||||
const plannedTargets: Array<{ location: string; fileName: string }> = [];
|
||||
|
||||
for (let i = 0; i < parsedItems.length; i++) {
|
||||
const item = parsedItems[i];
|
||||
@@ -550,8 +551,25 @@ export const AddDownloadsModal = () => {
|
||||
: destinationOverrides[i] || await categoryLocationForFile(finalFile);
|
||||
|
||||
const isUrlDupe = store.downloads.some(d => d.url === item.downloadUrl && d.status !== 'failed' && d.status !== 'completed');
|
||||
const hasBatchConflict = plannedTargets.some(target =>
|
||||
downloadLocationEquals(
|
||||
target.location,
|
||||
target.fileName,
|
||||
itemLocation,
|
||||
finalFile,
|
||||
platform.os
|
||||
)
|
||||
);
|
||||
if (isUrlDupe) {
|
||||
newConflicts.push({ id: i.toString(), fileName: finalFile, reason: { type: 'url', msg: 'URL already in queue' }, resolution: 'rename' });
|
||||
} else if (hasBatchConflict) {
|
||||
newConflicts.push({
|
||||
id: i.toString(),
|
||||
fileName: finalFile,
|
||||
reason: { type: 'file', msg: 'Another selected download uses this destination' },
|
||||
resolution: 'rename',
|
||||
replaceAllowed: false
|
||||
});
|
||||
} else {
|
||||
let fileExistsInStore = false;
|
||||
for (const download of store.downloads) {
|
||||
@@ -595,6 +613,7 @@ export const AddDownloadsModal = () => {
|
||||
});
|
||||
}
|
||||
}
|
||||
plannedTargets.push({ location: itemLocation, fileName: finalFile });
|
||||
}
|
||||
|
||||
if (newConflicts.length > 0) {
|
||||
@@ -646,6 +665,17 @@ export const AddDownloadsModal = () => {
|
||||
const ext = finalFile.includes('.') ? finalFile.substring(finalFile.lastIndexOf('.')) : '';
|
||||
let newName = finalFile;
|
||||
let exists = true;
|
||||
const batchTargets: Array<{ location: string; fileName: string }> = [];
|
||||
for (const [candidateIndex, candidate] of itemsToAdd.entries()) {
|
||||
if (!candidate || candidateIndex === idx) continue;
|
||||
const candidateFile = candidate.isMedia
|
||||
? mediaFileNameForSelectedFormat(candidate.file, candidate)
|
||||
: canonicalizeDownloadFileName(candidate.file);
|
||||
const candidateLocation = useSharedDestination
|
||||
? finalLocation
|
||||
: destinationOverrides[candidateIndex] || await categoryLocationForFile(candidateFile);
|
||||
batchTargets.push({ location: candidateLocation, fileName: candidateFile });
|
||||
}
|
||||
|
||||
while (exists && count < 1000) {
|
||||
newName = `${base} (${count})${ext}`;
|
||||
@@ -674,7 +704,14 @@ export const AddDownloadsModal = () => {
|
||||
path: await resolveDownloadFilePath(itemLocation, newName)
|
||||
});
|
||||
} catch(e) {}
|
||||
exists = storeHas || diskHas;
|
||||
const batchHas = batchTargets.some(target => downloadLocationEquals(
|
||||
target.location,
|
||||
target.fileName,
|
||||
itemLocation,
|
||||
newName,
|
||||
platform.os
|
||||
));
|
||||
exists = storeHas || diskHas || batchHas;
|
||||
count++;
|
||||
}
|
||||
if (exists) {
|
||||
@@ -1201,7 +1238,7 @@ export const AddDownloadsModal = () => {
|
||||
{metadataSummaryMessage(parsedItems)}
|
||||
</div>
|
||||
<div className="flex gap-2.5">
|
||||
<button onClick={() => toggleAddModal(false)} className="add-download-button add-download-button-cancel px-4 text-xs">
|
||||
<button onClick={() => toggleAddModal(false)} disabled={isSubmitting} className="add-download-button add-download-button-cancel px-4 text-xs">
|
||||
Cancel
|
||||
</button>
|
||||
<div ref={actionMenuRef} className="relative flex gap-2.5">
|
||||
|
||||
@@ -125,6 +125,31 @@ describe('add download metadata workflow', () => {
|
||||
});
|
||||
});
|
||||
|
||||
it('replaces a stale filename when a newer handoff supplies a new one', () => {
|
||||
const existing = row({
|
||||
file: 'old-name.zip',
|
||||
requestContextVersion: 1,
|
||||
generation: 2
|
||||
});
|
||||
|
||||
const refreshed = reconcileDownloadRows(
|
||||
existing.sourceUrl,
|
||||
[existing],
|
||||
undefined,
|
||||
new Set(),
|
||||
() => 'unused',
|
||||
{ [existing.sourceUrl]: 'new-name.zip' },
|
||||
{ [existing.sourceUrl]: 2 }
|
||||
);
|
||||
|
||||
expect(refreshed[0]).toMatchObject({
|
||||
file: 'new-name.zip',
|
||||
status: 'loading',
|
||||
generation: 3,
|
||||
requestContextVersion: 2
|
||||
});
|
||||
});
|
||||
|
||||
it('appends every unseen handoff after the observed version', () => {
|
||||
const merged = appendRequestUrlsAfterVersion(
|
||||
'https://existing.example/file.zip',
|
||||
|
||||
@@ -88,8 +88,12 @@ export const reconcileDownloadRows = (
|
||||
const contextChanged = requestContextVersion !== undefined
|
||||
&& requestContextVersion !== preserved.requestContextVersion;
|
||||
if ((forcedMedia && !preserved.isMedia) || contextChanged) {
|
||||
const requestedFilename = requestFilenames[input.sourceUrl];
|
||||
return {
|
||||
...preserved,
|
||||
file: contextChanged
|
||||
? canonicalizeDownloadFileName(requestedFilename || fileNameFromUrl(input.sourceUrl))
|
||||
: preserved.file,
|
||||
status: 'loading',
|
||||
generation: preserved.generation + 1,
|
||||
requestContextVersion,
|
||||
|
||||
Reference in New Issue
Block a user