From e6a8111d51953662b94872e26c4cff0438c262a5 Mon Sep 17 00:00:00 2001 From: Albrecht Degering Date: Wed, 22 Jul 2026 20:06:18 +0200 Subject: [PATCH] fix: separate resource previews from downloads --- src/App.test.tsx | 123 ++++++++++++++++++++++++++---- src/App.tsx | 28 +++++-- src/worker/onenote.client.test.ts | 44 ++++++++++- src/worker/onenote.client.ts | 21 ++++- src/worker/onenote.worker.ts | 9 ++- src/worker/parser-adapter.test.ts | 41 +++++++++- src/worker/parser-adapter.ts | 16 +++- src/worker/worker-protocol.ts | 3 + 8 files changed, 257 insertions(+), 28 deletions(-) diff --git a/src/App.test.tsx b/src/App.test.tsx index ed0caab..ec53044 100644 --- a/src/App.test.tsx +++ b/src/App.test.tsx @@ -53,12 +53,17 @@ class SuccessfulParserWorker implements WorkerPort { onerror: ((event: ErrorEvent) => void) | null = null; terminated = false; lastMessage: WorkerRequest | undefined; + readonly messages: WorkerRequest[] = []; lastTransfer: Transferable[] = []; - constructor(private readonly packageResult?: OneNotePackageDto) {} + constructor( + private readonly packageResult?: OneNotePackageDto, + private readonly resourcePage = false + ) {} postMessage(message: WorkerRequest, transfer: Transferable[] = []): void { this.lastMessage = message; + this.messages.push(message); this.lastTransfer = transfer; queueMicrotask(() => { if (message.type === 'parse-one') { @@ -84,27 +89,67 @@ class SuccessfulParserWorker implements WorkerPort { page: { ...page, id: message.pageId, - blocks: [ - { - kind: 'text', - id: 'text-1', - sourceText: page.text, - text: page.text, - runs: [ + blocks: this.resourcePage + ? [ { - start: 0, - end: page.text.length, + kind: 'image', + id: 'image-1', + resourceId: 'resource-image', + filename: 'large.png', + isBackground: false, + layout: {}, + }, + ] + : [ + { + kind: 'text', + id: 'text-1', + sourceText: page.text, text: page.text, - style: {}, + runs: [ + { + start: 0, + end: page.text.length, + text: page.text, + style: {}, + }, + ], + paragraphStyle: {}, + layout: {}, }, ], - paragraphStyle: {}, - layout: {}, - }, - ], diagnostics: [], }, }); + } else if (message.type === 'get-resource') { + if (message.intent === 'preview') { + this.respond({ + type: 'failure', + requestId: message.requestId, + error: { + code: 'resource-not-available', + message: + 'This image can be downloaded but is not approved for automatic preview.', + recoverable: true, + }, + }); + } else { + this.respond({ + type: 'resource', + requestId: message.requestId, + sessionId: message.sessionId, + resource: { + id: message.resourceId, + kind: 'image', + filename: 'large.png', + extension: 'png', + mediaType: 'image/png', + browserRenderable: false, + size: 4, + bytes: Uint8Array.of(1, 2, 3, 4).buffer, + }, + }); + } } else if (message.type === 'export-session') { this.respond({ type: 'export-artifact', @@ -231,6 +276,54 @@ describe('OneNoteApplication', () => { expect(click).toHaveBeenCalledOnce(); }); + it('requests a rejected preview separately from an explicit image download', async () => { + const user = userEvent.setup(); + const worker = new SuccessfulParserWorker(undefined, true); + vi.spyOn(URL, 'createObjectURL').mockReturnValue('blob:local-image'); + const click = vi + .spyOn(HTMLAnchorElement.prototype, 'click') + .mockImplementation(() => undefined); + render( + new OneNoteWorkerClient(() => worker)} + /> + ); + + await user.upload( + screen.getByLabelText('Choose OneNote files'), + new File([new Uint8Array([1, 2, 3])], 'Section.one') + ); + + expect( + await screen.findByText(/not approved for automatic preview/i) + ).toBeVisible(); + expect(worker.messages).toContainEqual( + expect.objectContaining({ + type: 'get-resource', + resourceId: 'resource-image', + intent: 'preview', + }) + ); + expect( + worker.messages.some( + (message) => + message.type === 'get-resource' && message.intent === 'download' + ) + ).toBe(false); + + await user.click( + screen.getByRole('button', { name: 'Download original image' }) + ); + expect(worker.messages).toContainEqual( + expect.objectContaining({ + type: 'get-resource', + resourceId: 'resource-image', + intent: 'download', + }) + ); + expect(click).toHaveBeenCalledOnce(); + }); + it('opens a dropped OneNote package through the local worker', async () => { const notebook: OneNotePackageDto = { format: 'onepkg', diff --git a/src/App.tsx b/src/App.tsx index 9ad58ea..3aca327 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -308,7 +308,7 @@ export function OneNoteApplication({ } }; - const loadSelectedResource = useCallback( + const previewSelectedResource = useCallback( async (resourceId: string): Promise => { const client = activeClient.current; if (!client || state.phase !== 'loaded') { @@ -317,7 +317,7 @@ export function OneNoteApplication({ 'The local parser session is no longer available.' ); } - const response = await client.getResource( + const response = await client.getResourcePreview( state.source.sessionId, resourceId, state.source.kind === 'onepkg' ? selectedSectionId : undefined @@ -335,7 +335,25 @@ export function OneNoteApplication({ const downloadSelectedResource = useCallback( async (resourceId: string, suggestedName?: string): Promise => { - const resource = await loadSelectedResource(resourceId); + const client = activeClient.current; + if (!client || state.phase !== 'loaded') { + throw new WorkerClientError( + 'session-not-available', + 'The local parser session is no longer available.' + ); + } + const response = await client.getResourceDownload( + state.source.sessionId, + resourceId, + state.source.kind === 'onepkg' ? selectedSectionId : undefined + ); + if (response.type === 'failure') { + throw new WorkerClientError( + response.error.code, + response.error.message + ); + } + const resource = response.resource; const filename = safeDownloadFilename( resource.filename ?? suggestedName ?? @@ -343,7 +361,7 @@ export function OneNoteApplication({ ); downloadBrowserFile(resource.bytes, resource.mediaType, filename); }, - [loadSelectedResource] + [selectedSectionId, state] ); const exportLoadedSource = useCallback( @@ -464,7 +482,7 @@ export function OneNoteApplication({ error={ pageState.phase === 'error' ? pageState.message : undefined } - loadResource={loadSelectedResource} + previewResource={previewSelectedResource} downloadResource={downloadSelectedResource} /> diff --git a/src/worker/onenote.client.test.ts b/src/worker/onenote.client.test.ts index 91eb53f..90144da 100644 --- a/src/worker/onenote.client.test.ts +++ b/src/worker/onenote.client.test.ts @@ -114,19 +114,59 @@ describe('OneNoteWorkerClient', () => { ); }); - it('requests a lazy resource from the retained parser session', async () => { + it('marks an automatic resource request as preview-only', async () => { const worker = new FakeWorker(); const client = new OneNoteWorkerClient(() => worker); - const result = client.getResource('session-1', 'resource-2', 'section-3'); + const result = client.getResourcePreview( + 'session-1', + 'resource-2', + 'section-3' + ); expect(worker.lastMessage).toEqual({ type: 'get-resource', requestId: 'resource-1', sessionId: 'session-1', resourceId: 'resource-2', + intent: 'preview', sectionId: 'section-3', }); + const bytes = Uint8Array.of(1, 2, 3).buffer; + worker.respond({ + type: 'resource', + requestId: 'resource-1', + sessionId: 'session-1', + resource: { + id: 'resource-2', + kind: 'image', + filename: 'image.png', + mediaType: 'image/png', + browserRenderable: true, + size: 3, + bytes, + }, + }); + + await expect(result).resolves.toMatchObject({ + type: 'resource', + resource: { bytes }, + }); + }); + + it('marks an explicit resource request as a download', async () => { + const worker = new FakeWorker(); + const client = new OneNoteWorkerClient(() => worker); + const result = client.getResourceDownload('session-1', 'resource-2'); + + expect(worker.lastMessage).toEqual({ + type: 'get-resource', + requestId: 'resource-1', + sessionId: 'session-1', + resourceId: 'resource-2', + intent: 'download', + }); + const bytes = Uint8Array.of(1, 2, 3).buffer; worker.respond({ type: 'resource', diff --git a/src/worker/onenote.client.ts b/src/worker/onenote.client.ts index 7029cfc..408f946 100644 --- a/src/worker/onenote.client.ts +++ b/src/worker/onenote.client.ts @@ -8,6 +8,7 @@ import type { WorkerCabinetPart, WorkerExportFormat, WorkerFailure, + WorkerResourceIntent, WorkerResponse, } from './worker-protocol.js'; @@ -148,10 +149,27 @@ export class OneNoteWorkerClient { }); } - getResource( + getResourcePreview( sessionId: string, resourceId: string, sectionId?: string + ): Promise { + return this.getResource(sessionId, resourceId, 'preview', sectionId); + } + + getResourceDownload( + sessionId: string, + resourceId: string, + sectionId?: string + ): Promise { + return this.getResource(sessionId, resourceId, 'download', sectionId); + } + + private getResource( + sessionId: string, + resourceId: string, + intent: WorkerResourceIntent, + sectionId?: string ): Promise { if (this.terminated) { return Promise.reject( @@ -173,6 +191,7 @@ export class OneNoteWorkerClient { requestId, sessionId, resourceId, + intent, ...(sectionId === undefined ? {} : { sectionId }), }); }); diff --git a/src/worker/onenote.worker.ts b/src/worker/onenote.worker.ts index 47400c3..c54c21b 100644 --- a/src/worker/onenote.worker.ts +++ b/src/worker/onenote.worker.ts @@ -168,14 +168,19 @@ async function handleRequest(data: WorkerRequest): Promise { const resource = session.resources.get( workerResourceKey(data.resourceId, data.sectionId) ); - const payload = resource ? resourcePayload(resource) : undefined; + const payload = resource + ? resourcePayload(resource, data.intent) + : undefined; if (!payload) { workerScope.postMessage({ type: 'failure', requestId: data.requestId, error: { code: 'resource-not-available', - message: 'The selected embedded resource is not available.', + message: + data.intent === 'preview' && resource + ? 'This image can be downloaded but is not approved for automatic preview.' + : 'The selected embedded resource is not available.', recoverable: true, }, }); diff --git a/src/worker/parser-adapter.test.ts b/src/worker/parser-adapter.test.ts index 7a02dfb..16a1459 100644 --- a/src/worker/parser-adapter.test.ts +++ b/src/worker/parser-adapter.test.ts @@ -43,7 +43,7 @@ describe('worker parser adapter', () => { expect(result.resources).toHaveLength(1); const resource = [...result.resources.values()][0]!; expect(result.resources.get(workerResourceKey(resource.id))).toBe(resource); - const payload = resourcePayload(resource); + const payload = resourcePayload(resource, 'preview'); expect(payload).toMatchObject({ kind: 'image', mediaType: 'image/png', @@ -55,6 +55,45 @@ describe('worker parser adapter', () => { ); }); + it('rejects unsafe previews before copying while preserving explicit downloads', () => { + const source = Uint8Array.of(1, 2, 3, 4); + const rejected = { + id: 'rejected-image', + kind: 'image' as const, + sourceJcid: 1, + extension: 'png', + mediaType: 'image/png', + browserRenderable: false, + size: source.byteLength, + blob: { source, offset: 0, size: source.byteLength }, + availability: 'available' as const, + }; + + expect(resourcePayload(rejected, 'preview')).toBeUndefined(); + expect(resourcePayload(rejected, 'download')).toMatchObject({ + id: 'rejected-image', + browserRenderable: false, + bytes: source.buffer, + }); + }); + + it('rejects an oversized preview before allocating its declared payload', () => { + const source = Uint8Array.of(0x89, 0x50, 0x4e, 0x47); + const oversized = { + id: 'oversized-image', + kind: 'image' as const, + sourceJcid: 1, + extension: 'png', + mediaType: 'image/png', + browserRenderable: true, + size: 32 * 1024 * 1024 + 1, + blob: { source, offset: 0, size: 32 * 1024 * 1024 + 1 }, + availability: 'available' as const, + }; + + expect(resourcePayload(oversized, 'preview')).toBeUndefined(); + }); + it('opens a real ONEPKG and addresses page IDs without rewriting them', async () => { const result = await openPackageForWorker( fixtureBytes('../../tests/fixtures/joplin-test.onepkg.base64'), diff --git a/src/worker/parser-adapter.ts b/src/worker/parser-adapter.ts index c88e5f6..262693c 100644 --- a/src/worker/parser-adapter.ts +++ b/src/worker/parser-adapter.ts @@ -22,7 +22,8 @@ import { type CabSetPart, type OneNotePackageOpenOptions, } from '../onenote/onepkg/index.js'; -import type { WorkerFailure } from './worker-protocol.js'; +import { DEFAULT_BROWSER_IMAGE_SAFETY_LIMITS } from '../onenote/image-safety.js'; +import type { WorkerFailure, WorkerResourceIntent } from './worker-protocol.js'; export interface ParsedWorkerSection { section: OneNoteSectionDto; @@ -95,10 +96,21 @@ export function workerResourceKey( } export function resourcePayload( - resource: OneNoteResource + resource: OneNoteResource, + intent: WorkerResourceIntent ): OneNoteResourcePayloadDto | undefined { const blob = resource.blob; if (resource.availability !== 'available' || !blob) return undefined; + if ( + intent === 'preview' && + (resource.kind !== 'image' || + !resource.browserRenderable || + (resource.mediaType !== 'image/png' && + resource.mediaType !== 'image/jpeg') || + blob.size > DEFAULT_BROWSER_IMAGE_SAFETY_LIMITS.maxBytes) + ) { + return undefined; + } const bytes = new ArrayBuffer(blob.size); new Uint8Array(bytes).set( blob.source.subarray(blob.offset, blob.offset + blob.size) diff --git a/src/worker/worker-protocol.ts b/src/worker/worker-protocol.ts index f625535..5d0f05b 100644 --- a/src/worker/worker-protocol.ts +++ b/src/worker/worker-protocol.ts @@ -13,6 +13,8 @@ export interface WorkerCabinetPart { export type WorkerExportFormat = 'static-zip' | 'json' | 'text' | 'markdown' | 'html'; +export type WorkerResourceIntent = 'preview' | 'download'; + export interface WorkerExportArtifact { filename: string; mediaType: string; @@ -45,6 +47,7 @@ export type WorkerRequest = requestId: string; sessionId: string; resourceId: string; + intent: WorkerResourceIntent; sectionId?: string; } | {