From 9c0604f849cf0fa00d5fe4f1a62533884003feab Mon Sep 17 00:00:00 2001 From: Matt Lewis Date: Wed, 4 Jun 2025 13:54:47 +0100 Subject: [PATCH] Fix issue with duplicate images (#30073) * ensure export file paths are unique * add unit test for filepath uniqueness. fix createMessagesRequest mock. * add return types --- src/utils/exportUtils/Exporter.ts | 29 +++++++++++++++- .../utils/exportUtils/HTMLExport-test.ts | 34 +++++++++++++++++-- 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/src/utils/exportUtils/Exporter.ts b/src/utils/exportUtils/Exporter.ts index 5409186941..4085126491 100644 --- a/src/utils/exportUtils/Exporter.ts +++ b/src/utils/exportUtils/Exporter.ts @@ -25,8 +25,17 @@ type BlobFile = { blob: Blob; }; +type FileDetails = { + directory: string; + name: string; + date: string; + extension: string; + count?: number; +}; + export default abstract class Exporter { protected files: BlobFile[] = []; + protected fileNames: Map = new Map(); protected cancelled = false; protected constructor( @@ -241,6 +250,19 @@ export default abstract class Exporter { return [fileName, "." + ext]; } + protected makeUniqueFilePath(details: FileDetails): string { + const makePath = ({ directory, name, date, extension, count = 0 }: FileDetails): string => + `${directory}/${name}-${date}${count > 0 ? ` (${count})` : ""}${extension}`; + const defaultPath = makePath(details); + const count = this.fileNames.get(defaultPath) || 0; + this.fileNames.set(defaultPath, count + 1); + if (count > 0) { + return makePath({ ...details, count }); + } + + return defaultPath; + } + public getFilePath(event: MatrixEvent): string { const mediaType = event.getContent().msgtype; let fileDirectory: string; @@ -263,7 +285,12 @@ export default abstract class Exporter { if (event.getType() === "m.sticker") fileExt = ".png"; if (isVoiceMessage(event)) fileExt = ".ogg"; - return fileDirectory + "/" + fileName + "-" + fileDate + fileExt; + return this.makeUniqueFilePath({ + directory: fileDirectory, + name: fileName, + date: fileDate, + extension: fileExt, + }); } protected isReply(event: MatrixEvent): boolean { diff --git a/test/unit-tests/utils/exportUtils/HTMLExport-test.ts b/test/unit-tests/utils/exportUtils/HTMLExport-test.ts index 6415efaf1d..91c4c3febd 100644 --- a/test/unit-tests/utils/exportUtils/HTMLExport-test.ts +++ b/test/unit-tests/utils/exportUtils/HTMLExport-test.ts @@ -104,8 +104,8 @@ describe("HTMLExport", () => { const chunk = events.slice(from, limit); return Promise.resolve({ chunk, - from: from.toString(), - to: (from + limit).toString(), + start: from.toString(), + end: (from + limit).toString(), }); }); } @@ -419,6 +419,36 @@ describe("HTMLExport", () => { expect(text).toBe(attachmentBody); }); + it("should handle attachments with identical names and dates", async () => { + mockMessages(EVENT_MESSAGE, EVENT_ATTACHMENT, EVENT_ATTACHMENT); + + const exporter = new HTMLExporter( + room, + ExportType.LastNMessages, + { + attachmentsIncluded: true, + maxSize: 1_024 * 1_024, + numberOfMessages: 40, + }, + () => {}, + ); + + await exporter.export(); + + const files = getFiles(exporter); + + // There should be 5 files: 2 attachments, 1 html file, 1 css file and 1 js file + expect(Object.keys(files)).toHaveLength(5); + + // Ensure that the attachment is present + const file = files[Object.keys(files).find((k) => k.endsWith(".txt"))!]; + expect(file).not.toBeUndefined(); + + // Ensure that the duplicate attachment has been uniquely named + const duplicateFile = files[Object.keys(files).find((k) => k.endsWith("(1).txt"))!]; + expect(duplicateFile).not.toBeUndefined(); + }); + it("should handle when attachment cannot be fetched", async () => { mockMessages(EVENT_MESSAGE, EVENT_ATTACHMENT_MALFORMED, EVENT_ATTACHMENT); const attachmentBody = "Lorem ipsum dolor sit amet";