diff --git a/CHANGELOG.md b/CHANGELOG.md index 4430892a..940e0575 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Exporting the config now reports a failure instead of saving an error response + as `stackyard-config.json`. + - A session that expires while you are working in Admin now raises the sign-in box over the page, and the interrupted save completes once you are back in, instead of failing as "Unauthorised" until the page is reloaded. diff --git a/ui/js/admin.js b/ui/js/admin.js index 338e622a..5ed6ecd9 100644 --- a/ui/js/admin.js +++ b/ui/js/admin.js @@ -10,7 +10,7 @@ import { snapshotItems, saveWithRevert, } from '/js/admin-save-logic.js?v=77cac1d1'; -import { API, toast, ag, ap, initInlineEdit, setReauthHandler } from '/js/admin-shared.js?v=bb36dbb3'; +import { toast, ag, ap, initInlineEdit, setReauthHandler } from '/js/admin-shared.js?v=bb36dbb3'; import { checkAuth, requireLogin, wirePasswordStrength } from '/js/admin-auth.js?v=dd849d4c'; import { state } from '/js/admin-state.js?v=3f9ad806'; import { buildWidgetForm } from '/js/admin-widget-form.js?v=653071f0'; @@ -1232,16 +1232,29 @@ function initLanguage() { const dashSaveEl = el('dash-save'); if (dashSaveEl) dashSaveEl.onclick = () => save(); +/* Fetched rather than reached by navigating a link. A link hands the request to + the browser, so the page never learns the outcome: a refusal or an expired + session saved an error body under the backup's own filename, and the catch + here could not see it. Going through `ag` also means an expired session + raises the sign-in box and the export finishes afterwards. */ el('btn-exp').onclick = async () => { + let url; try { + const config = await ag('/api/config/export'); + const blob = new Blob([JSON.stringify(config, null, 2)], { type: 'application/json' }); + url = URL.createObjectURL(blob); const a = document.createElement('a'); - a.href = API + '/api/config/export'; + a.href = url; a.download = 'stackyard-config.json'; document.body.appendChild(a); a.click(); document.body.removeChild(a); } catch (e) { toast(t('toast.exportFailed', { err: e.message }), 'err'); + } finally { + /* Revoked on the next frame: revoking it in this one races the download the + click just started, and the browser keeps nothing to save. */ + if (url) requestAnimationFrame(() => URL.revokeObjectURL(url)); } }; el('imp').onchange = async e => { diff --git a/ui/test/config-export.test.mjs b/ui/test/config-export.test.mjs new file mode 100644 index 00000000..de7c9106 --- /dev/null +++ b/ui/test/config-export.test.mjs @@ -0,0 +1,65 @@ +/* Regression test for the config export, which could not report a failure. + + The export set an anchor's href to /api/config/export and clicked it, which + hands the request to the browser. The page then never learns the outcome: a + refusal, a server error or an expired session was written to disk as an error + body under the backup's own filename, and the try/catch around the click + could not observe any of it. The failure surfaces when the backup is needed, + which is the worst moment to find out. + + Fetching it instead means the response is checked before anything is saved, + and going through `ag` means an expired session raises the sign-in box and + the export finishes afterwards. + + Scanned from the source rather than driven, because the download itself is + browser machinery: what matters is which of the two routes the code takes. */ + +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const root = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); +const admin = fs.readFileSync(path.join(root, 'js/admin.js'), 'utf8'); + +/* The handler only, so an unrelated mention of the endpoint elsewhere in the + file cannot pass or fail this. */ +function exportHandler() { + const start = admin.indexOf("el('btn-exp').onclick"); + assert.notEqual(start, -1, 'the export handler moved; this test needs updating'); + const end = admin.indexOf("el('imp').onchange", start); + assert.notEqual(end, -1, 'the export handler moved; this test needs updating'); + return admin.slice(start, end); +} + +test('the export never reaches the endpoint by navigating to it', () => { + const body = exportHandler(); + assert.ok( + !/\.href\s*=\s*[^;]*\/api\/config\/export/.test(body), + 'assigning the endpoint to an href hands the request to the browser, which cannot report a failure', + ); +}); + +test('the export goes through the shared helper, so a failure is visible', () => { + const body = exportHandler(); + assert.match( + body, + /await ag\(\s*'\/api\/config\/export'\s*\)/, + 'the response has to be checked before anything is saved', + ); +}); + +test('the export reports a failure it can now see', () => { + assert.match(exportHandler(), /toast\(\s*t\('toast\.exportFailed'/); +}); + +test('the object URL is released rather than leaked', () => { + const body = exportHandler(); + assert.match(body, /createObjectURL/); + assert.match(body, /revokeObjectURL/, 'a blob URL held for the page lifetime pins the whole config in memory'); +}); + +test('the download keeps the filename users already know', () => { + assert.match(exportHandler(), /download\s*=\s*'stackyard-config\.json'/); +});