fix: separate resource previews from downloads
This commit is contained in:
123
src/App.test.tsx
123
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(
|
||||
<OneNoteApplication
|
||||
createWorkerClient={() => 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',
|
||||
|
||||
28
src/App.tsx
28
src/App.tsx
@@ -308,7 +308,7 @@ export function OneNoteApplication({
|
||||
}
|
||||
};
|
||||
|
||||
const loadSelectedResource = useCallback(
|
||||
const previewSelectedResource = useCallback(
|
||||
async (resourceId: string): Promise<OneNoteResourcePayloadDto> => {
|
||||
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<void> => {
|
||||
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}
|
||||
/>
|
||||
</div>
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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<ResourceWorkerResponse> {
|
||||
return this.getResource(sessionId, resourceId, 'preview', sectionId);
|
||||
}
|
||||
|
||||
getResourceDownload(
|
||||
sessionId: string,
|
||||
resourceId: string,
|
||||
sectionId?: string
|
||||
): Promise<ResourceWorkerResponse> {
|
||||
return this.getResource(sessionId, resourceId, 'download', sectionId);
|
||||
}
|
||||
|
||||
private getResource(
|
||||
sessionId: string,
|
||||
resourceId: string,
|
||||
intent: WorkerResourceIntent,
|
||||
sectionId?: string
|
||||
): Promise<ResourceWorkerResponse> {
|
||||
if (this.terminated) {
|
||||
return Promise.reject(
|
||||
@@ -173,6 +191,7 @@ export class OneNoteWorkerClient {
|
||||
requestId,
|
||||
sessionId,
|
||||
resourceId,
|
||||
intent,
|
||||
...(sectionId === undefined ? {} : { sectionId }),
|
||||
});
|
||||
});
|
||||
|
||||
@@ -168,14 +168,19 @@ async function handleRequest(data: WorkerRequest): Promise<void> {
|
||||
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,
|
||||
},
|
||||
});
|
||||
|
||||
@@ -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'),
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
| {
|
||||
|
||||
Reference in New Issue
Block a user