From e184dd61b64b4596e7346fc0b307d390eec8fb0e Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Mon, 19 Oct 2026 12:45:49 -0400 Subject: [PATCH 01/10] feat(main): stream downloads through a first-party service worker (DSPX-159) Replace StreamSaver, which routed downloads through a service worker on jimmywarting.github.io, with our own worker in web-app/public/sw.js. The page sends output to the worker over a MessagePort, one chunk per pull, and a hidden iframe opens the streamed attachment response. Backpressure from the browser's download manager reaches back to the SDK, so memory stays flat at any file size. The existing sw.js could not have worked: its origin check never matched, and it acknowledged chunks on enqueue, so nothing bounded its queue. Save As now uses the native showSaveFilePicker and is disabled where the browser lacks it, so native-file-system-adapter is gone too. Tests: - Add tests/sink.spec.ts, which exercises the sink with no platform and can run a large, paced download with SINK_TEST_BYTES and SINK_TEST_MIBPS. - Add playwright.moz-firefox.config.ts to run the suite on stock Firefox over WebDriver BiDi. Stock Firefox ignores Playwright's download folder, so specs now find files with downloadPath() from acts.ts. - huge.spec.ts now runs on WebKit too. It still skips Playwright's bundled Firefox, which keeps every response body in memory (DSPX-5084). Signed-off-by: Dave Mihalcik --- web-app/package-lock.json | 100 +--------- web-app/package.json | 5 +- web-app/public/sw.js | 165 ++++++++++------ web-app/src/App.tsx | 29 +-- web-app/src/downloadSink.ts | 186 ++++++++++++++++++ web-app/src/main.tsx | 3 +- .../tests/playwright.moz-firefox.config.ts | 49 +++++ web-app/tests/tests/acts.ts | 22 +++ web-app/tests/tests/huge.spec.ts | 19 +- web-app/tests/tests/roundtrip.spec.ts | 18 +- web-app/tests/tests/sink.spec.ts | 155 +++++++++++++++ 11 files changed, 552 insertions(+), 199 deletions(-) create mode 100644 web-app/src/downloadSink.ts create mode 100644 web-app/tests/playwright.moz-firefox.config.ts create mode 100644 web-app/tests/tests/sink.spec.ts diff --git a/web-app/package-lock.json b/web-app/package-lock.json index d81b0e0a4..d1a20fbac 100644 --- a/web-app/package-lock.json +++ b/web-app/package-lock.json @@ -12,10 +12,8 @@ "@opentdf/sdk": "file:../lib/opentdf-sdk-0.22.0.tgz", "clsx": "^2.1.1", "dpop": "^1.4.1", - "native-file-system-adapter": "^3.0.1", "react": "^19.0.0", - "react-dom": "^19.2.4", - "streamsaver": "^2.0.6" + "react-dom": "^19.2.4" }, "devDependencies": { "@eslint/js": "10.0.1", @@ -24,7 +22,6 @@ "@types/node": "24.12.0", "@types/react": "^19.0.10", "@types/react-dom": "^19.2.3", - "@types/streamsaver": "^2.0.5", "@types/wicg-file-system-access": "^2023.10.5", "@vitejs/plugin-react": "^5.1.3", "@vitest/coverage-v8": "^4.1.11", @@ -1440,13 +1437,6 @@ "@types/react": "^19.2.0" } }, - "node_modules/@types/streamsaver": { - "version": "2.0.5", - "resolved": "https://registry.npmjs.org/@types/streamsaver/-/streamsaver-2.0.5.tgz", - "integrity": "sha512-93o0zjV8swEhR2YI57h/2ytbJF8bJh7sI9GNB02TLJHdM4fWDxZuChwfWhyD8vt2ub4kw4rsfZ0C0yAUX+3gcg==", - "dev": true, - "license": "MIT" - }, "node_modules/@types/wicg-file-system-access": { "version": "2023.10.5", "resolved": "https://registry.npmjs.org/@types/wicg-file-system-access/-/wicg-file-system-access-2023.10.5.tgz", @@ -2849,30 +2839,6 @@ } } }, - "node_modules/fetch-blob": { - "version": "3.2.0", - "resolved": "https://registry.npmjs.org/fetch-blob/-/fetch-blob-3.2.0.tgz", - "integrity": "sha512-7yAQpD2UMJzLi1Dqv7qFYnPbaPx7ZfFK6PiIxQ4PfkGPyNyl2Ugx+a/umUonmKqjhM4DnfbMvdX6otXq83soQQ==", - "funding": [ - { - "type": "github", - "url": "https://github.com/sponsors/jimmywarting" - }, - { - "type": "paypal", - "url": "https://paypal.me/jimmywarting" - } - ], - "license": "MIT", - "optional": true, - "dependencies": { - "node-domexception": "^1.0.0", - "web-streams-polyfill": "^3.0.3" - }, - "engines": { - "node": "^12.20 || >= 14.13" - } - }, "node_modules/fflate": { "version": "0.8.2", "resolved": "https://registry.npmjs.org/fflate/-/fflate-0.8.2.tgz", @@ -4058,28 +4024,6 @@ "node": "^10 || ^12 || ^13.7 || ^14 || >=15.0.1" } }, - "node_modules/native-file-system-adapter": { - "version": "3.0.1", - "resolved": "https://registry.npmjs.org/native-file-system-adapter/-/native-file-system-adapter-3.0.1.tgz", - "integrity": "sha512-ocuhsYk2SY0906LPc3QIMW+rCV3MdhqGiy7wV5Bf0e8/5TsMjDdyIwhNiVPiKxzTJLDrLT6h8BoV9ERfJscKhw==", - "funding": [ - { - "type": "github", - "url": "https://github.com/sponsors/jimmywarting" - }, - { - "type": "paypal", - "url": "https://paypal.me/jimmywarting" - } - ], - "license": "MIT", - "engines": { - "node": ">=14.8.0" - }, - "optionalDependencies": { - "fetch-blob": "^3.2.0" - } - }, "node_modules/natural-compare": { "version": "1.4.0", "resolved": "https://registry.npmjs.org/natural-compare/-/natural-compare-1.4.0.tgz", @@ -4087,26 +4031,6 @@ "dev": true, "license": "MIT" }, - "node_modules/node-domexception": { - "version": "1.0.0", - "resolved": "https://registry.npmjs.org/node-domexception/-/node-domexception-1.0.0.tgz", - "integrity": "sha512-/jKZoMpw0F8GRwl4/eLROPA3cfcXtLApP0QzLmUT/HuPCZWyB7IY9ZrMeKw2O/nFIqPQB3PVM9aYm0F312AXDQ==", - "funding": [ - { - "type": "github", - "url": "https://github.com/sponsors/jimmywarting" - }, - { - "type": "github", - "url": "https://paypal.me/jimmywarting" - } - ], - "license": "MIT", - "optional": true, - "engines": { - "node": ">=10.5.0" - } - }, "node_modules/node-preload": { "version": "0.2.1", "resolved": "https://registry.npmjs.org/node-preload/-/node-preload-0.2.1.tgz", @@ -5225,18 +5149,6 @@ "dev": true, "license": "MIT" }, - "node_modules/streamsaver": { - "version": "2.0.6", - "resolved": "https://registry.npmjs.org/streamsaver/-/streamsaver-2.0.6.tgz", - "integrity": "sha512-LK4e7TfCV8HzuM0PKXuVUfKyCB1FtT9L0EGxsFk5Up8njj0bXK8pJM9+Wq2Nya7/jslmCQwRK39LFm55h7NBTw==", - "funding": [ - { - "type": "github", - "url": "https://github.com/sponsors/jimmywarting" - } - ], - "license": "MIT" - }, "node_modules/string-width": { "version": "5.1.2", "resolved": "https://registry.npmjs.org/string-width/-/string-width-5.1.2.tgz", @@ -5881,16 +5793,6 @@ } } }, - "node_modules/web-streams-polyfill": { - "version": "3.3.3", - "resolved": "https://registry.npmjs.org/web-streams-polyfill/-/web-streams-polyfill-3.3.3.tgz", - "integrity": "sha512-d2JWLCivmZYTSIoge9MsgFCZrt571BikcWGYkjC1khllbTeDlGqZ2D8vD8E/lJa8WGWbb7Plm8/XJYV7IJHZZw==", - "license": "MIT", - "optional": true, - "engines": { - "node": ">= 8" - } - }, "node_modules/which": { "version": "2.0.2", "resolved": "https://registry.npmjs.org/which/-/which-2.0.2.tgz", diff --git a/web-app/package.json b/web-app/package.json index 72b18e799..6d6b9501a 100644 --- a/web-app/package.json +++ b/web-app/package.json @@ -21,10 +21,8 @@ "@opentdf/sdk": "file:../lib/opentdf-sdk-0.22.0.tgz", "clsx": "^2.1.1", "dpop": "^1.4.1", - "native-file-system-adapter": "^3.0.1", "react": "^19.0.0", - "react-dom": "^19.2.4", - "streamsaver": "^2.0.6" + "react-dom": "^19.2.4" }, "devDependencies": { "@eslint/js": "10.0.1", @@ -33,7 +31,6 @@ "@types/node": "24.12.0", "@types/react": "^19.0.10", "@types/react-dom": "^19.2.3", - "@types/streamsaver": "^2.0.5", "@types/wicg-file-system-access": "^2023.10.5", "@vitejs/plugin-react": "^5.1.3", "@vitest/coverage-v8": "^4.1.11", diff --git a/web-app/public/sw.js b/web-app/public/sw.js index c7d9a1a19..9e8a9412b 100644 --- a/web-app/public/sw.js +++ b/web-app/public/sw.js @@ -1,52 +1,80 @@ -const WRITE = 0; -const PULL = 0; -const ERROR = 1; -const ABORT = 1; -const CLOSE = 2; -const PING = 3; +// Streams downloads from the page straight into the browser's download manager, +// so a multi-GiB encrypt or decrypt never has to fit in memory or in storage +// quota. The page side, and the message protocol, live in src/downloadSink.ts. +// +// 1. The page posts { type: 'register', id, filename } with a MessagePort. +// 2. We wrap the port in a ReadableStream and reply { type: 'ready' }. +// 3. The page points a hidden iframe at __download__//, +// and we answer that navigation with the stream as an attachment. +// 4. Each time the stream wants more, we post { type: 'pull' }; the page sends +// one { type: 'chunk' } per pull, so backpressure from the download manager +// reaches all the way back to the SDK. -/** @implements {UnderlyingSource} */ -class MessagePortSource { - /** @type {ReadableStreamController} controller */ - controller; +const HIGH_WATER_MARK_BYTES = 4 * 2 ** 20; - /** @param {MessagePort} port */ - constructor(port) { - this.port = port; - this.port.onmessage = (evt) => this.onMessage(evt.data); - } +/** @type {Map, filename: string }>} */ +const pending = new Map(); + +function downloadBase() { + return new URL('__download__/', self.registration.scope).href; +} +/** @implements {UnderlyingDefaultSource} */ +class PortSource { /** - * @param {ReadableStreamController} controller + * @param {MessagePort} port + * @param {() => void} forget drops the entry if the download never started */ + constructor(port, forget) { + this.port = port; + this.forget = forget; + } + + /** @param {ReadableStreamDefaultController} controller */ start(controller) { - this.controller = controller; + this.port.onmessage = ({ data }) => { + switch (data?.type) { + case 'chunk': + controller.enqueue(new Uint8Array(data.chunk)); + break; + case 'close': + controller.close(); + this.port.close(); + break; + case 'abort': + // Erroring the body makes the browser mark the download failed, + // rather than leaving a truncated file that looks complete. + controller.error(new Error(data.reason)); + this.port.close(); + this.forget(); + break; + } + }; + } + + pull() { + this.port.postMessage({ type: 'pull' }); } - /** @param {Error} reason */ + /** @param {unknown} reason */ cancel(reason) { - // Firefox can notify a cancel event, chrome can't - // https://bugs.chromium.org/p/chromium/issues/detail?id=638494 - this.port.postMessage({ type: ERROR, reason: reason.message }); + // The user cancelled in the download UI. Firefox reports this; Chromium + // does not (https://crbug.com/638494), so there the page finds out only + // when it aborts on its own. + this.port.postMessage({ type: 'cancel', reason: String(reason) }); this.port.close(); } +} - /** @param {{ type: number; chunk: Uint8Array; reason: any; }} message */ - onMessage(message) { - // enqueue() will call pull() if needed when there's no backpressure - if (message.type === WRITE) { - this.controller.enqueue(message.chunk); - this.port.postMessage({ type: PULL }); - } - if (message.type === ABORT) { - this.controller.error(message.reason); - this.port.close(); - } - if (message.type === CLOSE) { - this.controller.close(); - this.port.close(); - } - } +/** @param {string} filename */ +function contentDisposition(filename) { + const safe = filename.replace(/[\u0000-\u001f\u007f/\\]/g, '_'); + const ascii = safe.replace(/[^\x20-\x7e]|["%]/g, '_'); + const encoded = encodeURIComponent(safe).replace( + /['()*]/g, + (c) => `%${c.charCodeAt(0).toString(16).toUpperCase()}` + ); + return `attachment; filename="${ascii}"; filename*=UTF-8''${encoded}`; } self.addEventListener('install', () => { @@ -57,33 +85,52 @@ self.addEventListener('activate', (event) => { event.waitUntil(self.clients.claim()); }); -const map = new Map(); - -// This should be called once per download -// Each event has a dataChannel that the data will be piped through -globalThis.addEventListener('message', (evt) => { - if (evt.origin !== 'http://localhost:65432/') { - console.log(`invalid origin ${evt.origin}`); +self.addEventListener('message', (event) => { + if (event.origin !== self.location.origin) { + console.warn(`download worker: ignoring message from ${event.origin}`); return; } - const data = evt.data; - if (data.url && data.readablePort) { - data.rs = new ReadableStream( - new MessagePortSource(evt.data.readablePort), - new CountQueuingStrategy({ highWaterMark: 4 }) - ); - map.set(data.url, data); + const { data } = event; + // 'ping' needs no handling: receiving a message is what keeps us alive. + if (data?.type !== 'register') { + return; + } + const [port] = event.ports; + if (!port || typeof data.id !== 'string' || typeof data.filename !== 'string') { + console.warn('download worker: malformed register message'); + return; } + const { id, filename } = data; + const stream = new ReadableStream( + new PortSource(port, () => pending.delete(id)), + new ByteLengthQueuingStrategy({ highWaterMark: HIGH_WATER_MARK_BYTES }) + ); + pending.set(id, { stream, filename }); + port.postMessage({ type: 'ready' }); }); -globalThis.addEventListener('fetch', (evt) => { - const url = evt.request.url; - const data = map.get(url); - if (!data) return null; - map.delete(url); - evt.respondWith( - new Response(data.rs, { - headers: data.headers, +self.addEventListener('fetch', (event) => { + const { url } = event.request; + const base = downloadBase(); + if (!url.startsWith(base)) { + return; + } + const [id] = url.slice(base.length).split('/'); + const entry = pending.get(id); + if (!entry) { + event.respondWith(new Response('No such download, or it already started', { status: 404 })); + return; + } + // Single use: a second request for the same id gets a 404. + pending.delete(id); + event.respondWith( + new Response(entry.stream, { + headers: { + 'Cache-Control': 'no-store', + 'Content-Disposition': contentDisposition(entry.filename), + 'Content-Type': 'application/octet-stream', + 'X-Content-Type-Options': 'nosniff', + }, }) ); }); diff --git a/web-app/src/App.tsx b/web-app/src/App.tsx index cb166d004..940b1b392 100644 --- a/web-app/src/App.tsx +++ b/web-app/src/App.tsx @@ -1,7 +1,5 @@ import { clsx } from 'clsx'; import { useState, useEffect, type ChangeEvent } from 'react'; -import streamsaver from 'streamsaver'; -import { showSaveFilePicker } from 'native-file-system-adapter'; import './App.css'; import { type Chunker, @@ -21,19 +19,7 @@ import { decryptedFileName, expectedKaoType, } from './fileNames.js'; - -async function toFile( - stream: ReadableStream, - filepath = 'download.tdf', - options?: StreamPipeOptions -): Promise { - const fileStream = streamsaver.createWriteStream(filepath, { - writableStrategy: { highWaterMark: 1 }, - readableStrategy: { highWaterMark: 1 }, - }); - - return stream.pipeTo(fileStream, options); -} +import { canSaveAs, toFile } from './downloadSink.js'; const oidcClient = new OidcClient(config.oidc.host, config.oidc.clientId, 'otdf-sample-web-app'); @@ -46,14 +32,13 @@ async function getNewFileHandle( { description: `${extension} files`, accept: { - 'application/octet-stream': [`.${extension}`], + 'application/octet-stream': [`.${extension}` as const], }, }, ], suggestedName, }; - //@ts-expect-error //TS2739: not a complete file picker interface - return showSaveFilePicker(options); + return window.showSaveFilePicker(options); } type CurrentDataController = AbortController | undefined; @@ -718,8 +703,14 @@ function App() { value="fsapi" onChange={(e) => setSinkType(e.target.value as SinkType)} checked={sinkType === 'fsapi'} + disabled={!canSaveAs()} />{' '} - +
{ + console.warn('Download service worker registration failed', e); + }); +} + +export function canSaveAs(): boolean { + return 'showSaveFilePicker' in window; +} + +export async function toFile( + stream: ReadableStream, + filename = 'download.tdf', + options?: StreamPipeOptions +): Promise { + const registration = await withTimeout(navigator.serviceWorker?.ready); + if (registration?.active) { + const sink = new PortSink(); + registration.active.postMessage({ type: 'register', id: sink.id, filename }, [sink.remotePort]); + // An outdated or broken worker may never answer; the stream is untouched + // until it does, so falling back is still possible. + if (await withTimeout(sink.ready)) { + await streamThroughWorker(registration, sink, stream, filename, options); + return 'service-worker'; + } + } + console.warn('Download service worker unavailable; buffering the whole output in memory'); + await saveAsBlob(stream, filename, options); + return 'blob'; +} + +async function streamThroughWorker( + registration: ServiceWorkerRegistration, + sink: PortSink, + stream: ReadableStream, + filename: string, + options?: StreamPipeOptions +) { + const iframe = document.createElement('iframe'); + iframe.hidden = true; + iframe.src = new URL( + `__download__/${sink.id}/${encodeURIComponent(filename)}`, + registration.scope + ).href; + document.body.appendChild(iframe); + const keepalive = setInterval( + () => registration.active?.postMessage({ type: 'ping' }), + KEEPALIVE_MS + ); + try { + await stream.pipeTo(new WritableStream(sink, { highWaterMark: 1 }), options); + } finally { + clearInterval(keepalive); + setTimeout(() => iframe.remove(), IFRAME_LINGER_MS); + } +} + +/** Sends one chunk per credit; the worker grants a credit each time its stream pulls. */ +class PortSink implements UnderlyingSink { + readonly id = crypto.randomUUID(); + readonly remotePort: MessagePort; + readonly ready: Promise; + #port: MessagePort; + #credits = 0; + #grant?: () => void; + // Set when the download is cancelled or the stream aborted; wakes a write + // that is waiting for a credit. + #stopped?: Error; + #controller?: WritableStreamDefaultController; + + constructor() { + const { port1, port2 } = new MessageChannel(); + this.#port = port1; + this.remotePort = port2; + let markReady: (ok: true) => void; + this.ready = new Promise((resolve) => (markReady = resolve)); + port1.onmessage = ({ data }: MessageEvent) => { + switch (data.type) { + case 'ready': + markReady(true); + break; + case 'pull': + this.#credits++; + this.#grant?.(); + break; + case 'cancel': + this.#stop(new Error(`Download cancelled: ${data.reason ?? 'no reason given'}`)); + this.#controller?.error(this.#stopped); + break; + } + }; + } + + start(controller: WritableStreamDefaultController) { + this.#controller = controller; + // abort() is only called once the in-flight write settles, and that write + // may be waiting for a credit the worker will never send (for example, + // because the browser hasn't started reading the download yet). + controller.signal.addEventListener('abort', () => this.#stop(controller.signal.reason)); + } + + #stop(reason: unknown) { + this.#stopped ??= reason instanceof Error ? reason : new Error(String(reason)); + this.#grant?.(); + } + + async write(chunk: Uint8Array) { + while (!this.#credits && !this.#stopped) { + await new Promise((resolve) => (this.#grant = resolve)); + } + if (this.#stopped) { + throw this.#stopped; + } + this.#credits--; + // Transfer a copy rather than the chunk's own buffer: the chunk may be a + // view into a larger buffer the SDK still holds. + const { buffer } = chunk.slice(); + this.#port.postMessage({ type: 'chunk', chunk: buffer }, [buffer]); + } + + // The worker closes its end once it has read these; closing ours here could + // drop the message. + close() { + this.#port.postMessage({ type: 'close' }); + } + + abort(reason: unknown) { + this.#port.postMessage({ type: 'abort', reason: String(reason) }); + } +} + +async function saveAsBlob( + stream: ReadableStream, + filename: string, + options?: StreamPipeOptions +) { + const chunks: Uint8Array[] = []; + await stream.pipeTo( + new WritableStream({ write: (chunk) => void chunks.push(chunk.slice()) }), + options + ); + const url = URL.createObjectURL(new Blob(chunks, { type: 'application/octet-stream' })); + const link = document.createElement('a'); + link.href = url; + link.download = filename; + link.click(); + setTimeout(() => URL.revokeObjectURL(url), IFRAME_LINGER_MS); +} + +function withTimeout(promise: Promise | undefined): Promise { + if (!promise) { + return Promise.resolve(undefined); + } + return Promise.race([ + promise, + new Promise((resolve) => setTimeout(resolve, WORKER_TIMEOUT_MS)), + ]); +} diff --git a/web-app/src/main.tsx b/web-app/src/main.tsx index 48e229b59..6a1ccd4d5 100644 --- a/web-app/src/main.tsx +++ b/web-app/src/main.tsx @@ -1,9 +1,10 @@ import React from 'react'; import ReactDOM from 'react-dom/client'; import App from './App.js'; +import { registerDownloadWorker } from './downloadSink.js'; import './index.css'; -void navigator.serviceWorker.register('./sw.js'); +registerDownloadWorker(); ReactDOM.createRoot(document.getElementById('root') as HTMLElement).render( diff --git a/web-app/tests/playwright.moz-firefox.config.ts b/web-app/tests/playwright.moz-firefox.config.ts new file mode 100644 index 000000000..6ca01f2b1 --- /dev/null +++ b/web-app/tests/playwright.moz-firefox.config.ts @@ -0,0 +1,49 @@ +import { mkdirSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import base from './playwright.config.js'; + +/** + * Runs the suite on stock Firefox (/Applications/Firefox.app, /snap/bin/firefox) + * over WebDriver BiDi, which Playwright supports experimentally: + * + * PLAYWRIGHT_TESTS_TO_RUN=huge npx playwright test -c playwright.moz-firefox.config.ts + * + * Playwright's own patched Firefox keeps a copy of every response body in its + * parent process, so a multi-GiB download exhausts memory however the page + * produces it. Under BiDi, memory still grows with each download, because + * Playwright registers a network data collector for response bodies. Firefox + * itself streams these downloads to disk with flat memory when no automation + * is attached. So this runs huge.spec.ts on a workstation with RAM to spare, + * but not on a CI runner. + * + * Stock Firefox ignores Playwright's download folder, so point it at one we + * know; downloadPath() in tests/acts.ts looks there. + */ +const downloadDir = join(tmpdir(), 'playwright-moz-firefox-downloads'); +process.env.PLAYWRIGHT_DOWNLOAD_DIR = downloadDir; +// Workers load this file too; only the main process may clear the folder. +if (!process.env.TEST_WORKER_INDEX) { + rmSync(downloadDir, { recursive: true, force: true }); +} +mkdirSync(downloadDir, { recursive: true }); + +export default { + ...base, + projects: [ + { + name: 'moz-firefox', + use: { + browserName: '_bidiFirefox', + channel: 'moz-firefox', + launchOptions: { + firefoxUserPrefs: { + 'browser.download.dir': downloadDir, + 'browser.download.folderList': 2, + 'browser.download.useDownloadDir': true, + }, + }, + }, + }, + ], +}; diff --git a/web-app/tests/tests/acts.ts b/web-app/tests/tests/acts.ts index be9d43bf6..8753d7073 100644 --- a/web-app/tests/tests/acts.ts +++ b/web-app/tests/tests/acts.ts @@ -1,3 +1,7 @@ +import { existsSync } from 'node:fs'; +import { join } from 'node:path'; +import type { Download, Page } from '@playwright/test'; + export const appUrl = 'http://localhost:65432/'; export const authorize = async (page: Page) => { @@ -17,6 +21,24 @@ export const authorize = async (page: Page) => { await Promise.all([page.waitForResponse('**/token'), page.click('#kc-login')]); }; +/** + * Where a finished download landed. Stock Firefox (playwright.moz-firefox.config.ts) + * ignores Playwright's download folder and saves under the suggested name in + * PLAYWRIGHT_DOWNLOAD_DIR instead, so `download.path()` names a missing file. + */ +export const downloadPath = async (download: Download): Promise => { + const reported = await download.path(); + if (existsSync(reported)) { + return reported; + } + const dir = process.env.PLAYWRIGHT_DOWNLOAD_DIR; + const saved = dir && join(dir, download.suggestedFilename()); + if (!saved || !existsSync(saved)) { + throw new Error(`Download ${download.suggestedFilename()} is not at ${reported} or ${saved}`); + } + return saved; +}; + export const loadFile = async (page: Page, path: string) => { await page.locator('#fileSelector').setInputFiles(path); }; diff --git a/web-app/tests/tests/huge.spec.ts b/web-app/tests/tests/huge.spec.ts index 39d47c170..fd3a1b6c9 100644 --- a/web-app/tests/tests/huge.spec.ts +++ b/web-app/tests/tests/huge.spec.ts @@ -1,7 +1,7 @@ import type { Page } from '@playwright/test'; import { test, expect } from '../fixtures.js'; import fs from 'node:fs'; -import { appUrl, authorize, loadFile } from './acts.js'; +import { appUrl, authorize, downloadPath, loadFile } from './acts.js'; test.beforeEach(async ({ page }) => { page.on('pageerror', (err) => { @@ -12,11 +12,14 @@ test.beforeEach(async ({ page }) => { }); }); -// Chromium only for now. The file sink uses StreamSaver, which on WebKit -// buffers the whole output as a Blob, and Playwright's WebKit cancels -// multi-GiB downloads made that way. Firefox passes at 3 GiB but took the CI -// runner down at 4 GiB; restoring it is tracked in DSPX-5084. -test.skip(({ browserName }) => browserName !== 'chromium', 'Chromium only; see DSPX-5084'); +// Playwright's patched Firefox keeps a copy of every response body, downloads +// included, in its parent process, so these sizes exhaust memory (DSPX-5084). +// Run Firefox with playwright.moz-firefox.config.ts instead, which drives +// stock Firefox. +test.skip( + ({ browserName }) => browserName === 'firefox', + "Playwright's Firefox buffers downloads in memory; use playwright.moz-firefox.config.ts" +); const GiB = 2 ** 30; @@ -50,7 +53,7 @@ for (const { name, bytes } of sizes) { await page.locator('#encryptButton').click(); const download = await downloadPromise; - const cipherTextPath = await download.path(); + const cipherTextPath = await downloadPath(download); try { expect(download.suggestedFilename()).toContain('bytes'); expect(cipherTextPath).toBeTruthy(); @@ -68,7 +71,7 @@ for (const { name, bytes } of sizes) { expect(decryptTuningLogs).toEqual([ 'Using decrypt read tuning {"segmentBatchSize":2,"maxConcurrentSegmentBatches":1}', ]); - const plainTextPath = await download2.path(); + const plainTextPath = await downloadPath(download2); if (!plainTextPath) { throw new Error(); } diff --git a/web-app/tests/tests/roundtrip.spec.ts b/web-app/tests/tests/roundtrip.spec.ts index be75fc4e3..7ce16e158 100644 --- a/web-app/tests/tests/roundtrip.spec.ts +++ b/web-app/tests/tests/roundtrip.spec.ts @@ -2,7 +2,7 @@ import { test, expect } from '../fixtures.js'; import fs from 'node:fs'; import { readFile } from 'node:fs/promises'; import { serve } from '../static-server.js'; -import { authorize, loadFile } from './acts.js'; +import { authorize, downloadPath, loadFile } from './acts.js'; // References // Playwright assertions: https://playwright.dev/docs/test-assertions @@ -36,7 +36,7 @@ test('roundtrip ztdf', async ({ page }) => { const download = await downloadPromise; // Encrypt tags the container with the default wrap algorithm. expect(download.suggestedFilename()).toContain('README.md-ecsecp256r1'); - const cipherTextPath = await download.path(); + const cipherTextPath = await downloadPath(download); expect(cipherTextPath).toBeTruthy(); if (!cipherTextPath) { throw new Error(); @@ -50,7 +50,7 @@ test('roundtrip ztdf', async ({ page }) => { await page.locator('#decryptButton').click(); const download2 = await plainDownloadPromise; expect(download2.suggestedFilename()).toContain('.decrypted'); - const plainTextPath = await download2.path(); + const plainTextPath = await downloadPath(download2); if (!plainTextPath) { throw new Error(); } @@ -84,7 +84,7 @@ for (const algorithm of ['mlkem:768', 'mlkem:1024'] as const) { // The wrap qualifier is the algorithm token without its colon, which for // ML-KEM is also the KAS kid. expect(download.suggestedFilename()).toContain(`README.md-${expectedKid}`); - const cipherTextPath = await download.path(); + const cipherTextPath = await downloadPath(download); expect(cipherTextPath).toBeTruthy(); if (!cipherTextPath) { throw new Error(); @@ -108,7 +108,7 @@ for (const algorithm of ['mlkem:768', 'mlkem:1024'] as const) { await page.locator('#decryptButton').click(); const download2 = await plainDownloadPromise; expect(download2.suggestedFilename()).toContain('.decrypted'); - const plainTextPath = await download2.path(); + const plainTextPath = await downloadPath(download2); if (!plainTextPath) { throw new Error(); } @@ -135,7 +135,7 @@ test('download names record both key wrap legs', async ({ page }) => { await page.locator('#encryptButton').click(); const download = await downloadPromise; expect(download.suggestedFilename()).toBe('README.md-mlkem768.tdf'); - const cipherTextPath = await download.path(); + const cipherTextPath = await downloadPath(download); if (!cipherTextPath) { throw new Error(); } @@ -160,7 +160,7 @@ test('download names record both key wrap legs', async ({ page }) => { // The download event fires when the stream opens, not when it finishes, so the // name alone would still be asserted had the mlkem:768 -> mlkem:1024 rewrap // failed partway. Read the bytes back to pin that the exchange completed. - const plainTextPath = await download2.path(); + const plainTextPath = await downloadPath(download2); if (!plainTextPath) { throw new Error(); } @@ -199,7 +199,7 @@ test('Remote Source Streaming', async ({ page }) => { await page.locator('#encryptButton').click(); const download = await downloadPromise; - const cipherTextPath = await download.path(); + const cipherTextPath = await downloadPath(download); expect(download.suggestedFilename()).toContain('README.md-ecsecp256r1'); expect(cipherTextPath).toBeTruthy(); if (!cipherTextPath) { @@ -217,7 +217,7 @@ test('Remote Source Streaming', async ({ page }) => { await page.locator('#fileSink').click(); await page.locator('#decryptButton').click(); const download2 = await plainDownloadPromise; - const plainTextPath = await download2.path(); + const plainTextPath = await downloadPath(download2); if (!plainTextPath) { throw new Error(); } diff --git a/web-app/tests/tests/sink.spec.ts b/web-app/tests/tests/sink.spec.ts new file mode 100644 index 000000000..efb47aa36 --- /dev/null +++ b/web-app/tests/tests/sink.spec.ts @@ -0,0 +1,155 @@ +import { execFile } from 'node:child_process'; +import fs from 'node:fs'; +import { promisify } from 'node:util'; +import type { Page } from '@playwright/test'; +import { test, expect } from '../fixtures.js'; +import { appUrl, downloadPath } from './acts.js'; + +// Exercises web-app/src/downloadSink.ts on its own, with no platform: the page +// imports the module from the dev server and streams generated bytes through +// it. Set SINK_TEST_BYTES to try a large file, and SINK_TEST_MIBPS to pace the +// source so the download outlives the browser's service worker timeouts. For +// example, 12 GiB at 32 MiB/s takes about six and a half minutes: +// +// SINK_TEST_BYTES=12884901888 SINK_TEST_MIBPS=32 PLAYWRIGHT_TESTS_TO_RUN=sink \ +// npx playwright test --project firefox + +const largeBytes = Number(process.env.SINK_TEST_BYTES ?? 0); +const mibPerSecond = Number(process.env.SINK_TEST_MIBPS ?? 0); +const sinkModule = `${appUrl}src/downloadSink.ts`; +// Byte at offset n is n % PERIOD, so any dropped, repeated or reordered chunk +// shows up as a mismatch. +const PERIOD = 251; + +async function streamDownload(page: Page, bytes: number, abortAfter?: number, mibps = 0) { + return page.evaluate( + async ({ sinkModule, bytes, abortAfter, mibps, PERIOD }) => { + const { toFile } = await import(sinkModule); + const chunkSize = 2 ** 20; + const pattern = new Uint8Array(chunkSize + PERIOD).map((_, i) => i % PERIOD); + const abort = new AbortController(); + let offset = 0; + const started = performance.now(); + const source = new ReadableStream({ + async pull(controller) { + if (mibps) { + const due = started + (offset / 2 ** 20 / mibps) * 1000; + await new Promise((resolve) => setTimeout(resolve, due - performance.now())); + } + if (abortAfter !== undefined && offset >= abortAfter) { + abort.abort(new Error('test abort')); + } + const n = Math.min(chunkSize, bytes - offset); + if (n <= 0) { + controller.close(); + return; + } + const start = offset % PERIOD; + controller.enqueue(pattern.subarray(start, start + n)); + offset += n; + }, + }); + try { + return await toFile(source, `sink-${bytes}.bin`, { signal: abort.signal }); + } catch (e) { + return `rejected: ${e instanceof Error ? e.message : String(e)}`; + } + }, + { sinkModule, bytes, abortAfter, mibps, PERIOD } + ); +} + +async function expectPattern(path: string, bytes: number) { + let offset = 0; + for await (const chunk of fs.createReadStream(path) as AsyncIterable) { + for (let i = 0; i < chunk.length; i++) { + if (chunk[i] !== (offset + i) % PERIOD) { + throw new Error(`byte ${offset + i} is ${chunk[i]}, expected ${(offset + i) % PERIOD}`); + } + } + offset += chunk.length; + } + expect(offset).toBe(bytes); +} + +/** + * Resident memory of this project's browser processes, sampled once a second. + * Only meaningful when one project runs at a time. For stock Firefox only the + * parent process is counted: its content processes can't be told apart from + * those of a Firefox the user has open. The parent is where downloads are + * written, though. + */ +async function sampleBrowserMemory(browserName: string) { + const ours = + browserName === '_bidiFirefox' + ? (line: string) => line.includes('playwright__bidiFirefox') + : (line: string) => + line.includes('ms-playwright') && line.toLowerCase().includes(browserName); + const sample = async () => { + const { stdout } = await promisify(execFile)('ps', ['-ax', '-o', 'rss=,command=']); + return stdout + .split('\n') + .filter(ours) + .reduce((sum, line) => sum + parseInt(line), 0); + }; + const baselineKiB = await sample(); + let peakKiB = baselineKiB; + const timer = setInterval(async () => { + peakKiB = Math.max(peakKiB, await sample()); + }, 1000); + return () => { + clearInterval(timer); + return { baselineKiB, peakKiB }; + }; +} + +const gib = (kib: number) => `${(kib / 2 ** 20).toFixed(2)} GiB`; + +test.beforeEach(async ({ page }) => { + page.on('pageerror', (err) => console.error(err)); + await page.goto(appUrl); +}); + +test('streams a download through the service worker', async ({ page }) => { + const bytes = 3 * 2 ** 20 + 17; + const downloadPromise = page.waitForEvent('download'); + const strategy = await streamDownload(page, bytes); + const download = await downloadPromise; + expect(download.suggestedFilename()).toBe(`sink-${bytes}.bin`); + const path = await downloadPath(download); + await expectPattern(path, bytes); + expect(strategy).toBe('service-worker'); +}); + +test('an abort fails the download instead of truncating it', async ({ page, browserName }) => { + // Firefox does error the download (the worker logs it), but Playwright's + // Firefox never settles download.failure() for it. + test.skip(browserName === 'firefox', 'Playwright does not report the failed download'); + const downloadPromise = page.waitForEvent('download'); + const result = await streamDownload(page, 64 * 2 ** 20, 8 * 2 ** 20); + expect(result).toMatch(/^rejected/); + const download = await downloadPromise; + expect(await download.failure()).not.toBeNull(); +}); + +test('large download keeps memory flat', async ({ page, browserName }) => { + test.skip(!largeBytes, 'set SINK_TEST_BYTES to run'); + test.setTimeout(0); + const stopSampling = await sampleBrowserMemory(browserName); + const started = Date.now(); + const downloadPromise = page.waitForEvent('download'); + const strategy = await streamDownload(page, largeBytes, undefined, mibPerSecond); + const download = await downloadPromise; + const path = await downloadPath(download); + const { baselineKiB, peakKiB } = stopSampling(); + try { + expect(fs.statSync(path).size).toBe(largeBytes); + } finally { + fs.unlinkSync(path); + } + console.log( + `[${browserName}] ${largeBytes.toLocaleString()} bytes via ${strategy} in ` + + `${((Date.now() - started) / 1000).toFixed(0)} s; ` + + `browser RSS ${gib(baselineKiB)} before, ${gib(peakKiB)} peak` + ); +}); From b697c3fefcb1929cbf1ef5b437d1859c55c9fe44 Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Tue, 20 Oct 2026 11:07:16 -0400 Subject: [PATCH 02/10] fix(main): fail streaming downloads that never start or stall (DSPX-159) The worker's stream fills its 4 MiB queue as soon as it is registered, so the page could pipe a small output to completion and report success even when the iframe's request never reached the worker (a frame-src CSP, an extension, DevTools "Bypass for network"). A larger output waited forever for credit. - The worker now posts 'started' when the download request arrives, and 'done' once the browser has read the last byte. The page fails the pipe if 'started' doesn't arrive within 10 s, and close() resolves only on 'done'. - After the download starts, a write or close that hears nothing for 10 minutes fails as stalled. That covers a terminated worker and Chromium's unreported download-UI cancel, while allowing a paused download time to resume (DSPX-5258). - Keepalive pings go to the worker holding the stream, not to registration.active, which an update can replace. - A late 'ready' is aborted so the worker frees its stream, and a failed registration falls back at once instead of after 5 s. - toFile() takes onFallback(reason); the app warns before buffering a whole output in memory. Tests: - downloadPath() asserts the service worker's __download__/ URL, so a broken worker fails roundtrip and huge instead of falling back silently. On stock Firefox it moves each found file aside so a later download of the same name isn't renamed, and that config runs one worker. - sink.spec.ts waits for the worker before each test, and covers a CSP-blocked download below and above the worker's queue, and the Blob fallback. Comments are updated to match, and corrected where review found them inaccurate. Signed-off-by: Dave Mihalcik --- web-app/public/sw.js | 80 ++++-- web-app/src/App.tsx | 19 +- web-app/src/downloadSink.ts | 229 +++++++++++++----- .../tests/playwright.moz-firefox.config.ts | 4 +- web-app/tests/tests/acts.ts | 36 ++- web-app/tests/tests/huge.spec.ts | 5 +- web-app/tests/tests/sink.spec.ts | 50 +++- 7 files changed, 327 insertions(+), 96 deletions(-) diff --git a/web-app/public/sw.js b/web-app/public/sw.js index 9e8a9412b..810f20583 100644 --- a/web-app/public/sw.js +++ b/web-app/public/sw.js @@ -1,18 +1,23 @@ // Streams downloads from the page straight into the browser's download manager, // so a multi-GiB encrypt or decrypt never has to fit in memory or in storage -// quota. The page side, and the message protocol, live in src/downloadSink.ts. +// quota. The page side lives in src/downloadSink.ts; spec/DSPX-159.md lists +// every message. // // 1. The page posts { type: 'register', id, filename } with a MessagePort. // 2. We wrap the port in a ReadableStream and reply { type: 'ready' }. -// 3. The page points a hidden iframe at __download__//, -// and we answer that navigation with the stream as an attachment. +// 3. The page points a hidden iframe at __download__//. +// We answer that navigation with the stream as an attachment and post +// { type: 'started' }. // 4. Each time the stream wants more, we post { type: 'pull' }; the page sends -// one { type: 'chunk' } per pull, so backpressure from the download manager -// reaches all the way back to the SDK. +// one { type: 'chunk' } per pull. The stream fills its HIGH_WATER_MARK_BYTES +// queue as soon as it is registered; after that, pulls follow the download +// manager's reads, so backpressure reaches all the way back to the SDK. +// 5. After the page posts { type: 'close' } and the browser has read the last +// byte, we post { type: 'done' }. const HIGH_WATER_MARK_BYTES = 4 * 2 ** 20; -/** @type {Map, filename: string }>} */ +/** @type {Map, source: PortSource, filename: string }>} */ const pending = new Map(); function downloadBase() { @@ -38,16 +43,18 @@ class PortSource { controller.enqueue(new Uint8Array(data.chunk)); break; case 'close': + // Keep the port open: the page waits for 'done'. controller.close(); - this.port.close(); break; case 'abort': // Erroring the body makes the browser mark the download failed, - // rather than leaving a truncated file that looks complete. - controller.error(new Error(data.reason)); - this.port.close(); - this.forget(); + // rather than leaving a truncated file that looks complete. This + // also arrives after 'close' if the download never started. + controller.error(new Error(String(data.reason ?? 'aborted'))); + this.end(); break; + default: + console.warn(`download worker: unknown message type ${data?.type}`); } }; } @@ -58,14 +65,45 @@ class PortSource { /** @param {unknown} reason */ cancel(reason) { - // The user cancelled in the download UI. Firefox reports this; Chromium - // does not (https://crbug.com/638494), so there the page finds out only - // when it aborts on its own. - this.port.postMessage({ type: 'cancel', reason: String(reason) }); + // The user cancelled in the download UI. Firefox is expected to report + // this (not yet verified); Chromium does not (https://crbug.com/638494), + // so there the page notices only when its stall watchdog fires. + this.port.postMessage({ type: 'cancel', reason: String(reason ?? 'no reason given') }); + this.end(); + } + + end() { this.port.close(); + this.forget(); } } +/** + * Passes the body through, posting { type: 'done' } once the browser has read + * all of it, so the page does not report success while bytes are still queued. + * @param {ReadableStream} stream + * @param {PortSource} source + */ +function reportWhenRead(stream, source) { + const reader = stream.getReader(); + return new ReadableStream( + { + async pull(controller) { + const { done, value } = await reader.read(); + if (done) { + controller.close(); + source.port.postMessage({ type: 'done' }); + source.end(); + } else { + controller.enqueue(value); + } + }, + cancel: (reason) => reader.cancel(reason), + }, + { highWaterMark: 0 } + ); +} + /** @param {string} filename */ function contentDisposition(filename) { const safe = filename.replace(/[\u0000-\u001f\u007f/\\]/g, '_'); @@ -77,8 +115,8 @@ function contentDisposition(filename) { return `attachment; filename="${ascii}"; filename*=UTF-8''${encoded}`; } -self.addEventListener('install', () => { - self.skipWaiting(); +self.addEventListener('install', (event) => { + event.waitUntil(self.skipWaiting()); }); self.addEventListener('activate', (event) => { @@ -101,11 +139,12 @@ self.addEventListener('message', (event) => { return; } const { id, filename } = data; + const source = new PortSource(port, () => pending.delete(id)); const stream = new ReadableStream( - new PortSource(port, () => pending.delete(id)), + source, new ByteLengthQueuingStrategy({ highWaterMark: HIGH_WATER_MARK_BYTES }) ); - pending.set(id, { stream, filename }); + pending.set(id, { stream, source, filename }); port.postMessage({ type: 'ready' }); }); @@ -123,8 +162,9 @@ self.addEventListener('fetch', (event) => { } // Single use: a second request for the same id gets a 404. pending.delete(id); + entry.source.port.postMessage({ type: 'started' }); event.respondWith( - new Response(entry.stream, { + new Response(reportWhenRead(entry.stream, entry.source), { headers: { 'Cache-Control': 'no-store', 'Content-Disposition': contentDisposition(entry.filename), diff --git a/web-app/src/App.tsx b/web-app/src/App.tsx index 940b1b392..1c63e996b 100644 --- a/web-app/src/App.tsx +++ b/web-app/src/App.tsx @@ -366,6 +366,14 @@ function App() { }; }; + // Appends, so it doesn't hide a wrapping-algorithm warning from the same run. + const warnBuffering = (reason: string) => { + const warning = + `Streaming download unavailable (${reason}), so the whole output is held in memory; ` + + 'files larger than available memory will fail.'; + setAlgorithmWarning((previous) => (previous ? `${previous} ${warning}` : warning)); + }; + const handleEncrypt = async () => { if (!inputSource) { console.warn('No input source selected'); @@ -481,11 +489,14 @@ function App() { try { switch (sinkType) { case 'file': - await toFile(cipherTextWithProgress, downloadName, { signal: sc.signal }); + await toFile(cipherTextWithProgress, downloadName, { + signal: sc.signal, + onFallback: warnBuffering, + }); break; case 'fsapi': { if (!f) { - throw new Error(); + throw new Error('No file handle from the Save As picker'); } const writable = await f.createWritable(); await cipherTextWithProgress.pipeTo(writable, { signal: sc.signal }); @@ -576,11 +587,11 @@ function App() { .pipeThrough(progressTransformers.writer); switch (sinkType) { case 'file': - await toFile(plainTextStream, dfn, { signal: sc.signal }); + await toFile(plainTextStream, dfn, { signal: sc.signal, onFallback: warnBuffering }); break; case 'fsapi': { if (!f) { - throw new Error(); + throw new Error('No file handle from the Save As picker'); } const writable = await f.createWritable(); await plainTextStream.pipeTo(writable, { signal: sc.signal }); diff --git a/web-app/src/downloadSink.ts b/web-app/src/downloadSink.ts index 3570eca5d..788f6237c 100644 --- a/web-app/src/downloadSink.ts +++ b/web-app/src/downloadSink.ts @@ -11,23 +11,54 @@ export type SinkStrategy = 'service-worker' | 'blob'; +export type ToFileOptions = StreamPipeOptions & { + /** + * Called with the reason before falling back to buffering the whole output + * in memory, so the caller can warn that large files may not fit. + */ + onFallback?: (reason: string) => void; +}; + // Browsers stop a service worker that has had no events for about 30 seconds. // Chunks arrive on a MessagePort, which may not count, so ping while streaming. const KEEPALIVE_MS = 10_000; // How long to wait for a worker before falling back to an in-memory Blob. const WORKER_TIMEOUT_MS = 5_000; -// The iframe only exists to start the download; by the time the stream has -// closed the browser owns it, but leave the frame a moment to be safe. -const IFRAME_LINGER_MS = 30_000; +// How long the iframe's request may take to reach the worker. If it never does +// (a frame-src Content-Security-Policy, an extension, DevTools "Bypass for +// network"), there is no download, and we must fail rather than report success. +const START_TIMEOUT_MS = 10_000; +// Once the download has started, how long the browser may go without reading +// before we give up. Chromium does not tell the worker when the user cancels +// in its download UI, and a terminated worker sends nothing, so silence is all +// there is to go on. Generous, because a download paused in Chromium's download +// UI looks the same, as does a slow disk or a save dialog (DSPX-5258). +const STALL_TIMEOUT_MS = 10 * 60_000; +// The iframe and the Blob URL only have to start the download, after which the +// browser owns it; wait a moment before removing them to be safe. +const CLEANUP_DELAY_MS = 30_000; + +type FromWorker = + | { type: 'ready' } + | { type: 'started' } + | { type: 'pull' } + | { type: 'done' } + | { type: 'cancel'; reason: string }; + +type ToWorker = + { type: 'chunk'; chunk: ArrayBuffer } | { type: 'close' } | { type: 'abort'; reason: string }; -type WorkerMessage = { type: 'ready' } | { type: 'pull' } | { type: 'cancel'; reason?: string }; +let registered: Promise | undefined; export function registerDownloadWorker(): void { if (!('serviceWorker' in navigator)) { console.warn('No service workers here (insecure context?); downloads will buffer in memory'); return; } - navigator.serviceWorker.register('./sw.js').catch((e: unknown) => { + registered = navigator.serviceWorker + .register('./sw.js') + .then(() => navigator.serviceWorker.ready); + registered.catch((e: unknown) => { console.warn('Download service worker registration failed', e); }); } @@ -39,60 +70,92 @@ export function canSaveAs(): boolean { export async function toFile( stream: ReadableStream, filename = 'download.tdf', - options?: StreamPipeOptions + { onFallback, ...options }: ToFileOptions = {} ): Promise { - const registration = await withTimeout(navigator.serviceWorker?.ready); - if (registration?.active) { + const found = await findWorker(); + let reason: string; + if (typeof found === 'string') { + reason = found; + } else { const sink = new PortSink(); - registration.active.postMessage({ type: 'register', id: sink.id, filename }, [sink.remotePort]); + found.worker.postMessage({ type: 'register', id: sink.id, filename }, [sink.remotePort]); // An outdated or broken worker may never answer; the stream is untouched // until it does, so falling back is still possible. if (await withTimeout(sink.ready)) { - await streamThroughWorker(registration, sink, stream, filename, options); + await streamThroughWorker(found.worker, found.scope, sink, stream, filename, options); return 'service-worker'; } + // Frees whatever the worker sets up if it answers after all. + sink.abort('the page stopped waiting for the worker'); + reason = 'the service worker did not answer (an outdated sw.js?)'; } - console.warn('Download service worker unavailable; buffering the whole output in memory'); + console.warn(`Streaming download unavailable (${reason}); buffering the whole output in memory`); + onFallback?.(reason); await saveAsBlob(stream, filename, options); return 'blob'; } +/** The active worker, or why there isn't one. */ +async function findWorker(): Promise<{ worker: ServiceWorker; scope: string } | string> { + if (!('serviceWorker' in navigator)) { + return 'no service workers here (insecure context?)'; + } + let registration: ServiceWorkerRegistration | undefined; + try { + // Pages that never called registerDownloadWorker may still have a worker. + registration = await withTimeout(registered ?? navigator.serviceWorker.ready); + } catch (e) { + return `service worker registration failed: ${e instanceof Error ? e.message : String(e)}`; + } + if (!registration?.active) { + return `no service worker became active within ${WORKER_TIMEOUT_MS / 1000} s`; + } + return { worker: registration.active, scope: registration.scope }; +} + async function streamThroughWorker( - registration: ServiceWorkerRegistration, + worker: ServiceWorker, + scope: string, sink: PortSink, stream: ReadableStream, filename: string, - options?: StreamPipeOptions + options: StreamPipeOptions ) { const iframe = document.createElement('iframe'); iframe.hidden = true; - iframe.src = new URL( - `__download__/${sink.id}/${encodeURIComponent(filename)}`, - registration.scope - ).href; + iframe.src = new URL(`__download__/${sink.id}/${encodeURIComponent(filename)}`, scope).href; document.body.appendChild(iframe); - const keepalive = setInterval( - () => registration.active?.postMessage({ type: 'ping' }), - KEEPALIVE_MS - ); + sink.expectStart(); + // Ping the worker holding the stream, not registration.active, which an + // update can replace mid-download. + const keepalive = setInterval(() => worker.postMessage({ type: 'ping' }), KEEPALIVE_MS); try { await stream.pipeTo(new WritableStream(sink, { highWaterMark: 1 }), options); } finally { clearInterval(keepalive); - setTimeout(() => iframe.remove(), IFRAME_LINGER_MS); + sink.dispose(); + setTimeout(() => iframe.remove(), CLEANUP_DELAY_MS); } } -/** Sends one chunk per credit; the worker grants a credit each time its stream pulls. */ +/** + * Sends one chunk per credit; the worker grants a credit each time its stream + * pulls. Closing succeeds only once the browser has read every byte, and the + * pipe fails if the download never reaches the worker or stops being read. + */ class PortSink implements UnderlyingSink { readonly id = crypto.randomUUID(); readonly remotePort: MessagePort; readonly ready: Promise; #port: MessagePort; #credits = 0; - #grant?: () => void; - // Set when the download is cancelled or the stream aborted; wakes a write - // that is waiting for a credit. + #started = false; + #done = false; + #startTimer?: ReturnType; + // Wakes a write or close waiting for the next message from the worker. + #wake?: () => void; + // Set when the download is cancelled, stalls or the stream aborts; wakes a + // waiting write or close so it can throw. #stopped?: Error; #controller?: WritableStreamDefaultController; @@ -102,20 +165,27 @@ class PortSink implements UnderlyingSink { this.remotePort = port2; let markReady: (ok: true) => void; this.ready = new Promise((resolve) => (markReady = resolve)); - port1.onmessage = ({ data }: MessageEvent) => { + port1.onmessage = ({ data }: MessageEvent) => { switch (data.type) { case 'ready': markReady(true); break; + case 'started': + this.#started = true; + clearTimeout(this.#startTimer); + break; case 'pull': this.#credits++; - this.#grant?.(); + break; + case 'done': + this.#done = true; break; case 'cancel': - this.#stop(new Error(`Download cancelled: ${data.reason ?? 'no reason given'}`)); - this.#controller?.error(this.#stopped); + // The worker has already closed its end. + this.#fail(new Error(`Download cancelled: ${data.reason}`), false); break; } + this.#wake?.(); }; } @@ -123,44 +193,97 @@ class PortSink implements UnderlyingSink { this.#controller = controller; // abort() is only called once the in-flight write settles, and that write // may be waiting for a credit the worker will never send (for example, - // because the browser hasn't started reading the download yet). + // because its queue is full and the browser hasn't started reading). controller.signal.addEventListener('abort', () => this.#stop(controller.signal.reason)); } - #stop(reason: unknown) { - this.#stopped ??= reason instanceof Error ? reason : new Error(String(reason)); - this.#grant?.(); + /** Call once the iframe points at the download URL. */ + expectStart() { + this.#startTimer = setTimeout( + () => + this.#fail( + new Error( + 'The download never reached the service worker; a Content-Security-Policy ' + + 'frame-src, a browser extension or DevTools "Bypass for network" may be blocking it' + ) + ), + START_TIMEOUT_MS + ); + } + + dispose() { + clearTimeout(this.#startTimer); } async write(chunk: Uint8Array) { - while (!this.#credits && !this.#stopped) { - await new Promise((resolve) => (this.#grant = resolve)); - } - if (this.#stopped) { - throw this.#stopped; - } + await this.#until(() => this.#credits > 0); this.#credits--; // Transfer a copy rather than the chunk's own buffer: the chunk may be a // view into a larger buffer the SDK still holds. const { buffer } = chunk.slice(); - this.#port.postMessage({ type: 'chunk', chunk: buffer }, [buffer]); + this.#send({ type: 'chunk', chunk: buffer }, [buffer]); } - // The worker closes its end once it has read these; closing ours here could - // drop the message. - close() { - this.#port.postMessage({ type: 'close' }); + async close() { + this.#send({ type: 'close' }); + // The bytes are sent, but up to a queue's worth may not be saved yet. + await this.#until(() => this.#done); } abort(reason: unknown) { - this.#port.postMessage({ type: 'abort', reason: String(reason) }); + this.#send({ type: 'abort', reason: String(reason) }); + } + + #send(message: ToWorker, transfer: Transferable[] = []) { + this.#port.postMessage(message, transfer); + } + + // Waits until `ready()` holds, rechecking after each message from the worker. + async #until(ready: () => boolean) { + while (!ready() && !this.#stopped) { + // Until the download starts, the start timer is the deadline. + const stall = this.#started + ? setTimeout( + () => + this.#fail( + new Error( + `Download stalled: the browser read nothing for ${STALL_TIMEOUT_MS / 60_000} minutes. ` + + 'Was it cancelled in the downloads list?' + ) + ), + STALL_TIMEOUT_MS + ) + : undefined; + await new Promise((resolve) => (this.#wake = resolve)); + clearTimeout(stall); + } + if (this.#stopped) { + throw this.#stopped; + } + } + + #stop(reason: unknown) { + this.#stopped ??= reason instanceof Error ? reason : new Error(String(reason)); + this.#wake?.(); + } + + // For failures the writable stream doesn't know about yet. + #fail(error: Error, tellWorker = true) { + if (this.#stopped) { + return; + } + this.#stop(error); + this.#controller?.error(error); + if (tellWorker) { + this.abort(error.message); + } } } async function saveAsBlob( stream: ReadableStream, filename: string, - options?: StreamPipeOptions + options: StreamPipeOptions ) { const chunks: Uint8Array[] = []; await stream.pipeTo( @@ -172,15 +295,13 @@ async function saveAsBlob( link.href = url; link.download = filename; link.click(); - setTimeout(() => URL.revokeObjectURL(url), IFRAME_LINGER_MS); + setTimeout(() => URL.revokeObjectURL(url), CLEANUP_DELAY_MS); } -function withTimeout(promise: Promise | undefined): Promise { - if (!promise) { - return Promise.resolve(undefined); - } +function withTimeout(promise: Promise): Promise { + let timer: number | undefined; return Promise.race([ promise, - new Promise((resolve) => setTimeout(resolve, WORKER_TIMEOUT_MS)), - ]); + new Promise((resolve) => (timer = window.setTimeout(resolve, WORKER_TIMEOUT_MS))), + ]).finally(() => clearTimeout(timer)); } diff --git a/web-app/tests/playwright.moz-firefox.config.ts b/web-app/tests/playwright.moz-firefox.config.ts index 6ca01f2b1..72927614e 100644 --- a/web-app/tests/playwright.moz-firefox.config.ts +++ b/web-app/tests/playwright.moz-firefox.config.ts @@ -18,7 +18,8 @@ import base from './playwright.config.js'; * but not on a CI runner. * * Stock Firefox ignores Playwright's download folder, so point it at one we - * know; downloadPath() in tests/acts.ts looks there. + * know; downloadPath() in tests/acts.ts looks there. Downloads are found by + * name, so run one test at a time. */ const downloadDir = join(tmpdir(), 'playwright-moz-firefox-downloads'); process.env.PLAYWRIGHT_DOWNLOAD_DIR = downloadDir; @@ -30,6 +31,7 @@ mkdirSync(downloadDir, { recursive: true }); export default { ...base, + workers: 1, projects: [ { name: 'moz-firefox', diff --git a/web-app/tests/tests/acts.ts b/web-app/tests/tests/acts.ts index 8753d7073..bf2cf0e5e 100644 --- a/web-app/tests/tests/acts.ts +++ b/web-app/tests/tests/acts.ts @@ -1,6 +1,7 @@ -import { existsSync } from 'node:fs'; +import { randomUUID } from 'node:crypto'; +import { existsSync, mkdirSync, renameSync } from 'node:fs'; import { join } from 'node:path'; -import type { Download, Page } from '@playwright/test'; +import { expect, type Download, type Page } from '@playwright/test'; export const appUrl = 'http://localhost:65432/'; @@ -22,21 +23,36 @@ export const authorize = async (page: Page) => { }; /** - * Where a finished download landed. Stock Firefox (playwright.moz-firefox.config.ts) - * ignores Playwright's download folder and saves under the suggested name in - * PLAYWRIGHT_DOWNLOAD_DIR instead, so `download.path()` names a missing file. + * Where a finished download landed, after checking which way it was saved. The + * in-memory Blob fallback writes the same bytes as the service worker, so + * without this check a broken worker would go unnoticed. + * + * Stock Firefox (playwright.moz-firefox.config.ts) ignores Playwright's download + * folder and saves under the suggested name in PLAYWRIGHT_DOWNLOAD_DIR, so + * `download.path()` names a missing file. It would save a later download with + * the same name as `name(1).ext`, so each file is moved into a folder of its + * own once found. That relies on one download at a time, so that config runs a + * single worker. */ -export const downloadPath = async (download: Download): Promise => { +export const downloadPath = async ( + download: Download, + via: 'service-worker' | 'blob' = 'service-worker' +): Promise => { + expect(download.url()).toContain(via === 'blob' ? 'blob:' : '/__download__/'); const reported = await download.path(); if (existsSync(reported)) { return reported; } const dir = process.env.PLAYWRIGHT_DOWNLOAD_DIR; - const saved = dir && join(dir, download.suggestedFilename()); - if (!saved || !existsSync(saved)) { - throw new Error(`Download ${download.suggestedFilename()} is not at ${reported} or ${saved}`); + const name = download.suggestedFilename(); + const saved = dir && join(dir, name); + if (!dir || !saved || !existsSync(saved)) { + throw new Error(`Download ${name} is not at ${reported} or ${saved}`); } - return saved; + const claimed = join(dir, 'claimed', randomUUID()); + mkdirSync(claimed, { recursive: true }); + renameSync(saved, join(claimed, name)); + return join(claimed, name); }; export const loadFile = async (page: Page, path: string) => { diff --git a/web-app/tests/tests/huge.spec.ts b/web-app/tests/tests/huge.spec.ts index fd3a1b6c9..abac23671 100644 --- a/web-app/tests/tests/huge.spec.ts +++ b/web-app/tests/tests/huge.spec.ts @@ -14,8 +14,9 @@ test.beforeEach(async ({ page }) => { // Playwright's patched Firefox keeps a copy of every response body, downloads // included, in its parent process, so these sizes exhaust memory (DSPX-5084). -// Run Firefox with playwright.moz-firefox.config.ts instead, which drives -// stock Firefox. +// playwright.moz-firefox.config.ts drives stock Firefox instead, but its BiDi +// channel buffers too, so use it only on a workstation with RAM to spare. +// WebKit's memory can also grow when the source outruns the disk (DSPX-5246). test.skip( ({ browserName }) => browserName === 'firefox', "Playwright's Firefox buffers downloads in memory; use playwright.moz-firefox.config.ts" diff --git a/web-app/tests/tests/sink.spec.ts b/web-app/tests/tests/sink.spec.ts index efb47aa36..ea49a29a1 100644 --- a/web-app/tests/tests/sink.spec.ts +++ b/web-app/tests/tests/sink.spec.ts @@ -12,7 +12,9 @@ import { appUrl, downloadPath } from './acts.js'; // example, 12 GiB at 32 MiB/s takes about six and a half minutes: // // SINK_TEST_BYTES=12884901888 SINK_TEST_MIBPS=32 PLAYWRIGHT_TESTS_TO_RUN=sink \ -// npx playwright test --project firefox +// npx playwright test --project chromium +// +// Not --project firefox: Playwright's Firefox keeps every download in memory. const largeBytes = Number(process.env.SINK_TEST_BYTES ?? 0); const mibPerSecond = Number(process.env.SINK_TEST_MIBPS ?? 0); @@ -105,9 +107,13 @@ async function sampleBrowserMemory(browserName: string) { const gib = (kib: number) => `${(kib / 2 ** 20).toFixed(2)} GiB`; -test.beforeEach(async ({ page }) => { +test.beforeEach(async ({ page, serviceWorkers }) => { page.on('pageerror', (err) => console.error(err)); await page.goto(appUrl); + // A cold runner can take longer to install the worker than toFile waits. + if (serviceWorkers === 'allow') { + await page.evaluate(() => navigator.serviceWorker.ready); + } }); test('streams a download through the service worker', async ({ page }) => { @@ -122,16 +128,50 @@ test('streams a download through the service worker', async ({ page }) => { }); test('an abort fails the download instead of truncating it', async ({ page, browserName }) => { - // Firefox does error the download (the worker logs it), but Playwright's - // Firefox never settles download.failure() for it. + // Firefox does error the download, but Playwright's Firefox never settles + // download.failure() for it. test.skip(browserName === 'firefox', 'Playwright does not report the failed download'); const downloadPromise = page.waitForEvent('download'); + // The worker queues 4 MiB before the browser starts reading, so aborting at + // 8 MiB happens after the download has started. const result = await streamDownload(page, 64 * 2 ** 20, 8 * 2 ** 20); - expect(result).toMatch(/^rejected/); + expect(result).toMatch(/^rejected: .*test abort/); const download = await downloadPromise; expect(await download.failure()).not.toBeNull(); }); +for (const bytes of [2 ** 20, 16 * 2 ** 20]) { + test(`a blocked download fails instead of hanging (${bytes} bytes)`, async ({ page }) => { + test.setTimeout(30_000); + // Below the worker's 4 MiB queue, every byte is accepted before the iframe + // would be read, so this once reported success with no file; above it, the + // pipe waited forever for credit. + await page.evaluate(() => { + const csp = document.createElement('meta'); + csp.httpEquiv = 'Content-Security-Policy'; + csp.content = "frame-src 'none'"; + document.head.append(csp); + }); + const result = await streamDownload(page, bytes); + expect(result).toMatch(/^rejected: The download never reached the service worker/); + }); +} + +test.describe('without a service worker', () => { + test.use({ serviceWorkers: 'block' }); + + test('falls back to an in-memory Blob', async ({ page }) => { + // toFile waits up to 5 s for a worker before giving up. + test.setTimeout(30_000); + const bytes = 2 ** 20 + 17; + const downloadPromise = page.waitForEvent('download'); + const strategy = await streamDownload(page, bytes); + const download = await downloadPromise; + await expectPattern(await downloadPath(download, 'blob'), bytes); + expect(strategy).toBe('blob'); + }); +}); + test('large download keeps memory flat', async ({ page, browserName }) => { test.skip(!largeBytes, 'set SINK_TEST_BYTES to run'); test.setTimeout(0); From 03401a8049d3eabdeab469e25eaabad1ecde670e Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Wed, 21 Oct 2026 12:34:16 -0400 Subject: [PATCH 03/10] fix(main): let Cancel end a download that is waiting to finish (DSPX-159) pipeTo stops watching its abort signal once the source has closed, and the download sink's close() now waits for the browser to read the last byte: up to 10 s if the download never started, or until the stall timeout. During that wait the app's Cancel did nothing, and the abandoned run's eventual failure overwrote the status of the next run. The sink now watches the caller's signal itself and fails the pipe, telling the worker. Signed-off-by: Dave Mihalcik --- web-app/src/downloadSink.ts | 11 ++++++ web-app/tests/tests/sink.spec.ts | 57 ++++++++++++++++++++++++-------- 2 files changed, 54 insertions(+), 14 deletions(-) diff --git a/web-app/src/downloadSink.ts b/web-app/src/downloadSink.ts index 788f6237c..c0645f3f7 100644 --- a/web-app/src/downloadSink.ts +++ b/web-app/src/downloadSink.ts @@ -129,9 +129,15 @@ async function streamThroughWorker( // Ping the worker holding the stream, not registration.active, which an // update can replace mid-download. const keepalive = setInterval(() => worker.postMessage({ type: 'ping' }), KEEPALIVE_MS); + // pipeTo stops watching its signal once the source has closed, but close() + // can then wait minutes for the browser to finish reading. + const { signal } = options; + const onAbort = () => sink.fail(signal?.reason); + signal?.addEventListener('abort', onAbort, { once: true }); try { await stream.pipeTo(new WritableStream(sink, { highWaterMark: 1 }), options); } finally { + signal?.removeEventListener('abort', onAbort); clearInterval(keepalive); sink.dispose(); setTimeout(() => iframe.remove(), CLEANUP_DELAY_MS); @@ -215,6 +221,11 @@ class PortSink implements UnderlyingSink { clearTimeout(this.#startTimer); } + /** Fails the pipe from outside it, even while close() waits for 'done'. */ + fail(reason: unknown) { + this.#fail(reason instanceof Error ? reason : new Error(String(reason))); + } + async write(chunk: Uint8Array) { await this.#until(() => this.#credits > 0); this.#credits--; diff --git a/web-app/tests/tests/sink.spec.ts b/web-app/tests/tests/sink.spec.ts index ea49a29a1..02dc536e4 100644 --- a/web-app/tests/tests/sink.spec.ts +++ b/web-app/tests/tests/sink.spec.ts @@ -23,13 +23,27 @@ const sinkModule = `${appUrl}src/downloadSink.ts`; // shows up as a mismatch. const PERIOD = 251; -async function streamDownload(page: Page, bytes: number, abortAfter?: number, mibps = 0) { +type StreamOptions = { + // Abort once this many bytes have been produced, or this long after starting. + abortAfter?: number; + abortAfterMs?: number; + mibps?: number; +}; + +async function streamDownload( + page: Page, + bytes: number, + { abortAfter, abortAfterMs, mibps = 0 }: StreamOptions = {} +) { return page.evaluate( - async ({ sinkModule, bytes, abortAfter, mibps, PERIOD }) => { + async ({ sinkModule, bytes, abortAfter, abortAfterMs, mibps, PERIOD }) => { const { toFile } = await import(sinkModule); const chunkSize = 2 ** 20; const pattern = new Uint8Array(chunkSize + PERIOD).map((_, i) => i % PERIOD); const abort = new AbortController(); + if (abortAfterMs !== undefined) { + setTimeout(() => abort.abort(new Error('test abort')), abortAfterMs); + } let offset = 0; const started = performance.now(); const source = new ReadableStream({ @@ -57,7 +71,7 @@ async function streamDownload(page: Page, bytes: number, abortAfter?: number, mi return `rejected: ${e instanceof Error ? e.message : String(e)}`; } }, - { sinkModule, bytes, abortAfter, mibps, PERIOD } + { sinkModule, bytes, abortAfter, abortAfterMs, mibps, PERIOD } ); } @@ -134,29 +148,44 @@ test('an abort fails the download instead of truncating it', async ({ page, brow const downloadPromise = page.waitForEvent('download'); // The worker queues 4 MiB before the browser starts reading, so aborting at // 8 MiB happens after the download has started. - const result = await streamDownload(page, 64 * 2 ** 20, 8 * 2 ** 20); + const result = await streamDownload(page, 64 * 2 ** 20, { abortAfter: 8 * 2 ** 20 }); expect(result).toMatch(/^rejected: .*test abort/); const download = await downloadPromise; expect(await download.failure()).not.toBeNull(); }); +// Keeps the download's iframe request from ever reaching the worker. +async function blockFrames(page: Page) { + await page.evaluate(() => { + const csp = document.createElement('meta'); + csp.httpEquiv = 'Content-Security-Policy'; + csp.content = "frame-src 'none'"; + document.head.append(csp); + }); +} + for (const bytes of [2 ** 20, 16 * 2 ** 20]) { test(`a blocked download fails instead of hanging (${bytes} bytes)`, async ({ page }) => { test.setTimeout(30_000); - // Below the worker's 4 MiB queue, every byte is accepted before the iframe - // would be read, so this once reported success with no file; above it, the - // pipe waited forever for credit. - await page.evaluate(() => { - const csp = document.createElement('meta'); - csp.httpEquiv = 'Content-Security-Policy'; - csp.content = "frame-src 'none'"; - document.head.append(csp); - }); + // Below the worker's 4 MiB queue, every byte fits in it without the + // browser reading any, so this once reported success with no file; above + // it, the pipe waited forever for credit. + await blockFrames(page); const result = await streamDownload(page, bytes); expect(result).toMatch(/^rejected: The download never reached the service worker/); }); } +test('an abort still works once the source has closed', async ({ page }) => { + // The whole source fits in the worker's queue, so the sink is already + // waiting in close(), where pipeTo no longer watches its signal. + await blockFrames(page); + const started = Date.now(); + const result = await streamDownload(page, 2 ** 20, { abortAfterMs: 500 }); + expect(result).toMatch(/^rejected: .*test abort/); + expect(Date.now() - started).toBeLessThan(5_000); +}); + test.describe('without a service worker', () => { test.use({ serviceWorkers: 'block' }); @@ -178,7 +207,7 @@ test('large download keeps memory flat', async ({ page, browserName }) => { const stopSampling = await sampleBrowserMemory(browserName); const started = Date.now(); const downloadPromise = page.waitForEvent('download'); - const strategy = await streamDownload(page, largeBytes, undefined, mibPerSecond); + const strategy = await streamDownload(page, largeBytes, { mibps: mibPerSecond }); const download = await downloadPromise; const path = await downloadPath(download); const { baselineKiB, peakKiB } = stopSampling(); From 0899ca522106a95908e4023d15fa6dd517c738c2 Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Wed, 21 Oct 2026 12:35:38 -0400 Subject: [PATCH 04/10] fix(main): report Complete only once the output is saved (DSPX-159) The progress transform's flush set "Complete" when the SDK output ended, before the sink had saved it. With a blocked download that showed "Complete" for 10 s before flipping to the failure. Flush now says the output is finishing, and each handler reports completion after its sink resolves. Signed-off-by: Dave Mihalcik --- web-app/src/App.tsx | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/web-app/src/App.tsx b/web-app/src/App.tsx index 1c63e996b..8e22d0b62 100644 --- a/web-app/src/App.tsx +++ b/web-app/src/App.tsx @@ -359,8 +359,9 @@ function App() { }, flush() { const d = Date.now() - startTime; - console.log(`✍️ ${type} Complete after ${d.toLocaleString()} milliseconds`); - setDownloadState(`✍️ ${type} Complete`); + console.log(`✍️ ${type}ed output ended after ${d.toLocaleString()} milliseconds`); + // The sink may still be saving; the handler reports completion. + setDownloadState(`✍️ ${type}ed; finishing...`); }, }), }; @@ -506,6 +507,7 @@ function App() { await cipherTextWithProgress.pipeTo(drain(), { signal: sc.signal }); break; } + setDownloadState('✍️ Encrypt Complete'); } catch (e) { setDownloadState(`Encrypt Failed: ${e instanceof Error ? e.message : String(e)}`); console.error('Encrypt Failed', e); @@ -601,6 +603,7 @@ function App() { await plainTextStream.pipeTo(drain(), { signal: sc.signal }); break; } + setDownloadState('✍️ Decrypt Complete'); const { fqns: requiredObligations } = await reader.obligations(); console.log( `Found required obligations count: ${requiredObligations.length}. ${requiredObligations.length ?? JSON.stringify(requiredObligations)}` From 335939c3376f3ead0024f743110d493272e80269 Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Wed, 21 Oct 2026 12:36:08 -0400 Subject: [PATCH 05/10] ci: run the download sink tests on pull requests (DSPX-159) sink.spec.ts needs no platform, only the dev server the roundtrip job already starts, and it is the only check that a blocked download fails and that the in-memory fallback works. Give its tests 30 s, so the beforeEach wait for a cold worker install doesn't share a 10 s budget. Signed-off-by: Dave Mihalcik --- .github/workflows/reusable_build-and-test.yaml | 2 +- web-app/tests/tests/sink.spec.ts | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/.github/workflows/reusable_build-and-test.yaml b/.github/workflows/reusable_build-and-test.yaml index 3d90c35cb..6eb739191 100644 --- a/.github/workflows/reusable_build-and-test.yaml +++ b/.github/workflows/reusable_build-and-test.yaml @@ -325,7 +325,7 @@ jobs: go env GOWORK - run: docker compose up -d --wait --wait-timeout 240 - env: - PLAYWRIGHT_TESTS_TO_RUN: roundtrip + PLAYWRIGHT_TESTS_TO_RUN: roundtrip sink run: |- ./wait-and-test.sh platform - uses: actions/upload-artifact@bbbca2ddaa5d8feaa63e36b76fdaad77386f024f #v7.0.0 diff --git a/web-app/tests/tests/sink.spec.ts b/web-app/tests/tests/sink.spec.ts index 02dc536e4..8e0253e84 100644 --- a/web-app/tests/tests/sink.spec.ts +++ b/web-app/tests/tests/sink.spec.ts @@ -121,6 +121,10 @@ async function sampleBrowserMemory(browserName: string) { const gib = (kib: number) => `${(kib / 2 ** 20).toFixed(2)} GiB`; +// The hook below can wait out a cold worker install, and several tests wait +// out toFile's 5 s worker or 10 s start timeouts. +test.describe.configure({ timeout: 30_000 }); + test.beforeEach(async ({ page, serviceWorkers }) => { page.on('pageerror', (err) => console.error(err)); await page.goto(appUrl); @@ -166,7 +170,6 @@ async function blockFrames(page: Page) { for (const bytes of [2 ** 20, 16 * 2 ** 20]) { test(`a blocked download fails instead of hanging (${bytes} bytes)`, async ({ page }) => { - test.setTimeout(30_000); // Below the worker's 4 MiB queue, every byte fits in it without the // browser reading any, so this once reported success with no file; above // it, the pipe waited forever for credit. @@ -190,8 +193,6 @@ test.describe('without a service worker', () => { test.use({ serviceWorkers: 'block' }); test('falls back to an in-memory Blob', async ({ page }) => { - // toFile waits up to 5 s for a worker before giving up. - test.setTimeout(30_000); const bytes = 2 ** 20 + 17; const downloadPromise = page.waitForEvent('download'); const strategy = await streamDownload(page, bytes); From 5663e55c9235b4d02a4a27d23d9078eca8cb9ad9 Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Wed, 21 Oct 2026 12:48:39 -0400 Subject: [PATCH 06/10] test(main): unit-test the download sink's protocol and timeouts (DSPX-159) Nothing on a pull request checked that the page handles 'started' and 'done', or that the stall watchdog fires: a 3 MiB download finishes before any timeout. Add Vitest tests that drive toFile against a scripted worker with fake timers: one chunk per pull, close waiting for 'done', the start and stall timeouts, a slow download that keeps reading, cancel, abort during close, and both fallbacks. Add a paced sink.spec download that outlasts the start timeout, so the real worker must report 'started'. Signed-off-by: Dave Mihalcik --- web-app/src/downloadSink.test.ts | 207 +++++++++++++++++++++++++++++++ web-app/tests/tests/sink.spec.ts | 11 ++ 2 files changed, 218 insertions(+) create mode 100644 web-app/src/downloadSink.test.ts diff --git a/web-app/src/downloadSink.test.ts b/web-app/src/downloadSink.test.ts new file mode 100644 index 000000000..658712a59 --- /dev/null +++ b/web-app/src/downloadSink.test.ts @@ -0,0 +1,207 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { toFile } from './downloadSink.js'; + +// Drives the page side of the download protocol against a scripted worker, with +// fake timers so the start and stall timeouts run instantly. Port messages +// still travel on the real event loop, which the fake timers leave alone. + +const MINUTE = 60_000; + +type Message = { type: string; [key: string]: unknown }; + +/** Plays public/sw.js: records what the page sends and answers on cue. */ +class FakeWorker { + port?: MessagePort; + received: Message[] = []; + + postMessage(message: Message, transfer: Transferable[] = []) { + if (message.type !== 'register') { + return; + } + this.port = transfer[0] as MessagePort; + this.port.onmessage = ({ data }: MessageEvent) => void this.received.push(data); + } + + send(...messages: Message[]) { + for (const message of messages) { + this.port?.postMessage(message); + } + } + + types() { + return this.received.map(({ type }) => type); + } +} + +function source(chunks: number) { + let sent = 0; + return new ReadableStream({ + pull(controller) { + if (sent++ < chunks) { + controller.enqueue(new Uint8Array(1024)); + } else { + controller.close(); + } + }, + }); +} + +function track(promise: Promise) { + const state: { settled: boolean; value?: T; error?: unknown } = { settled: false }; + promise.then( + (value) => Object.assign(state, { settled: true, value }), + (error: unknown) => Object.assign(state, { settled: true, error }) + ); + return state; +} + +// Lets queued port messages, and the stream work they trigger, run. +async function flush() { + for (let i = 0; i < 20; i++) { + await new Promise((resolve) => setImmediate(resolve)); + } +} + +let worker: FakeWorker; + +beforeEach(() => { + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout', 'setInterval', 'clearInterval'] }); + worker = new FakeWorker(); + vi.stubGlobal('window', globalThis); + vi.stubGlobal('navigator', { + serviceWorker: { ready: Promise.resolve({ active: worker, scope: 'http://app.test/' }) }, + }); + vi.stubGlobal('document', { + body: { appendChild() {} }, + createElement: () => ({ click() {}, remove() {} }), + }); + vi.spyOn(console, 'warn').mockImplementation(() => {}); +}); + +afterEach(() => { + worker.port?.close(); + vi.useRealTimers(); + vi.unstubAllGlobals(); + vi.restoreAllMocks(); +}); + +describe('toFile through the service worker', () => { + it("resolves only once the worker reports 'done'", async () => { + const result = track(toFile(source(2))); + await flush(); + worker.send({ type: 'ready' }, { type: 'started' }, { type: 'pull' }, { type: 'pull' }); + await flush(); + expect(worker.types()).toEqual(['chunk', 'chunk', 'close']); + expect(result.settled).toBe(false); + worker.send({ type: 'done' }); + await flush(); + expect(result).toEqual({ settled: true, value: 'service-worker' }); + }); + + it('sends one chunk per pull', async () => { + track(toFile(source(3))); + await flush(); + worker.send({ type: 'ready' }, { type: 'started' }); + await flush(); + expect(worker.types()).toEqual([]); + worker.send({ type: 'pull' }); + await flush(); + expect(worker.types()).toEqual(['chunk']); + }); + + it('fails if the download never reaches the worker', async () => { + const result = track(toFile(source(1))); + await flush(); + // Credit arrives before any download exists: the worker fills its queue. + worker.send({ type: 'ready' }, { type: 'pull' }, { type: 'pull' }); + await flush(); + expect(worker.types()).toEqual(['chunk', 'close']); + await vi.advanceTimersByTimeAsync(10_000); + await flush(); + expect(String(result.error)).toMatch(/never reached the service worker/); + expect(worker.types()).toEqual(['chunk', 'close', 'abort']); + }); + + it('fails a started download once the browser stops reading', async () => { + const result = track(toFile(source(2))); + await flush(); + worker.send({ type: 'ready' }, { type: 'started' }, { type: 'pull' }); + await flush(); + await vi.advanceTimersByTimeAsync(10 * MINUTE - 1); + await flush(); + expect(result.settled).toBe(false); + await vi.advanceTimersByTimeAsync(1); + await flush(); + expect(String(result.error)).toMatch(/Download stalled/); + expect(worker.types()).toEqual(['chunk', 'abort']); + }); + + it('keeps a slow download going while the browser keeps reading', async () => { + const result = track(toFile(source(3))); + await flush(); + worker.send({ type: 'ready' }, { type: 'started' }); + for (let i = 0; i < 3; i++) { + await vi.advanceTimersByTimeAsync(9 * MINUTE); + worker.send({ type: 'pull' }); + await flush(); + } + // close() waits for 'done' under the same rule. + await vi.advanceTimersByTimeAsync(9 * MINUTE); + worker.send({ type: 'done' }); + await flush(); + expect(result).toEqual({ settled: true, value: 'service-worker' }); + }); + + it("rejects on the worker's cancel without sending abort back", async () => { + const result = track(toFile(source(2))); + await flush(); + worker.send({ type: 'ready' }, { type: 'started' }); + worker.send({ type: 'cancel', reason: 'user cancelled' }); + await flush(); + expect(String(result.error)).toMatch(/Download cancelled: user cancelled/); + expect(worker.types()).not.toContain('abort'); + }); + + it('an abort ends the wait for done', async () => { + const abort = new AbortController(); + const result = track(toFile(source(1), 'f.bin', { signal: abort.signal })); + await flush(); + worker.send({ type: 'ready' }, { type: 'started' }, { type: 'pull' }); + await flush(); + expect(worker.types()).toEqual(['chunk', 'close']); + abort.abort(new Error('user abort')); + await flush(); + expect(String(result.error)).toMatch(/user abort/); + expect(worker.types()).toEqual(['chunk', 'close', 'abort']); + }); +}); + +describe('toFile fallback', () => { + it("buffers in memory, and frees the worker's stream, when ready never comes", async () => { + const onFallback = vi.fn(); + const result = track(toFile(source(1), 'f.bin', { onFallback })); + await flush(); + await vi.advanceTimersByTimeAsync(5_000); + await flush(); + expect(result).toEqual({ settled: true, value: 'blob' }); + expect(onFallback).toHaveBeenCalledWith(expect.stringMatching(/did not answer/)); + expect(worker.types()).toEqual(['abort']); + }); + + it('falls back at once when registration failed', async () => { + vi.stubGlobal('navigator', { + serviceWorker: { register: () => Promise.reject(new Error('sw.js 404')) }, + }); + // registerDownloadWorker keeps module state, so use a fresh copy. + vi.resetModules(); + const sink = await import('./downloadSink.js'); + sink.registerDownloadWorker(); + const onFallback = vi.fn(); + const result = track(sink.toFile(source(1), 'f.bin', { onFallback })); + await flush(); + expect(result).toEqual({ settled: true, value: 'blob' }); + expect(onFallback).toHaveBeenCalledWith( + expect.stringMatching(/registration failed: sw.js 404/) + ); + }); +}); diff --git a/web-app/tests/tests/sink.spec.ts b/web-app/tests/tests/sink.spec.ts index 8e0253e84..65b4d5df3 100644 --- a/web-app/tests/tests/sink.spec.ts +++ b/web-app/tests/tests/sink.spec.ts @@ -145,6 +145,17 @@ test('streams a download through the service worker', async ({ page }) => { expect(strategy).toBe('service-worker'); }); +test('a download that outlasts the start timeout completes', async ({ page }) => { + // Paced past toFile's 10 s start timeout, so this fails if the worker stops + // reporting that the download started. + const bytes = 12 * 2 ** 20; + const downloadPromise = page.waitForEvent('download'); + const strategy = await streamDownload(page, bytes, { mibps: 1 }); + const download = await downloadPromise; + await expectPattern(await downloadPath(download), bytes); + expect(strategy).toBe('service-worker'); +}); + test('an abort fails the download instead of truncating it', async ({ page, browserName }) => { // Firefox does error the download, but Playwright's Firefox never settles // download.failure() for it. From 01a5116e9e7b518a0d522b228449fb074e8d7bf2 Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Wed, 21 Oct 2026 12:50:34 -0400 Subject: [PATCH 07/10] fix(main): fail a download whose port message can't be read (DSPX-159) If a chunk fails to deserialize, the port fires messageerror and the chunk is lost after the page has spent its credit on it. Nothing listened, so the file could finish with a hole in it and still report success. Both ends now fail the download instead. Signed-off-by: Dave Mihalcik --- web-app/public/sw.js | 7 +++++++ web-app/src/downloadSink.ts | 2 ++ 2 files changed, 9 insertions(+) diff --git a/web-app/public/sw.js b/web-app/public/sw.js index 810f20583..310111d9b 100644 --- a/web-app/public/sw.js +++ b/web-app/public/sw.js @@ -57,6 +57,13 @@ class PortSource { console.warn(`download worker: unknown message type ${data?.type}`); } }; + // A chunk that can't be deserialized is lost, and the page has already + // spent its credit on it; fail rather than save a file with a hole in it. + this.port.onmessageerror = () => { + const error = new Error('a message from the page could not be read'); + controller.error(error); + this.cancel(error.message); + }; } pull() { diff --git a/web-app/src/downloadSink.ts b/web-app/src/downloadSink.ts index c0645f3f7..cf2c1e3c5 100644 --- a/web-app/src/downloadSink.ts +++ b/web-app/src/downloadSink.ts @@ -193,6 +193,8 @@ class PortSink implements UnderlyingSink { } this.#wake?.(); }; + port1.onmessageerror = () => + this.#fail(new Error('A message from the download worker could not be read')); } start(controller: WritableStreamDefaultController) { From c8902f55da9255225158f3f3d7d18e2faa2a7165 Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Wed, 21 Oct 2026 12:51:21 -0400 Subject: [PATCH 08/10] fix(main): say when only saving failed, and what to do (DSPX-159) Failures of the download itself (it never started, stalled, was cancelled in the browser, or a message couldn't be read) were shown as "Encrypt Failed" or "Decrypt Failed", which suggests the KAS or the crypto went wrong. The sink now throws DownloadError for these, and the app shows "Encrypted, but saving failed" instead. The start timeout message now names a site update as a cause and tells the user to reload, and the fallback reason no longer claims a 5 s wait that didn't happen. Signed-off-by: Dave Mihalcik --- web-app/src/App.tsx | 15 ++++++++++++--- web-app/src/downloadSink.test.ts | 9 +++++++-- web-app/src/downloadSink.ts | 23 +++++++++++++++-------- web-app/tests/tests/sink.spec.ts | 2 +- 4 files changed, 35 insertions(+), 14 deletions(-) diff --git a/web-app/src/App.tsx b/web-app/src/App.tsx index 8e22d0b62..00cbf9784 100644 --- a/web-app/src/App.tsx +++ b/web-app/src/App.tsx @@ -19,7 +19,7 @@ import { decryptedFileName, expectedKaoType, } from './fileNames.js'; -import { canSaveAs, toFile } from './downloadSink.js'; +import { canSaveAs, DownloadError, toFile } from './downloadSink.js'; const oidcClient = new OidcClient(config.oidc.host, config.oidc.clientId, 'otdf-sample-web-app'); @@ -145,6 +145,11 @@ function fileNameFor(inputSource: InputSource) { return pathname.slice(i + 1); } +// The SDK can have succeeded when only saving its output failed. +function failureLabel(e: unknown, type: 'Encrypt' | 'Decrypt') { + return e instanceof DownloadError ? `${type}ed, but saving failed` : `${type} Failed`; +} + function drain() { let byteCounter = 0; let startTime: number; @@ -509,7 +514,9 @@ function App() { } setDownloadState('✍️ Encrypt Complete'); } catch (e) { - setDownloadState(`Encrypt Failed: ${e instanceof Error ? e.message : String(e)}`); + setDownloadState( + `${failureLabel(e, 'Encrypt')}: ${e instanceof Error ? e.message : String(e)}` + ); console.error('Encrypt Failed', e); } setStreamController(undefined); @@ -610,7 +617,9 @@ function App() { ); } catch (e) { console.error('Decrypt Failed', e); - setDownloadState(`Decrypt Failed: ${e instanceof Error ? e.message : String(e)}`); + setDownloadState( + `${failureLabel(e, 'Decrypt')}: ${e instanceof Error ? e.message : String(e)}` + ); } setStreamController(undefined); return false; diff --git a/web-app/src/downloadSink.test.ts b/web-app/src/downloadSink.test.ts index 658712a59..abfc6bc01 100644 --- a/web-app/src/downloadSink.test.ts +++ b/web-app/src/downloadSink.test.ts @@ -1,5 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -import { toFile } from './downloadSink.js'; +import { DownloadError, toFile } from './downloadSink.js'; // Drives the page side of the download protocol against a scripted worker, with // fake timers so the start and stall timeouts run instantly. Port messages @@ -118,7 +118,8 @@ describe('toFile through the service worker', () => { expect(worker.types()).toEqual(['chunk', 'close']); await vi.advanceTimersByTimeAsync(10_000); await flush(); - expect(String(result.error)).toMatch(/never reached the service worker/); + expect(result.error).toBeInstanceOf(DownloadError); + expect(String(result.error)).toMatch(/didn't start within 10 s/); expect(worker.types()).toEqual(['chunk', 'close', 'abort']); }); @@ -132,6 +133,7 @@ describe('toFile through the service worker', () => { expect(result.settled).toBe(false); await vi.advanceTimersByTimeAsync(1); await flush(); + expect(result.error).toBeInstanceOf(DownloadError); expect(String(result.error)).toMatch(/Download stalled/); expect(worker.types()).toEqual(['chunk', 'abort']); }); @@ -158,6 +160,7 @@ describe('toFile through the service worker', () => { worker.send({ type: 'ready' }, { type: 'started' }); worker.send({ type: 'cancel', reason: 'user cancelled' }); await flush(); + expect(result.error).toBeInstanceOf(DownloadError); expect(String(result.error)).toMatch(/Download cancelled: user cancelled/); expect(worker.types()).not.toContain('abort'); }); @@ -171,6 +174,8 @@ describe('toFile through the service worker', () => { expect(worker.types()).toEqual(['chunk', 'close']); abort.abort(new Error('user abort')); await flush(); + // The caller's own abort, not a failed download. + expect(result.error).not.toBeInstanceOf(DownloadError); expect(String(result.error)).toMatch(/user abort/); expect(worker.types()).toEqual(['chunk', 'close', 'abort']); }); diff --git a/web-app/src/downloadSink.ts b/web-app/src/downloadSink.ts index cf2c1e3c5..2be3759f3 100644 --- a/web-app/src/downloadSink.ts +++ b/web-app/src/downloadSink.ts @@ -11,6 +11,11 @@ export type SinkStrategy = 'service-worker' | 'blob'; +/** The download itself failed, as opposed to the stream feeding it. */ +export class DownloadError extends Error { + override name = 'DownloadError'; +} + export type ToFileOptions = StreamPipeOptions & { /** * Called with the reason before falling back to buffering the whole output @@ -108,7 +113,7 @@ async function findWorker(): Promise<{ worker: ServiceWorker; scope: string } | return `service worker registration failed: ${e instanceof Error ? e.message : String(e)}`; } if (!registration?.active) { - return `no service worker became active within ${WORKER_TIMEOUT_MS / 1000} s`; + return 'no service worker is active'; } return { worker: registration.active, scope: registration.scope }; } @@ -188,13 +193,13 @@ class PortSink implements UnderlyingSink { break; case 'cancel': // The worker has already closed its end. - this.#fail(new Error(`Download cancelled: ${data.reason}`), false); + this.#fail(new DownloadError(`Download cancelled: ${data.reason}`), false); break; } this.#wake?.(); }; port1.onmessageerror = () => - this.#fail(new Error('A message from the download worker could not be read')); + this.#fail(new DownloadError('A message from the download worker could not be read')); } start(controller: WritableStreamDefaultController) { @@ -210,9 +215,11 @@ class PortSink implements UnderlyingSink { this.#startTimer = setTimeout( () => this.#fail( - new Error( - 'The download never reached the service worker; a Content-Security-Policy ' + - 'frame-src, a browser extension or DevTools "Bypass for network" may be blocking it' + new DownloadError( + `The download didn't start within ${START_TIMEOUT_MS / 1000} s: the browser never ` + + "fetched it from the app's service worker. A frame-src Content-Security-Policy, " + + 'a browser extension, DevTools "Bypass for network" or a site update can cause ' + + 'this. Reload the page and try again.' ) ), START_TIMEOUT_MS @@ -259,9 +266,9 @@ class PortSink implements UnderlyingSink { ? setTimeout( () => this.#fail( - new Error( + new DownloadError( `Download stalled: the browser read nothing for ${STALL_TIMEOUT_MS / 60_000} minutes. ` + - 'Was it cancelled in the downloads list?' + "Was it cancelled or paused in the browser's downloads list?" ) ), STALL_TIMEOUT_MS diff --git a/web-app/tests/tests/sink.spec.ts b/web-app/tests/tests/sink.spec.ts index 65b4d5df3..64bc965f1 100644 --- a/web-app/tests/tests/sink.spec.ts +++ b/web-app/tests/tests/sink.spec.ts @@ -186,7 +186,7 @@ for (const bytes of [2 ** 20, 16 * 2 ** 20]) { // it, the pipe waited forever for credit. await blockFrames(page); const result = await streamDownload(page, bytes); - expect(result).toMatch(/^rejected: The download never reached the service worker/); + expect(result).toMatch(/^rejected: The download didn't start within 10 s/); }); } From 1c172a95becd5af04b2f464fb7a7867dac328c9a Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Wed, 21 Oct 2026 12:52:53 -0400 Subject: [PATCH 09/10] docs(main): correct DSPX-159 comments after review Fix comments the second review found inaccurate or ambiguous: when the worker's queue fills, that an abort can also follow 'close' after a stall, that the sink also stops when a download never starts, and that the moz-firefox config runs one test at a time. The sw.js header now lists every message itself rather than pointing at a spec. Signed-off-by: Dave Mihalcik --- web-app/public/sw.js | 12 ++++++++---- web-app/src/downloadSink.ts | 13 +++++++------ web-app/tests/tests/acts.ts | 4 ++-- 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/web-app/public/sw.js b/web-app/public/sw.js index 310111d9b..8d0256ec6 100644 --- a/web-app/public/sw.js +++ b/web-app/public/sw.js @@ -1,7 +1,6 @@ // Streams downloads from the page straight into the browser's download manager, // so a multi-GiB encrypt or decrypt never has to fit in memory or in storage -// quota. The page side lives in src/downloadSink.ts; spec/DSPX-159.md lists -// every message. +// quota. The page side lives in web-app/src/downloadSink.ts. // // 1. The page posts { type: 'register', id, filename } with a MessagePort. // 2. We wrap the port in a ReadableStream and reply { type: 'ready' }. @@ -14,6 +13,11 @@ // manager's reads, so backpressure reaches all the way back to the SDK. // 5. After the page posts { type: 'close' } and the browser has read the last // byte, we post { type: 'done' }. +// +// Either side can end a download early: the page posts +// { type: 'abort', reason }, and we post { type: 'cancel', reason } when the +// browser cancels the response. While streaming, the page also posts +// { type: 'ping' } every 10 s, so the browser doesn't stop us mid-download. const HIGH_WATER_MARK_BYTES = 4 * 2 ** 20; @@ -48,8 +52,8 @@ class PortSource { break; case 'abort': // Erroring the body makes the browser mark the download failed, - // rather than leaving a truncated file that looks complete. This - // also arrives after 'close' if the download never started. + // rather than leaving a truncated file that looks complete. This can + // also arrive after 'close', if the download never started or stalled. controller.error(new Error(String(data.reason ?? 'aborted'))); this.end(); break; diff --git a/web-app/src/downloadSink.ts b/web-app/src/downloadSink.ts index 2be3759f3..a26e3b398 100644 --- a/web-app/src/downloadSink.ts +++ b/web-app/src/downloadSink.ts @@ -3,10 +3,11 @@ * * The page hands the stream to our own service worker (public/sw.js) over a * MessagePort, then navigates a hidden iframe to a URL the worker answers with - * a streamed attachment. The browser's download manager pulls from that - * response, and each pull grants the page one more chunk, so backpressure - * reaches back through the SDK to its source. This replaces StreamSaver, which - * did the same through a service worker on a third-party origin. + * a streamed attachment. Once the worker's 4 MiB queue has filled, each pull + * by the browser's download manager grants the page one more chunk, so + * backpressure reaches back through the SDK to its source. This replaces + * StreamSaver, which did the same through a service worker on a third-party + * origin. */ export type SinkStrategy = 'service-worker' | 'blob'; @@ -165,8 +166,8 @@ class PortSink implements UnderlyingSink { #startTimer?: ReturnType; // Wakes a write or close waiting for the next message from the worker. #wake?: () => void; - // Set when the download is cancelled, stalls or the stream aborts; wakes a - // waiting write or close so it can throw. + // Set when the download is cancelled, never starts, stalls or the stream + // aborts; wakes a waiting write or close so it can throw. #stopped?: Error; #controller?: WritableStreamDefaultController; diff --git a/web-app/tests/tests/acts.ts b/web-app/tests/tests/acts.ts index bf2cf0e5e..f89e456c4 100644 --- a/web-app/tests/tests/acts.ts +++ b/web-app/tests/tests/acts.ts @@ -31,8 +31,8 @@ export const authorize = async (page: Page) => { * folder and saves under the suggested name in PLAYWRIGHT_DOWNLOAD_DIR, so * `download.path()` names a missing file. It would save a later download with * the same name as `name(1).ext`, so each file is moved into a folder of its - * own once found. That relies on one download at a time, so that config runs a - * single worker. + * own once found. That relies on one download at a time, so that config runs + * one test at a time (`workers: 1`). */ export const downloadPath = async ( download: Download, From 83e8ae36459d266c66baa4c091c1c8193fbd2f98 Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Wed, 21 Oct 2026 13:09:04 -0400 Subject: [PATCH 10/10] test(main): skip the Blob fallback test under Playwright's BiDi Firefox (DSPX-159) Stock Firefox saves the in-memory fallback download, but Playwright's BiDi driver only reports downloads that come from a navigation, so the test timed out waiting for an event. The rest of sink.spec.ts passes on stock Firefox. Signed-off-by: Dave Mihalcik --- web-app/tests/tests/sink.spec.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/web-app/tests/tests/sink.spec.ts b/web-app/tests/tests/sink.spec.ts index 64bc965f1..90755a117 100644 --- a/web-app/tests/tests/sink.spec.ts +++ b/web-app/tests/tests/sink.spec.ts @@ -203,7 +203,10 @@ test('an abort still works once the source has closed', async ({ page }) => { test.describe('without a service worker', () => { test.use({ serviceWorkers: 'block' }); - test('falls back to an in-memory Blob', async ({ page }) => { + test('falls back to an in-memory Blob', async ({ page, browserName }) => { + // Stock Firefox saves the file, but Playwright's BiDi driver reports only + // downloads that come from a navigation, not a click on a Blob link. + test.skip(browserName === '_bidiFirefox', 'Playwright BiDi misses Blob downloads'); const bytes = 2 ** 20 + 17; const downloadPromise = page.waitForEvent('download'); const strategy = await streamDownload(page, bytes);