From 8e8f57661c2988f191eb685fab0d4a9f1957a6e3 Mon Sep 17 00:00:00 2001 From: David Nguyen Date: Wed, 25 Feb 2026 19:26:09 +1100 Subject: [PATCH] fix: refactors --- .../envelope-editor-fields-page-renderer.tsx | 2 +- .../envelope-signer-page-renderer.tsx | 2 +- .../get-envelope-item-image-by-token.ts | 4 ---- .../files/routes/get-envelope-item-image.ts | 13 ++++++++---- .../files/routes/get-envelope-item-meta.ts | 2 -- .../hooks/use-field-page-coords.ts | 7 +++++++ .../client-only/hooks/use-page-renderer.ts | 6 +++--- .../providers/envelope-render-provider.tsx | 20 ++----------------- .../internal/seal-document.handler.ts | 13 +++++++----- .../document-data/create-document-data.ts | 15 ++++++++++++-- packages/lib/types/document-data.ts | 2 -- .../lib/universal/upload/put-file.server.ts | 10 ++++------ .../lib/universal/upload/server-actions.ts | 5 +++-- packages/lib/utils/envelope-images.ts | 6 ++++++ .../pdf-viewer/envelope-pdf-viewer.tsx | 6 +----- .../pdf-viewer/pdf-viewer-states.tsx | 7 +------ 16 files changed, 59 insertions(+), 61 deletions(-) diff --git a/apps/remix/app/components/general/envelope-editor/envelope-editor-fields-page-renderer.tsx b/apps/remix/app/components/general/envelope-editor/envelope-editor-fields-page-renderer.tsx index 648a78a89..c09e9f3f7 100644 --- a/apps/remix/app/components/general/envelope-editor/envelope-editor-fields-page-renderer.tsx +++ b/apps/remix/app/components/general/envelope-editor/envelope-editor-fields-page-renderer.tsx @@ -62,7 +62,7 @@ export default function EnvelopeEditorFieldsPageRenderer({ editorFields.localFields.filter( (field) => field.page === pageNumber && field.envelopeItemId === currentEnvelopeItem?.id, ), - [editorFields.localFields, pageNumber], + [editorFields.localFields, pageNumber, currentEnvelopeItem?.id], ); const handleResizeOrMove = (event: KonvaEventObject) => { diff --git a/apps/remix/app/components/general/envelope-signing/envelope-signer-page-renderer.tsx b/apps/remix/app/components/general/envelope-signing/envelope-signer-page-renderer.tsx index 5dfc4b462..ae26fc5e0 100644 --- a/apps/remix/app/components/general/envelope-signing/envelope-signer-page-renderer.tsx +++ b/apps/remix/app/components/general/envelope-signing/envelope-signer-page-renderer.tsx @@ -98,7 +98,7 @@ export default function EnvelopeSignerPageRenderer({ pageData }: { pageData: Pag return fieldsToRender.filter( (field) => field.page === pageNumber && field.envelopeItemId === currentEnvelopeItem?.id, ); - }, [recipientFields, selectedAssistantRecipientFields, pageNumber]); + }, [recipientFields, selectedAssistantRecipientFields, pageNumber, currentEnvelopeItem?.id]); /** * Returns fields that have been fully signed by other recipients for this specific diff --git a/apps/remix/server/api/files/routes/get-envelope-item-image-by-token.ts b/apps/remix/server/api/files/routes/get-envelope-item-image-by-token.ts index 524d8904f..4d892be4e 100644 --- a/apps/remix/server/api/files/routes/get-envelope-item-image-by-token.ts +++ b/apps/remix/server/api/files/routes/get-envelope-item-image-by-token.ts @@ -51,10 +51,6 @@ route.get( return c.json({ error: 'Not found' }, 404); } - // We can hard cache this since since it's a unique URL for a given recipient. - // Might be dicey if the handler returns a cacheable error code. - c.header('Cache-Control', 'public, max-age=31536000, immutable'); - return await handleEnvelopeItemPageRequest({ c, envelopeItem, diff --git a/apps/remix/server/api/files/routes/get-envelope-item-image.ts b/apps/remix/server/api/files/routes/get-envelope-item-image.ts index aec552d3c..18db2797d 100644 --- a/apps/remix/server/api/files/routes/get-envelope-item-image.ts +++ b/apps/remix/server/api/files/routes/get-envelope-item-image.ts @@ -133,16 +133,17 @@ export const handleEnvelopeItemPageRequest = async ({ const documentDataToUse = version === 'current' ? envelopeItem.documentData.data : envelopeItem.documentData.initialData; - c.header('Content-Type', 'image/jpeg'); - c.header('Cache-Control', `${cacheStrategy}, max-age=31536000, immutable`); - // Return the image if it already exists in S3. if (envelopeItem.documentData.type === 'S3_PATH') { const s3Key = getEnvelopeItemPageImageS3Key(documentDataToUse, pageIndex); - const image = await UNSAFE_getS3File(s3Key); + const image = await UNSAFE_getS3File(s3Key).catch(() => null); if (image) { + // Note: Only set these headers on success. + c.header('Content-Type', 'image/jpeg'); + c.header('Cache-Control', `${cacheStrategy}, max-age=31536000, immutable`); + return c.body(image); } } @@ -169,6 +170,10 @@ export const handleEnvelopeItemPageRequest = async ({ return c.json({ error: 'Failed to render page to image' }, 500); } + // Note: Only set these headers on success. + c.header('Content-Type', 'image/jpeg'); + c.header('Cache-Control', `${cacheStrategy}, max-age=31536000, immutable`); + return c.body(image); }; diff --git a/apps/remix/server/api/files/routes/get-envelope-item-meta.ts b/apps/remix/server/api/files/routes/get-envelope-item-meta.ts index 085820e88..57bf4115a 100644 --- a/apps/remix/server/api/files/routes/get-envelope-item-meta.ts +++ b/apps/remix/server/api/files/routes/get-envelope-item-meta.ts @@ -110,12 +110,10 @@ export const handleEnvelopeItemsMetaRequest = async ({ const pdfPageMetadata: TDocumentDataMeta['pages'] = await extractAndStorePdfImages( new Uint8Array(pdfBytes).buffer, item.documentData.id, - item.documentData.type, ); pageMetadata = { pages: pdfPageMetadata, - documentDataType: item.documentData.type, }; } diff --git a/packages/lib/client-only/hooks/use-field-page-coords.ts b/packages/lib/client-only/hooks/use-field-page-coords.ts index 59d77b59e..ecb088194 100644 --- a/packages/lib/client-only/hooks/use-field-page-coords.ts +++ b/packages/lib/client-only/hooks/use-field-page-coords.ts @@ -64,13 +64,19 @@ export const useFieldPageCoords = ( const pageSelector = `${PDF_VIEWER_PAGE_SELECTOR}[data-page-number="${field.page}"]`; let resizeObserver: ResizeObserver | null = null; + let observedElement: HTMLElement | null = null; const attachResizeObserver = ($page: HTMLElement) => { + if ($page === observedElement) { + return; + } + resizeObserver?.disconnect(); resizeObserver = new ResizeObserver(() => { calculateCoords(); }); resizeObserver.observe($page); + observedElement = $page; }; // Try to attach immediately if the page already exists. @@ -99,6 +105,7 @@ export const useFieldPageCoords = ( return () => { mutationObserver.disconnect(); resizeObserver?.disconnect(); + observedElement = null; }; }, [calculateCoords, field.page]); diff --git a/packages/lib/client-only/hooks/use-page-renderer.ts b/packages/lib/client-only/hooks/use-page-renderer.ts index 232905bb2..0d3018fe3 100644 --- a/packages/lib/client-only/hooks/use-page-renderer.ts +++ b/packages/lib/client-only/hooks/use-page-renderer.ts @@ -8,7 +8,7 @@ import { EAGER_LOAD_PAGE_COUNT, type PageRenderData } from '../providers/envelop type RenderFunction = (props: { stage: Konva.Stage; pageLayer: Konva.Layer }) => void; -export function usePageRenderer(renderFunction: RenderFunction, pageData: PageRenderData) { +export const usePageRenderer = (renderFunction: RenderFunction, pageData: PageRenderData) => { const { pageWidth, pageHeight, scale, imageUrl, pageNumber } = pageData; const konvaContainer = useRef(null); @@ -70,7 +70,7 @@ export function usePageRenderer(renderFunction: RenderFunction, pageData: PageRe src: imageUrl, 'data-page-number': pageNumber, }), - [renderViewport, scaledViewport], + [renderViewport, scaledViewport, imageUrl], ); useEffect(() => { @@ -120,4 +120,4 @@ export function usePageRenderer(renderFunction: RenderFunction, pageData: PageRe renderStatus, setRenderStatus, }; -} +}; diff --git a/packages/lib/client-only/providers/envelope-render-provider.tsx b/packages/lib/client-only/providers/envelope-render-provider.tsx index e9ec9da75..43b7e499b 100644 --- a/packages/lib/client-only/providers/envelope-render-provider.tsx +++ b/packages/lib/client-only/providers/envelope-render-provider.tsx @@ -29,9 +29,6 @@ import { getEnvelopeItemMetaUrl, getEnvelopeItemPageImageUrl } from '../../utils */ export const EAGER_LOAD_PAGE_COUNT = 5; -// Todo: Embeds -export const PRESIGNED_ENVELOPE_ITEM_ID_PREFIX = 'PRESIGNED_'; - export type PageRenderData = BasePageRenderData & { scale: number; }; @@ -170,7 +167,7 @@ export const EnvelopeRenderProvider = ({ const fetchStartedAtRef = useRef(0); const envelopeItems = useMemo( - () => envelopeItemsFromProps.sort((a, b) => a.order - b.order), + () => [...envelopeItemsFromProps].sort((a, b) => a.order - b.order), [envelopeItemsFromProps], ); @@ -183,7 +180,7 @@ export const EnvelopeRenderProvider = ({ */ useEffect(() => { void fetchEnvelopeRenderData(); - }, [envelope.id, envelopeItems, token, version]); + }, [envelope.id, envelopeItems, token, version, presignToken]); const fetchEnvelopeRenderData = useCallback(async () => { if (envelopeItems.length === 0) { @@ -333,19 +330,6 @@ export const EnvelopeRenderProvider = ({ ); } - // Append all local embedding files. - const localFiles = envelopeItems.filter( - (item) => item.id.startsWith(PRESIGNED_ENVELOPE_ITEM_ID_PREFIX) && item.data, - ); - - for (const item of localFiles) { - if (!item.data) { - throw new Error('Not possible'); - } - - // Handle local files - } - setEnvelopeItemsMeta(metaMap); setEnvelopeItemsMetaLoadingState('loaded'); diff --git a/packages/lib/jobs/definitions/internal/seal-document.handler.ts b/packages/lib/jobs/definitions/internal/seal-document.handler.ts index 32b4af899..5d44b7147 100644 --- a/packages/lib/jobs/definitions/internal/seal-document.handler.ts +++ b/packages/lib/jobs/definitions/internal/seal-document.handler.ts @@ -491,11 +491,14 @@ const decorateAndSignPdf = async ({ // Add suffix based on document status const suffix = isRejected ? '_rejected.pdf' : '_signed.pdf'; - const newDocumentData = await putPdfFileServerSide({ - name: `${name}${suffix}`, - type: 'application/pdf', - arrayBuffer: async () => Promise.resolve(pdfBytes), - }); + const newDocumentData = await putPdfFileServerSide( + { + name: `${name}${suffix}`, + type: 'application/pdf', + arrayBuffer: async () => Promise.resolve(pdfBytes), + }, + envelopeItem.documentData.initialData, + ); return { oldDocumentDataId: envelopeItem.documentData.id, diff --git a/packages/lib/server-only/document-data/create-document-data.ts b/packages/lib/server-only/document-data/create-document-data.ts index 9cf2d7979..62757abcb 100644 --- a/packages/lib/server-only/document-data/create-document-data.ts +++ b/packages/lib/server-only/document-data/create-document-data.ts @@ -5,14 +5,25 @@ import { prisma } from '@documenso/prisma'; export type CreateDocumentDataOptions = { type: DocumentDataType; data: string; + + /** + * The initial data that was used to create the document data. + * + * If not provided, the current data will be used. + */ + initialData?: string; }; -export const createDocumentData = async ({ type, data }: CreateDocumentDataOptions) => { +export const createDocumentData = async ({ + type, + data, + initialData, +}: CreateDocumentDataOptions) => { return await prisma.documentData.create({ data: { type, data, - initialData: data, + initialData: initialData || data, }, }); }; diff --git a/packages/lib/types/document-data.ts b/packages/lib/types/document-data.ts index 497250ee4..683d8417e 100644 --- a/packages/lib/types/document-data.ts +++ b/packages/lib/types/document-data.ts @@ -1,9 +1,7 @@ -import { DocumentDataType } from '@prisma/client'; import { z } from 'zod'; export const ZDocumentDataMetaSchema = z.object({ // Could store other things such as PDF size, etc here. - documentDataType: z.nativeEnum(DocumentDataType), pages: z .object({ originalWidth: z.number().describe('Original PDF page width'), diff --git a/packages/lib/universal/upload/put-file.server.ts b/packages/lib/universal/upload/put-file.server.ts index 0bbb06b6b..262501e9b 100644 --- a/packages/lib/universal/upload/put-file.server.ts +++ b/packages/lib/universal/upload/put-file.server.ts @@ -26,7 +26,7 @@ type File = { * Uploads a document file to the appropriate storage location and creates * a document data record. */ -export const putPdfFileServerSide = async (file: File) => { +export const putPdfFileServerSide = async (file: File, initialData?: string) => { const isEncryptedDocumentsAllowed = false; // Was feature flag. const arrayBuffer = await file.arrayBuffer(); @@ -47,9 +47,9 @@ export const putPdfFileServerSide = async (file: File) => { const { type, data } = await putFileServerSide(file); - const newDocumentData = await createDocumentData({ type, data }); + const newDocumentData = await createDocumentData({ type, data, initialData }); - void extractAndStorePdfImages(arrayBuffer, newDocumentData.id, type).catch((err) => { + void extractAndStorePdfImages(arrayBuffer, newDocumentData.id).catch((err) => { console.error(`Error extracting and storing PDF images: ${err}`); // Do nothing. @@ -64,7 +64,6 @@ export const putPdfFileServerSide = async (file: File) => { export const extractAndStorePdfImages = async ( arrayBuffer: ArrayBuffer, documentDataId: string, - documentDataType: DocumentDataType, ): Promise => { const images = await pdfToImages(new Uint8Array(arrayBuffer)); @@ -78,7 +77,6 @@ export const extractAndStorePdfImages = async ( const documentDataMetadata = ZDocumentDataMetaSchema.parse({ pages: pageMetadata, - documentDataType, } satisfies TDocumentDataMeta); // Only update metadata (page dimensions). Never update type, data, or initialData: @@ -142,7 +140,7 @@ export const putNormalizedPdfFileServerSide = async ( data: documentData.data, }); - void extractAndStorePdfImages(normalized, newDocumentData.id, documentData.type).catch((err) => { + void extractAndStorePdfImages(normalized, newDocumentData.id).catch((err) => { console.error(`Error extracting and storing PDF images: ${err}`); // Do nothing. diff --git a/packages/lib/universal/upload/server-actions.ts b/packages/lib/universal/upload/server-actions.ts index 36bdaf737..f48bae3a8 100644 --- a/packages/lib/universal/upload/server-actions.ts +++ b/packages/lib/universal/upload/server-actions.ts @@ -129,7 +129,8 @@ export const deleteS3File = async (key: string) => { * frontend to ever pull a file from S3 directly. */ export const UNSAFE_getS3File = async (key: string) => { - // Additional safeguard to prevent path traversal. + // Basic safeguard to prevent path traversal. + // Key should never be user-controlled. if (key.includes('..') || key.startsWith('/')) { throw new Error('Invalid S3 key'); } @@ -143,7 +144,7 @@ export const UNSAFE_getS3File = async (key: string) => { }), ); - return response.Body; + return response.Body || null; }; const getS3Client = () => { diff --git a/packages/lib/utils/envelope-images.ts b/packages/lib/utils/envelope-images.ts index df2fb6d25..ada567c8f 100644 --- a/packages/lib/utils/envelope-images.ts +++ b/packages/lib/utils/envelope-images.ts @@ -73,5 +73,11 @@ export const getEnvelopeItemPageImageS3Key = ( const baseKey = documentDataId.split('/')[0]; + // Basic safeguard to prevent path traversal. + // Key should never be user-controlled. + if (baseKey.includes('..') || baseKey.startsWith('/')) { + throw new Error('Invalid S3 key'); + } + return `${baseKey}/${pageIndex}.jpeg`; }; diff --git a/packages/ui/components/pdf-viewer/envelope-pdf-viewer.tsx b/packages/ui/components/pdf-viewer/envelope-pdf-viewer.tsx index 79cbaa73c..97c1f2c89 100644 --- a/packages/ui/components/pdf-viewer/envelope-pdf-viewer.tsx +++ b/packages/ui/components/pdf-viewer/envelope-pdf-viewer.tsx @@ -123,11 +123,7 @@ export const EnvelopePdfViewer = ({ {/* No current item selected */} {envelopeItemsMetaLoadingState === 'loaded' && !currentEnvelopeItem && ( -
+

No document selected

diff --git a/packages/ui/components/pdf-viewer/pdf-viewer-states.tsx b/packages/ui/components/pdf-viewer/pdf-viewer-states.tsx index fc84f21fa..f6d0a2266 100644 --- a/packages/ui/components/pdf-viewer/pdf-viewer-states.tsx +++ b/packages/ui/components/pdf-viewer/pdf-viewer-states.tsx @@ -1,15 +1,10 @@ import { Trans } from '@lingui/react/macro'; -import { cn } from '@documenso/ui/lib/utils'; import { Spinner } from '@documenso/ui/primitives/spinner'; export const PdfViewerLoadingState = () => { return ( -
+
);