fix(system): keep a restore review's upload form alive until it is answered (#583)
WebKit can read a FormData's file part after fetch() returned, and with a
request interceptor armed it does. uploadRestore held its FormData only in a
local that was dead after the call, so a garbage collection in between dropped
the form and the server got an empty file ("Uploaded file is empty"); the
Restore preview never opened. The page now holds each upload's form until the
upload is answered.
This commit is contained in:
@@ -284,6 +284,22 @@ test('a restore seen finishing elsewhere closes an open restore review and disca
|
||||
expect((await serverJson(request, '/__test__/system/backups')).stages).toBe(0);
|
||||
});
|
||||
|
||||
// #583: in WebKit, while a page.route is armed, a file part of a FormData body is read only when the request goes on;
|
||||
// a garbage collection in between dropped the form once nothing referenced it, and the server got an empty file
|
||||
// ("Uploaded file is empty"). The page keeps the form until its upload is answered. The route holds the upload
|
||||
// through a collection, so the loss is certain rather than left to when the collector happens to run.
|
||||
test('a review upload survives a garbage collection while it is in flight', async ({ page, request }) => {
|
||||
const bytes = await backupBytes(request);
|
||||
await page.route(`**${PREVIEW_URL}`, async route => {
|
||||
await page.requestGC();
|
||||
await route.continue();
|
||||
});
|
||||
await openBackups(page);
|
||||
expect((await chooseBackup(page, bytes)).status()).toBe(200);
|
||||
await expect(dialog(page).locator('#modal-title')).toHaveText('Restore preview');
|
||||
expect((await serverJson(request, '/__test__/system/backups')).stages).toBe(1);
|
||||
});
|
||||
|
||||
test('invalid file shows the validator message; nothing is staged', async ({ page, request }) => {
|
||||
const before = await serverJson(request, '/__test__/system/digest');
|
||||
await openBackups(page);
|
||||
|
||||
@@ -128,6 +128,24 @@ test('invalid file shows the validator message; a refusal names its job; a lost
|
||||
assert.deepEqual(await uploadRestore(file, { ApiError, fetchUrl: async () => response(200, PREVIEW) }), PREVIEW);
|
||||
});
|
||||
|
||||
// #583: WebKit reads a FormData's file part by reference, possibly after a garbage collection; the page holds the form
|
||||
// until the upload is answered, then lets it go, whether the server staged it, refused it or the request failed.
|
||||
test('a review upload keeps its form referenced until the upload is answered', async () => {
|
||||
const { uploadRestore, uploadsInFlight } = await backups;
|
||||
const file = new File(['bytes'], 'b.db');
|
||||
for (const answer of [async () => response(200, PREVIEW), async () => response(400, { success: false, error: 'x' }),
|
||||
async () => { throw new TypeError('offline'); }]) {
|
||||
let held;
|
||||
const upload = uploadRestore(file, { ApiError, fetchUrl: async (url, init) => {
|
||||
held = uploadsInFlight.has(init.body) && init.body.get('database_file').size === file.size;
|
||||
return answer();
|
||||
} }).catch(() => {});
|
||||
await upload;
|
||||
assert.equal(held, true);
|
||||
assert.equal(uploadsInFlight.size, 0);
|
||||
}
|
||||
});
|
||||
|
||||
test('Cancel discards the stage through the api client and never throws', async () => {
|
||||
const { discardRestore, PREVIEW_URL } = await backups;
|
||||
const calls = [];
|
||||
|
||||
@@ -179,13 +179,26 @@ export function reviewRefusal(error) {
|
||||
return REVIEW_FAILED;
|
||||
}
|
||||
|
||||
/**
|
||||
* #583: the review uploads not answered yet. WebKit can read a FormData's file part after the fetch call returned (it
|
||||
* did whenever a request interceptor, here Playwright's, was armed); a garbage collection in between dropped a form
|
||||
* nothing referenced any more, and the server got an empty file. Each form is held here until its upload is answered.
|
||||
*/
|
||||
export const uploadsInFlight = new Set();
|
||||
|
||||
/** Stage `file` for review; the server's answer, or an ApiError naming its refusal. */
|
||||
export async function uploadRestore(file, { fetchUrl = window.ApiClient.fetchUrl, ApiError = window.ApiClient.ApiError,
|
||||
signal } = {}) {
|
||||
const form = new FormData();
|
||||
form.append('database_file', file, file.name);
|
||||
const response = await fetchUrl(PREVIEW_URL, { method: 'POST', body: form, cache: 'no-store', credentials: 'same-origin',
|
||||
signal });
|
||||
uploadsInFlight.add(form);
|
||||
let response;
|
||||
try {
|
||||
response = await fetchUrl(PREVIEW_URL, { method: 'POST', body: form, cache: 'no-store', credentials: 'same-origin',
|
||||
signal });
|
||||
} finally {
|
||||
uploadsInFlight.delete(form);
|
||||
}
|
||||
let data = null;
|
||||
try {
|
||||
data = await response.json();
|
||||
|
||||
Reference in New Issue
Block a user