From 628da73349c2a795b069aeb85c1c833e83632ff7 Mon Sep 17 00:00:00 2001 From: Bill Leoutsakos Date: Wed, 9 Sep 2026 11:02:19 -0700 Subject: [PATCH 1/2] fix(pptx): initialize isolated embedded PDF renderer --- .../pptx-renderer/utils/pdf-renderer.test.ts | 58 +++++++++++++++++++ .../lib/pptx-renderer/utils/pdf-renderer.ts | 55 +++++++++++------- 2 files changed, 93 insertions(+), 20 deletions(-) create mode 100644 apps/sim/lib/pptx-renderer/utils/pdf-renderer.test.ts diff --git a/apps/sim/lib/pptx-renderer/utils/pdf-renderer.test.ts b/apps/sim/lib/pptx-renderer/utils/pdf-renderer.test.ts new file mode 100644 index 00000000000..e17b57f8b11 --- /dev/null +++ b/apps/sim/lib/pptx-renderer/utils/pdf-renderer.test.ts @@ -0,0 +1,58 @@ +/** + * @vitest-environment node + */ +import { afterEach, describe, expect, it, vi } from 'vitest' +import { renderPdfToImage } from '@/lib/pptx-renderer/utils/pdf-renderer' + +afterEach(() => { + vi.unstubAllGlobals() + vi.restoreAllMocks() + vi.useRealTimers() +}) + +describe('renderPdfToImage', () => { + it('skips rendering when isolated canvas rendering is unavailable', async () => { + vi.stubGlobal('OffscreenCanvas', undefined) + expect(await renderPdfToImage(new Uint8Array([1]), 10, 10)).toBeNull() + }) + + it('supplies matching library and worker assets and preserves result/error behavior', async () => { + vi.useFakeTimers() + const postMessage = vi.fn() + const worker = { postMessage, onmessage: null as ((event: MessageEvent) => void) | null } + vi.stubGlobal('window', { location: { href: 'https://example.com/preview' } }) + /** Model the root-relative asset strings returned by the production bundler. */ + vi.spyOn(URL.prototype, 'toString').mockImplementation(function () { + return `/_next/static/media/${this.pathname.split('/').pop()}` + }) + vi.stubGlobal('OffscreenCanvas', class {}) + vi.stubGlobal( + 'Worker', + vi.fn(function Worker() { + return worker + }) + ) + vi.spyOn(URL, 'createObjectURL').mockReturnValue('blob:rendered-pdf') + const input = new Uint8Array([1, 2, 3]) + + const result = renderPdfToImage(input, 20, 10) + const [message, transfer] = postMessage.mock.calls[0] + expect(message).toMatchObject({ + width: 20, + height: 10, + pdfjsUrl: 'https://example.com/_next/static/media/pdf.min.mjs', + pdfjsWorkerUrl: 'https://example.com/_next/static/media/pdf.worker.min.mjs', + }) + expect(message.pdfData).toEqual(input) + expect(message.pdfData).not.toBe(input) + expect(transfer).toEqual([message.pdfData.buffer]) + worker.onmessage?.({ data: { id: message.id, blob: new Blob(['png']) } } as MessageEvent) + expect(await result).toBe('blob:rendered-pdf') + + const failed = renderPdfToImage(input, 20, 10) + worker.onmessage?.({ + data: { id: postMessage.mock.calls[1][0].id, error: 'Invalid PDF' }, + } as MessageEvent) + expect(await failed).toBeNull() + }) +}) diff --git a/apps/sim/lib/pptx-renderer/utils/pdf-renderer.ts b/apps/sim/lib/pptx-renderer/utils/pdf-renderer.ts index c51bfcaeab7..d80d00a9dfb 100644 --- a/apps/sim/lib/pptx-renderer/utils/pdf-renderer.ts +++ b/apps/sim/lib/pptx-renderer/utils/pdf-renderer.ts @@ -17,41 +17,46 @@ // Resolved pdfjs URL — computed once from main thread's module resolution -let _pdfjsUrl: string | null = null +let _pdfjsUrls: { library: string; worker: string } | null = null -function getPdfjsUrl(): string | null { - if (_pdfjsUrl !== null) return _pdfjsUrl +function getPdfjsUrls(): { library: string; worker: string } | null { + if (_pdfjsUrls !== null) return _pdfjsUrls try { - // Resolve via the bundler/dev server so the URL is usable from a Worker - _pdfjsUrl = new URL('pdfjs-dist/build/pdf.min.mjs', import.meta.url).toString() + const library = new URL('pdfjs-dist/build/pdf.min.mjs', import.meta.url).toString() + const worker = new URL('pdfjs-dist/build/pdf.worker.min.mjs', import.meta.url).toString() + /** Bundlers can emit root-relative asset URLs, which cannot resolve inside a blob worker. */ + _pdfjsUrls = { + library: new URL(library, window.location.href).href, + worker: new URL(worker, window.location.href).href, + } } catch { - _pdfjsUrl = '' + return null } - return _pdfjsUrl || null + return _pdfjsUrls } // Worker-based renderer (fully isolated from main thread pdfjs) /** * Inline source for the PDF render worker. - * Receives: { id, pdfData, width, height, pdfjsUrl } + * Receives: { id, pdfData, width, height, pdfjsUrl, pdfjsWorkerUrl } * Posts back: { id, blob } or { id, error } * * The worker loads its OWN pdfjs instance via dynamic import, so its static * PagesMapper state is completely independent of the main thread. - * pdfjs's own internal worker is disabled (workerPort = null, workerSrc = '') - * so pdfjs runs single-threaded inside this worker — acceptable for tiny - * 1-page EMF PDFs. + * Loading the matching worker module installs its WorkerMessageHandler in + * this isolated global scope. PDF.js then uses its in-context worker fallback + * without creating another worker or changing the host app's configuration. */ const WORKER_SRC = /* js */ ` let pdfjsLib = null; self.onmessage = async (e) => { - const { id, pdfData, width, height, pdfjsUrl } = e.data; + const { id, pdfData, width, height, pdfjsUrl, pdfjsWorkerUrl } = e.data; try { if (!pdfjsLib) { + await import(pdfjsWorkerUrl); pdfjsLib = await import(pdfjsUrl); - pdfjsLib.GlobalWorkerOptions.workerSrc = ''; } const doc = await pdfjsLib.getDocument({ data: pdfData }).promise; @@ -88,7 +93,7 @@ const _pending = new Map< { resolve: (b: Blob | null) => void; reject: (e: Error) => void } >() -function getWorker(_pdfjsUrl: string): Worker | null { +function getWorker(): Worker | null { if (_workerFailed) return null if (_worker) return _worker @@ -130,10 +135,10 @@ function renderInWorker( pdfData: Uint8Array, width: number, height: number, - pdfjsUrl: string + pdfjsUrls: { library: string; worker: string } ): Promise { return new Promise((resolve) => { - const worker = getWorker(pdfjsUrl) + const worker = getWorker() if (!worker) { resolve(null) return @@ -147,7 +152,17 @@ function renderInWorker( // Transfer the buffer to avoid copying const copy = pdfData.slice() // copy so caller retains original - worker.postMessage({ id, pdfData: copy, width, height, pdfjsUrl }, [copy.buffer]) + worker.postMessage( + { + id, + pdfData: copy, + width, + height, + pdfjsUrl: pdfjsUrls.library, + pdfjsWorkerUrl: pdfjsUrls.worker, + }, + [copy.buffer] + ) // Timeout: if worker doesn't respond in 15s, give up setTimeout(() => { @@ -175,14 +190,14 @@ export async function renderPdfToImage( width: number, height: number ): Promise { - const pdfjsUrl = getPdfjsUrl() + const pdfjsUrls = getPdfjsUrls() - if (!pdfjsUrl || typeof OffscreenCanvas === 'undefined' || typeof Worker === 'undefined') { + if (!pdfjsUrls || typeof OffscreenCanvas === 'undefined' || typeof Worker === 'undefined') { return null } try { - const blob = await renderInWorker(pdfData, width, height, pdfjsUrl) + const blob = await renderInWorker(pdfData, width, height, pdfjsUrls) if (blob) return URL.createObjectURL(blob) } catch { // Worker failed — no fallback, return null From cbeb95c613fb54d583b8319fe1eba772676549fd Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Wed, 9 Sep 2026 11:42:00 -0700 Subject: [PATCH 2/2] test(pptx): cover embedded PDF worker source directly Restore the resolve-once memoization for the pdfjs asset URLs so the failure path is not retried per embedded PDF, matching the documented intent of the cache. Execute WORKER_SRC in-process against a stand-in pdfjs so the fix itself is covered rather than only the message it posts. The worker template was previously an untested string, which is how the falsy workerSrc assignment shipped green. Asserts that the worker module is imported before the library and that GlobalWorkerOptions is never written to. Also fixes the canvas-guard test, which passed because window was undefined rather than because the guard fired, and adds the missing Worker guard case. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01GTTSjsX7CBMfXrdmq4PL85 --- .../pptx-renderer/utils/pdf-renderer.test.ts | 200 +++++++++++++++++- .../lib/pptx-renderer/utils/pdf-renderer.ts | 27 ++- 2 files changed, 210 insertions(+), 17 deletions(-) diff --git a/apps/sim/lib/pptx-renderer/utils/pdf-renderer.test.ts b/apps/sim/lib/pptx-renderer/utils/pdf-renderer.test.ts index e17b57f8b11..d692ec529c3 100644 --- a/apps/sim/lib/pptx-renderer/utils/pdf-renderer.test.ts +++ b/apps/sim/lib/pptx-renderer/utils/pdf-renderer.test.ts @@ -1,8 +1,24 @@ /** * @vitest-environment node */ -import { afterEach, describe, expect, it, vi } from 'vitest' -import { renderPdfToImage } from '@/lib/pptx-renderer/utils/pdf-renderer' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { renderPdfToImage, WORKER_SRC } from '@/lib/pptx-renderer/utils/pdf-renderer' + +const PAGE_URL = 'https://example.com/preview' +const LIBRARY_URL = 'https://example.com/_next/static/media/pdf.min.mjs' +const WORKER_URL = 'https://example.com/_next/static/media/pdf.worker.min.mjs' + +/** + * Reproduce the root-relative asset strings a production bundler emits for the + * two pdfjs assets, so the absolute-URL resolution is exercised rather than + * assumed. Only `toString()` is stubbed; the module resolves the final value + * through `href`. + */ +function stubBundlerAssetUrls() { + vi.spyOn(URL.prototype, 'toString').mockImplementation(function (this: URL) { + return `/_next/static/media/${this.pathname.split('/').pop()}` + }) +} afterEach(() => { vi.unstubAllGlobals() @@ -11,20 +27,29 @@ afterEach(() => { }) describe('renderPdfToImage', () => { + beforeEach(() => { + vi.stubGlobal('window', { location: { href: PAGE_URL } }) + stubBundlerAssetUrls() + }) + it('skips rendering when isolated canvas rendering is unavailable', async () => { + vi.stubGlobal('Worker', class {}) vi.stubGlobal('OffscreenCanvas', undefined) + expect(await renderPdfToImage(new Uint8Array([1]), 10, 10)).toBeNull() }) - it('supplies matching library and worker assets and preserves result/error behavior', async () => { + it('skips rendering when workers are unavailable', async () => { + vi.stubGlobal('OffscreenCanvas', class {}) + vi.stubGlobal('Worker', undefined) + + expect(await renderPdfToImage(new Uint8Array([1]), 10, 10)).toBeNull() + }) + + it('supplies absolute library and worker assets and preserves result/error behavior', async () => { vi.useFakeTimers() const postMessage = vi.fn() const worker = { postMessage, onmessage: null as ((event: MessageEvent) => void) | null } - vi.stubGlobal('window', { location: { href: 'https://example.com/preview' } }) - /** Model the root-relative asset strings returned by the production bundler. */ - vi.spyOn(URL.prototype, 'toString').mockImplementation(function () { - return `/_next/static/media/${this.pathname.split('/').pop()}` - }) vi.stubGlobal('OffscreenCanvas', class {}) vi.stubGlobal( 'Worker', @@ -40,8 +65,8 @@ describe('renderPdfToImage', () => { expect(message).toMatchObject({ width: 20, height: 10, - pdfjsUrl: 'https://example.com/_next/static/media/pdf.min.mjs', - pdfjsWorkerUrl: 'https://example.com/_next/static/media/pdf.worker.min.mjs', + pdfjsUrl: LIBRARY_URL, + pdfjsWorkerUrl: WORKER_URL, }) expect(message.pdfData).toEqual(input) expect(message.pdfData).not.toBe(input) @@ -56,3 +81,158 @@ describe('renderPdfToImage', () => { expect(await failed).toBeNull() }) }) + +interface WorkerScope { + onmessage: ((event: { data: Record }) => Promise) | null + postMessage: (message: Record) => void +} + +interface WorkerHarness { + imported: string[] + posted: Array> + globalWorkerOptions: Record + destroy: ReturnType + send: (data: Record) => Promise +} + +/** + * Execute {@link WORKER_SRC} in-process against a stand-in pdfjs. + * + * Node refuses dynamic `import()` inside `new Function` ("A dynamic import + * callback was not specified") and the `node:vm` hook needs + * `--experimental-vm-modules`, so the two import sites are redirected to an + * injected loader. The substitution count is asserted, so a source change that + * drops either import fails here rather than silently testing nothing. + */ +function runWorkerSource( + overrides: { pages?: number; getDocument?: () => { promise: Promise } } = {} +): WorkerHarness { + const imported: string[] = [] + const posted: Array> = [] + const globalWorkerOptions: Record = {} + const destroy = vi.fn() + + const page = { + getViewport: ({ scale }: { scale: number }) => ({ width: 100 * scale, height: 50 * scale }), + render: () => ({ promise: Promise.resolve() }), + } + const doc = { numPages: overrides.pages ?? 1, getPage: async () => page, destroy } + const library = { + GlobalWorkerOptions: globalWorkerOptions, + getDocument: overrides.getDocument ?? (() => ({ promise: Promise.resolve(doc) })), + } + + const load = async (url: string) => { + imported.push(url) + return url.includes('pdf.worker') ? {} : library + } + + class FakeOffscreenCanvas { + constructor( + public width: number, + public height: number + ) {} + getContext() { + return {} + } + async convertToBlob() { + return new Blob(['png']) + } + } + + expect(WORKER_SRC.split('await import(').length - 1).toBe(2) + const source = WORKER_SRC.replaceAll('await import(', 'await __load(') + + const scope: WorkerScope = { + onmessage: null, + postMessage: (message) => posted.push(message), + } + new Function('self', '__load', 'OffscreenCanvas', source)(scope, load, FakeOffscreenCanvas) + + return { + imported, + posted, + globalWorkerOptions, + destroy, + send: async (data) => { + await scope.onmessage?.({ data }) + }, + } +} + +describe('WORKER_SRC', () => { + it('imports the worker module before the library and renders a blob', async () => { + const harness = runWorkerSource() + + await harness.send({ + id: 7, + pdfData: new Uint8Array([1]), + width: 20, + height: 10, + pdfjsUrl: LIBRARY_URL, + pdfjsWorkerUrl: WORKER_URL, + }) + + expect(harness.imported).toEqual([WORKER_URL, LIBRARY_URL]) + expect(harness.posted).toHaveLength(1) + expect(harness.posted[0].id).toBe(7) + expect(harness.posted[0].blob).toBeInstanceOf(Blob) + expect(harness.destroy).toHaveBeenCalledOnce() + }) + + /** + * The original defect: pdfjs reads `workerSrc` through a getter that throws + * when falsy, outside its own try/catch, so assigning it here broke every + * render. The worker must leave pdfjs configuration untouched. + */ + it('never writes to GlobalWorkerOptions', async () => { + const harness = runWorkerSource() + + await harness.send({ + id: 1, + pdfData: new Uint8Array([1]), + width: 20, + height: 10, + pdfjsUrl: LIBRARY_URL, + pdfjsWorkerUrl: WORKER_URL, + }) + + expect(harness.globalWorkerOptions).toEqual({}) + expect('workerSrc' in harness.globalWorkerOptions).toBe(false) + }) + + it('reports an error instead of a blob when the document has no pages', async () => { + const harness = runWorkerSource({ pages: 0 }) + + await harness.send({ + id: 2, + pdfData: new Uint8Array([1]), + width: 20, + height: 10, + pdfjsUrl: LIBRARY_URL, + pdfjsWorkerUrl: WORKER_URL, + }) + + expect(harness.posted).toEqual([{ id: 2, error: 'no pages' }]) + expect(harness.destroy).toHaveBeenCalledOnce() + }) + + it('reports an error when pdfjs rejects', async () => { + const harness = runWorkerSource({ + getDocument: () => ({ promise: Promise.reject(new Error('Invalid PDF')) }), + }) + + await harness.send({ + id: 3, + pdfData: new Uint8Array([1]), + width: 20, + height: 10, + pdfjsUrl: LIBRARY_URL, + pdfjsWorkerUrl: WORKER_URL, + }) + + expect(harness.posted).toHaveLength(1) + expect(harness.posted[0].id).toBe(3) + expect(harness.posted[0].error).toContain('Invalid PDF') + }) +}) diff --git a/apps/sim/lib/pptx-renderer/utils/pdf-renderer.ts b/apps/sim/lib/pptx-renderer/utils/pdf-renderer.ts index d80d00a9dfb..3bb2eec6ecf 100644 --- a/apps/sim/lib/pptx-renderer/utils/pdf-renderer.ts +++ b/apps/sim/lib/pptx-renderer/utils/pdf-renderer.ts @@ -15,12 +15,19 @@ * fallback, no global state pollution. */ -// Resolved pdfjs URL — computed once from main thread's module resolution +// Resolved pdfjs URLs — computed once from main thread's module resolution -let _pdfjsUrls: { library: string; worker: string } | null = null +interface PdfjsUrls { + library: string + worker: string +} + +let _pdfjsUrls: PdfjsUrls | null = null +let _pdfjsUrlsResolved = false -function getPdfjsUrls(): { library: string; worker: string } | null { - if (_pdfjsUrls !== null) return _pdfjsUrls +function getPdfjsUrls(): PdfjsUrls | null { + if (_pdfjsUrlsResolved) return _pdfjsUrls + _pdfjsUrlsResolved = true try { const library = new URL('pdfjs-dist/build/pdf.min.mjs', import.meta.url).toString() const worker = new URL('pdfjs-dist/build/pdf.worker.min.mjs', import.meta.url).toString() @@ -30,7 +37,7 @@ function getPdfjsUrls(): { library: string; worker: string } | null { worker: new URL(worker, window.location.href).href, } } catch { - return null + _pdfjsUrls = null } return _pdfjsUrls } @@ -47,8 +54,14 @@ function getPdfjsUrls(): { library: string; worker: string } | null { * Loading the matching worker module installs its WorkerMessageHandler in * this isolated global scope. PDF.js then uses its in-context worker fallback * without creating another worker or changing the host app's configuration. + * + * Never assign `GlobalWorkerOptions.workerSrc` here: pdfjs reads it through a + * getter that throws when falsy, and the read happens outside its own + * try/catch, so a falsy assignment makes every `getDocument` call fail. + * + * @internal Exported so tests can execute this source directly. */ -const WORKER_SRC = /* js */ ` +export const WORKER_SRC = /* js */ ` let pdfjsLib = null; self.onmessage = async (e) => { @@ -135,7 +148,7 @@ function renderInWorker( pdfData: Uint8Array, width: number, height: number, - pdfjsUrls: { library: string; worker: string } + pdfjsUrls: PdfjsUrls ): Promise { return new Promise((resolve) => { const worker = getWorker()