diff --git a/components/email/email-composer.tsx b/components/email/email-composer.tsx index 73008d852..28082b666 100644 --- a/components/email/email-composer.tsx +++ b/components/email/email-composer.tsx @@ -18,6 +18,7 @@ import { isEditableEventTarget } from "@/lib/keyboard"; import { buildQuotedHtmlBlock, serializeEditorContent } from "@/components/email/quoted-html"; import { buildSignatureBlock, containsEmbeddedSignature, SIGNATURE_RANGE_MARKER } from "@/components/email/signature-block"; import { emailHooks, contactHooks, isExternalAttachmentResult } from "@/lib/plugin-hooks"; +import { onUploadProgress } from "@/lib/upload-progress"; import type { AlmostSavedDraft, OutgoingEmail, RecipientSuggestion } from "@/lib/plugin-types"; import { useAuthStore } from "@/stores/auth-store"; import { useIdentityStore } from "@/stores/identity-store"; @@ -217,6 +218,14 @@ type ComposerAttachment = { size: number; blobId?: string; uploading?: boolean; + /** + * Transferred share of the upload, 0-100. Set while `uploading` when the + * transport reports byte progress: the stock path via the JMAP client's + * `onProgress`, a plugin offload via the staged-file-id registry + * (`lib/upload-progress.ts`). Absent when nothing has reported yet, in + * which case the chip falls back to its indeterminate bar. + */ + progress?: number; error?: boolean; abortController?: AbortController; // Carried through for parts hydrated from an existing draft so inline @@ -1387,60 +1396,82 @@ export function EmailComposer({ const fileId = generateUUID(); await fileStorage.saveFile(fileId, file); - - const transformed = await emailHooks.onBeforeBlobUpload.transform(fileId); - // A handler can offload the file elsewhere and hand back replacement - // content instead of a file id. Drop the binary attachment and put the - // replacement in the body. - if (isExternalAttachmentResult(transformed)) { - // `transformed.fileId` when the handler re-saved the staged file - // under a new id; otherwise the id we handed it. - await fileStorage.deleteFile(transformed.fileId ?? fileId); - // The user may have removed the attachment while the handler was - // offloading it - don't drop a link for a file they cancelled. - if (controller?.signal.aborted) continue; - setAttachments(prev => prev.filter(att => att.file !== file)); - if (plainTextMode) { - setBody(previous => - previous && !previous.endsWith('\n') ? `${previous}\n${transformed.text}` : previous + transformed.text - ); - } else { - // Never trust plugin markup in the document (or in the message the - // user then sends); insert through the editor so it lands at the - // caret rather than after the signature and quoted block. - const html = sanitizePluginBodyHtml(transformed.html); - if (!html) continue; - if (editorRef.current) { - editorRef.current.chain().focus().insertContent(html).run(); + // Byte progress for the chip, from whichever transport moves the + // bytes: the JMAP client below reports directly, a plugin offloading + // via `api.http.post` reports through the staged-file-id registry + // (it echoes `fileId` as `progressFileId`, see doHttpPost). + const reportProgress = (loaded: number, total: number) => { + if (total <= 0 || controller?.signal.aborted) return; + const pct = Math.min(100, Math.floor((loaded / total) * 100)); + setAttachments(prev => + prev.map(att => (att.file === file ? { ...att, progress: pct } : att)) + ); + }; + const stopProgress = onUploadProgress(fileId, reportProgress); + + try { + const transformed = await emailHooks.onBeforeBlobUpload.transform(fileId); + + // A handler can offload the file elsewhere and hand back replacement + // content instead of a file id. Drop the binary attachment and put the + // replacement in the body. + if (isExternalAttachmentResult(transformed)) { + // `transformed.fileId` when the handler re-saved the staged file + // under a new id; otherwise the id we handed it. + await fileStorage.deleteFile(transformed.fileId ?? fileId); + // The user may have removed the attachment while the handler was + // offloading it - don't drop a link for a file they cancelled. + if (controller?.signal.aborted) continue; + setAttachments(prev => prev.filter(att => att.file !== file)); + if (plainTextMode) { + setBody(previous => + previous && !previous.endsWith('\n') ? `${previous}\n${transformed.text}` : previous + transformed.text + ); } else { - setBody(previous => previous + html); + // Never trust plugin markup in the document (or in the message the + // user then sends); insert through the editor so it lands at the + // caret rather than after the signature and quoted block. + const html = sanitizePluginBodyHtml(transformed.html); + if (!html) continue; + if (editorRef.current) { + editorRef.current.chain().focus().insertContent(html).run(); + } else { + setBody(previous => previous + html); + } } + continue; } - continue; + + const newFileId = typeof transformed === 'string' ? transformed : fileId; + + const newFile = await fileStorage.getFile(newFileId) || file; + await fileStorage.deleteFile(newFileId); + + // Passing the signal also makes cancel abort the transfer itself, + // instead of only being checked once the upload has finished. + const { blobId } = await client.uploadBlob(newFile, { + onProgress: reportProgress, + signal: controller?.signal, + }); + + if (controller?.signal.aborted) continue; + setAttachments(prev => + prev.map(att => + att.file === file + ? { ...att, blobId, uploading: false, abortController: undefined } + : att + ) + ); + emailHooks.onAfterAttachmentUpload.emit({ + name: file.name, + type: file.type || 'application/octet-stream', + size: file.size, + blobId, + }); + } finally { + stopProgress(); } - - const newFileId = typeof transformed === 'string' ? transformed : fileId; - - const newFile = await fileStorage.getFile(newFileId) || file; - await fileStorage.deleteFile(newFileId); - - const { blobId } = await client.uploadBlob(newFile); - - if (controller?.signal.aborted) continue; - setAttachments(prev => - prev.map(att => - att.file === file - ? { ...att, blobId, uploading: false, abortController: undefined } - : att - ) - ); - emailHooks.onAfterAttachmentUpload.emit({ - name: file.name, - type: file.type || 'application/octet-stream', - size: file.size, - blobId, - }); } catch (error) { if (controller?.signal.aborted) continue; debug.error(`Failed to upload ${file.name}:`, error); @@ -2891,7 +2922,16 @@ export function EmailComposer({ {att.uploading && (
-
+ {typeof att.progress === 'number' ? ( + // Real byte progress from the transport; see + // ComposerAttachment.progress for who reports it. +
+ ) : ( +
+ )}
)}
diff --git a/lib/__tests__/plugin-upload-progress.test.ts b/lib/__tests__/plugin-upload-progress.test.ts new file mode 100644 index 000000000..7f1362a8c --- /dev/null +++ b/lib/__tests__/plugin-upload-progress.test.ts @@ -0,0 +1,154 @@ +import { afterEach, beforeEach, describe, it, expect, vi } from 'vitest'; +import type { InstalledPlugin } from '../plugin-types'; +import { useAuthStore } from '@/stores/auth-store'; +import { onUploadProgress } from '../upload-progress'; + +vi.mock('@/stores/toast-store', () => ({ + toast: { success: vi.fn(), error: vi.fn(), info: vi.fn(), warning: vi.fn() }, +})); + +import { dispatchApiCall } from '../plugin-sandbox/host-api'; + +function plugin(overrides: Partial = {}): InstalledPlugin { + return { + id: 'p1', + name: 'Test plugin', + version: '1.0.0', + author: 'test', + description: '', + type: 'hook', + permissions: ['http:post'], + grantedPermissions: ['http:post'], + entrypoint: 'index.js', + enabled: true, + status: 'running', + settings: {}, + apiPostPaths: ['/api/upload'], + ...overrides, + }; +} + +/** + * Stand-in for XMLHttpRequest that records the request and lets the test + * drive upload progress and completion by hand. + */ +class FakeXHR { + static instances: FakeXHR[] = []; + method = ''; + url = ''; + headers: Record = {}; + sentBody: unknown = null; + status = 200; + responseText = '{"url":"https://example.test/d/1"}'; + upload: { onprogress: ((e: { lengthComputable: boolean; loaded: number; total: number }) => void) | null } = { onprogress: null }; + onload: (() => void) | null = null; + onerror: (() => void) | null = null; + ontimeout: (() => void) | null = null; + open(method: string, url: string) { this.method = method; this.url = url; } + setRequestHeader(name: string, value: string) { this.headers[name] = value; } + send(body: unknown) { this.sentBody = body; FakeXHR.instances.push(this); } +} + +describe('plugin binary upload progress', () => { + beforeEach(() => { + FakeXHR.instances = []; + vi.stubGlobal('XMLHttpRequest', FakeXHR); + useAuthStore.setState({ + client: { + getAuthHeader: () => 'Basic dGVzdA==', + getUsername: () => 'tester', + } as never, + }); + }); + + afterEach(() => { + vi.unstubAllGlobals(); + useAuthStore.setState({ client: null }); + }); + + it('reports byte progress into the registry keyed by progressFileId', async () => { + const seen: Array<[number, number]> = []; + const off = onUploadProgress('staged-1', (loaded, total) => seen.push([loaded, total])); + + const call = dispatchApiCall(plugin(), 'http.post', [ + '/api/upload', + new Blob(['0123456789']), + { progressFileId: 'staged-1' }, + ]); + + // send() is synchronous inside the promise executor, so the request is + // already captured even though the call promise is still pending. + expect(FakeXHR.instances).toHaveLength(1); + const xhr = FakeXHR.instances[0]; + xhr.upload.onprogress?.({ lengthComputable: true, loaded: 4, total: 10 }); + xhr.upload.onprogress?.({ lengthComputable: true, loaded: 10, total: 10 }); + xhr.onload?.(); + + const result = await call as { ok: boolean; status: number; data: unknown }; + expect(seen).toEqual([[4, 10], [10, 10]]); + expect(result.ok).toBe(true); + expect(result.data).toEqual({ url: 'https://example.test/d/1' }); + off(); + }); + + it('still applies credential headers after plugin headers on the XHR path', async () => { + const call = dispatchApiCall(plugin(), 'http.post', [ + '/api/upload', + new Blob(['x'], { type: 'application/octet-stream' }), + { headers: { 'X-Plugin-Name': 'a.bin' }, progressFileId: 'staged-2' }, + ]); + const xhr = FakeXHR.instances[0]; + expect(xhr.headers['Authorization']).toBe('Basic dGVzdA=='); + expect(xhr.headers['X-JMAP-Username']).toBe('tester'); + expect(xhr.headers['X-Plugin-Name']).toBe('a.bin'); + xhr.onload?.(); + await call; + }); + + it('skips progress events whose length is not computable', async () => { + const listener = vi.fn(); + const off = onUploadProgress('staged-3', listener); + const call = dispatchApiCall(plugin(), 'http.post', [ + '/api/upload', + new Blob(['x']), + { progressFileId: 'staged-3' }, + ]); + const xhr = FakeXHR.instances[0]; + xhr.upload.onprogress?.({ lengthComputable: false, loaded: 0, total: 0 }); + xhr.onload?.(); + await call; + expect(listener).not.toHaveBeenCalled(); + off(); + }); + + it('rejects a malformed progressFileId', async () => { + await expect( + dispatchApiCall(plugin(), 'http.post', [ + '/api/upload', + new Blob(['x']), + { progressFileId: 'x'.repeat(129) }, + ]), + ).rejects.toThrow(/progressFileId/); + expect(FakeXHR.instances).toHaveLength(0); + }); + + it('keeps the fetch path when no progressFileId is given', async () => { + const fetchSpy = vi.fn().mockResolvedValue( + new Response(JSON.stringify({ ok: 1 }), { status: 200 }), + ); + vi.stubGlobal('fetch', fetchSpy); + await dispatchApiCall(plugin(), 'http.post', ['/api/upload', new Blob(['x']), {}]); + expect(fetchSpy).toHaveBeenCalledTimes(1); + expect(FakeXHR.instances).toHaveLength(0); + }); + + it('surfaces a network error as a rejection', async () => { + const call = dispatchApiCall(plugin(), 'http.post', [ + '/api/upload', + new Blob(['x']), + { progressFileId: 'staged-4' }, + ]); + FakeXHR.instances[0].onerror?.(); + await expect(call).rejects.toThrow(/Network error/); + }); +}); diff --git a/lib/__tests__/upload-progress.test.ts b/lib/__tests__/upload-progress.test.ts new file mode 100644 index 000000000..21d149285 --- /dev/null +++ b/lib/__tests__/upload-progress.test.ts @@ -0,0 +1,50 @@ +import { describe, it, expect, vi } from 'vitest'; +import { onUploadProgress, reportUploadProgress } from '../upload-progress'; + +describe('upload progress registry', () => { + it('delivers reports to the listener registered for the id', () => { + const listener = vi.fn(); + const off = onUploadProgress('file-1', listener); + reportUploadProgress('file-1', 512, 2048); + expect(listener).toHaveBeenCalledWith(512, 2048); + off(); + }); + + it('drops reports for an id nobody listens to', () => { + const listener = vi.fn(); + const off = onUploadProgress('file-1', listener); + reportUploadProgress('file-2', 1, 2); + expect(listener).not.toHaveBeenCalled(); + off(); + }); + + it('stops delivering after unsubscribe', () => { + const listener = vi.fn(); + const off = onUploadProgress('file-1', listener); + off(); + reportUploadProgress('file-1', 1, 2); + expect(listener).not.toHaveBeenCalled(); + }); + + it('ignores non-finite byte counts', () => { + const listener = vi.fn(); + const off = onUploadProgress('file-1', listener); + reportUploadProgress('file-1', NaN, 100); + reportUploadProgress('file-1', 50, Infinity); + expect(listener).not.toHaveBeenCalled(); + off(); + }); + + it("unsubscribing a stale registration does not drop the id's current listener", () => { + const first = vi.fn(); + const second = vi.fn(); + const offFirst = onUploadProgress('file-1', first); + const offSecond = onUploadProgress('file-1', second); + // The composer re-registered the id; the stale unsubscribe must be a no-op. + offFirst(); + reportUploadProgress('file-1', 10, 100); + expect(second).toHaveBeenCalledWith(10, 100); + expect(first).not.toHaveBeenCalled(); + offSecond(); + }); +}); diff --git a/lib/plugin-sandbox/host-api.ts b/lib/plugin-sandbox/host-api.ts index df7719a62..161bdf36f 100644 --- a/lib/plugin-sandbox/host-api.ts +++ b/lib/plugin-sandbox/host-api.ts @@ -18,7 +18,8 @@ import { type KeywordVisibility, } from '@/stores/settings-store'; import type { MessageListTabsConfig } from '../plugin-types'; -import { apiFetch } from '../browser-navigation'; +import { apiFetch, getPathPrefix } from '../browser-navigation'; +import { reportUploadProgress } from '../upload-progress'; import { DEFAULT_KEYWORD_SCAN_LIMIT } from '../jmap/client'; import { suggestKeywordColor } from '../keyword-discovery'; import { MAX_KEYWORD_LENGTH } from '../keyword-nesting'; @@ -286,6 +287,15 @@ function isApiPostPathAllowed(path: string, allowlist: readonly string[]): boole interface PluginHttpPostOptions { headers?: Record; + /** + * Staged-attachment file id (the one `onBeforeBlobUpload` handed to the + * plugin) to report byte-level upload progress for. Only meaningful on a + * `Blob`/`File` body. Progress goes to the host-side registry the composer + * listens on (`lib/upload-progress.ts`), never back into the sandbox, so + * the attachment chip can show a real percentage while a plugin offloads + * the file. Reports for an id the composer isn't tracking are dropped. + */ + progressFileId?: string; } /** @@ -356,6 +366,21 @@ async function doHttpPost( headers['Authorization'] = client.getAuthHeader(); headers['X-JMAP-Username'] = client.getUsername(); } + + // Progress requires XMLHttpRequest: fetch() exposes no upload progress + // events (request streaming is Chrome-only), and the JMAP client's blob + // upload already made the same trade for the same reason (#333). Callers + // that don't ask for progress keep the fetch path untouched. + if (body instanceof Blob && options?.progressFileId !== undefined) { + const fileId = options.progressFileId; + if (typeof fileId !== 'string' || fileId.length === 0 || fileId.length > 128) { + throw new Error('progressFileId must be a non-empty string of at most 128 characters'); + } + return xhrPost(url.pathname + url.search, headers, body, (loaded, total) => + reportUploadProgress(fileId, loaded, total), + ); + } + const res = await apiFetch(url.pathname + url.search, { method: 'POST', headers, @@ -365,6 +390,37 @@ async function doHttpPost( return { ok: res.ok, status: res.status, data }; } +/** + * POST via XMLHttpRequest so `xhr.upload.onprogress` can report transferred + * bytes. Same shape of result as the fetch path in `doHttpPost`; the path is + * prefixed exactly like `apiFetch` does for a same-origin `/api/*` path. + */ +function xhrPost( + path: string, + headers: Record, + body: Blob, + onProgress: (loaded: number, total: number) => void, +): Promise<{ ok: boolean; status: number; data: unknown }> { + return new Promise((resolve, reject) => { + const xhr = new XMLHttpRequest(); + xhr.open('POST', getPathPrefix() + path); + for (const [name, value] of Object.entries(headers)) { + xhr.setRequestHeader(name, value); + } + xhr.upload.onprogress = (event) => { + if (event.lengthComputable) onProgress(event.loaded, event.total); + }; + xhr.onload = () => { + let data: unknown = null; + try { data = JSON.parse(xhr.responseText); } catch { /* non-JSON body */ } + resolve({ ok: xhr.status >= 200 && xhr.status < 300, status: xhr.status, data }); + }; + xhr.onerror = () => reject(new Error('Network error during plugin upload')); + xhr.ontimeout = () => reject(new Error('Plugin upload timed out')); + xhr.send(body); + }); +} + // ─── http.fetch (cross-origin, manifest-allowlisted) ────────── interface PluginFetchInit { diff --git a/lib/upload-progress.ts b/lib/upload-progress.ts new file mode 100644 index 000000000..8f299e60d --- /dev/null +++ b/lib/upload-progress.ts @@ -0,0 +1,42 @@ +/** + * Byte-level progress for staged-attachment uploads, keyed by the staged file + * id the composer hands to `onBeforeBlobUpload`. + * + * The composer cannot observe a plugin's own transfer directly: a plugin that + * offloads an attachment (see `isExternalAttachmentResult`) uploads the bytes + * itself via `api.http.post`, and the File it holds is a structured clone from + * the sandbox boundary, so object identity can't correlate that request back + * to an attachment. The staged file id can: the composer generated it, handed + * it to the hook, and the plugin passes it back via `progressFileId` on the + * post options. The host counts the bytes (see `doHttpPost`) and reports them + * here; the composer subscribes before running the hook. + * + * Progress never crosses into the sandbox. Reports for an id nobody listens to + * are dropped, so a plugin sending noise for ids it was never handed reaches + * nothing. + */ + +export type UploadProgressListener = (loaded: number, total: number) => void; + +const listeners = new Map(); + +/** + * Subscribe to progress reports for one staged file id. Returns the + * unsubscribe function. One listener per id: the composer owns the staged id, + * nothing else has a reason to listen. + */ +export function onUploadProgress(fileId: string, listener: UploadProgressListener): () => void { + listeners.set(fileId, listener); + return () => { + // Only clear our own registration, in case the id was re-registered. + if (listeners.get(fileId) === listener) listeners.delete(fileId); + }; +} + +/** Report transferred bytes for a staged file id. No listener, no effect. */ +export function reportUploadProgress(fileId: string, loaded: number, total: number): void { + const listener = listeners.get(fileId); + if (!listener) return; + if (!Number.isFinite(loaded) || !Number.isFinite(total)) return; + listener(loaded, total); +}