Fix batch attachment download renaming and overwriting files

ts/signal.ts

@@ -89,6 +89,10 @@ type MigrationsModuleType = {
   getAbsoluteDraftPath: (path: string) => string;
   getAbsoluteStickerPath: (path: string) => string;
   getAbsoluteTempPath: (path: string) => string;
+  getUnusedFilename: (options: {
+    filename: string;
+    baseDir?: string;
+  }) => string;
   loadAttachmentData: (
     attachment: Partial<AttachmentType>
   ) => Promise<AttachmentWithHydratedData>;
@@ -173,6 +177,7 @@ export function initializeMigrations({
     getStickersPath,
     getBadgesPath,
     getTempPath,
+    getUnusedFilename,
     readAndDecryptDataFromDisk,
     saveAttachmentToDisk,
   } = Attachments;
@@ -304,6 +309,7 @@ export function initializeMigrations({
     getAbsoluteDraftPath,
     getAbsoluteStickerPath,
     getAbsoluteTempPath,
+    getUnusedFilename,
     loadAttachmentData,
     loadContactData,
     loadMessage: MessageType.createAttachmentLoader(loadAttachmentData),
@@ -404,6 +410,10 @@ type AttachmentsModuleType = {
   ) => (relativePath: string) => string;
 
   createDoesExist: (root: string) => (relativePath: string) => Promise<boolean>;
+  getUnusedFilename: (options: {
+    filename: string;
+    baseDir?: string;
+  }) => string;
   saveAttachmentToDisk: ({
     data,
     name,

ts/state/ducks/conversations.ts

@@ -4054,12 +4054,13 @@ function saveAttachment(
       return;
     }
 
-    const { readAttachmentData, saveAttachmentToDisk } =
+    const { getUnusedFilename, readAttachmentData, saveAttachmentToDisk } =
       window.Signal.Migrations;
 
     const fullPath = await Attachment.save({
       attachment,
       index: index + 1,
+      getUnusedFilename,
       readAttachmentData,
       saveAttachmentToDisk,
       timestamp,
@@ -4120,43 +4121,56 @@ function saveAttachments(
       return;
     }
 
-    const { readAttachmentData, saveAttachmentToDisk } =
+    const { getUnusedFilename, readAttachmentData, saveAttachmentToDisk } =
       window.Signal.Migrations;
 
     let fullPath;
     let index = 0;
-    for (const attachment of attachments) {
-      index += 1;
+    try {
+      for (const attachment of attachments) {
+        index += 1;
 
-      // eslint-disable-next-line no-await-in-loop
-      const result = await Attachment.save({
-        attachment,
-        index,
-        readAttachmentData,
-        saveAttachmentToDisk,
-        timestamp,
-        baseDir: dirPath,
-      });
+        // eslint-disable-next-line no-await-in-loop
+        const result = await Attachment.save({
+          attachment,
+          index,
+          getUnusedFilename,
+          readAttachmentData,
+          saveAttachmentToDisk,
+          timestamp,
+          baseDir: dirPath,
+        });
 
-      if (fullPath === undefined) {
-        fullPath = result;
+        if (fullPath === undefined) {
+          fullPath = result;
+        }
       }
-    }
 
-    if (fullPath == null) {
-      throw new Error('saveAttachments: Returned path to attachment is null!');
-    }
+      if (fullPath == null) {
+        throw new Error(
+          'saveAttachments: Returned path to attachment is null!'
+        );
+      }
 
-    dispatch({
-      type: SHOW_TOAST,
-      payload: {
-        toastType: ToastType.FileSaved,
-        parameters: {
-          countOfFiles: attachments.length,
-          fullPath,
+      dispatch({
+        type: SHOW_TOAST,
+        payload: {
+          toastType: ToastType.FileSaved,
+          parameters: {
+            countOfFiles: attachments.length,
+            fullPath,
+          },
         },
-      },
-    });
+      });
+    } catch (e) {
+      log.error('Error in saveAttachments():', Errors.toLogFormat(e));
+      dispatch({
+        type: SHOW_TOAST,
+        payload: {
+          toastType: ToastType.Error,
+        },
+      });
+    }
   };
 }
 

ts/test-node/types/Attachment_test.ts

@@ -120,7 +120,7 @@ describe('Attachment', () => {
           timestamp,
           index: 2,
         });
-        const expected = 'signal-1970-01-02-000000_002.mov';
+        const expected = 'funny-cat.mov';
         assert.strictEqual(actual, expected);
       });
 

ts/types/Attachment.ts

@@ -1034,6 +1034,7 @@ export const isVoiceMessage = (
 export const save = async ({
   attachment,
   index,
+  getUnusedFilename,
   readAttachmentData,
   saveAttachmentToDisk,
   timestamp,
@@ -1041,6 +1042,10 @@ export const save = async ({
 }: {
   attachment: AttachmentType;
   index?: number;
+  getUnusedFilename: (options: {
+    filename: string;
+    baseDir?: string;
+  }) => string;
   readAttachmentData: (
     attachment: Partial<AddressableAttachmentType>
   ) => Promise<Uint8Array>;
@@ -1065,7 +1070,17 @@ export const save = async ({
     throw new Error('Attachment had neither path nor data');
   }
 
-  const name = getSuggestedFilename({ attachment, timestamp, index });
+  const suggestedFilename = getSuggestedFilename({
+    attachment,
+    timestamp,
+    index,
+  });
+
+  /**
+   * When baseDir is provided, saveAttachmentToDisk() will save without prompting
+   * and may overwrite existing files, so we need to append a suffix
+   */
+  const name = getUnusedFilename({ filename: suggestedFilename, baseDir });
 
   const result = await saveAttachmentToDisk({
     data,
@@ -1090,7 +1105,7 @@ export const getSuggestedFilename = ({
   index?: number;
 }): string => {
   const { fileName } = attachment;
-  if (fileName && (!isNumber(index) || index === 1)) {
+  if (fileName) {
     return fileName;
   }
 

ts/windows/attachments.ts

@@ -3,7 +3,8 @@
 
 import { ipcRenderer } from 'electron';
 import { isString, isTypedArray } from 'lodash';
-import { join, normalize, basename } from 'path';
+import { join, normalize, basename, parse as pathParse } from 'path';
+import { existsSync } from 'fs';
 import fse from 'fs-extra';
 import { v4 as getGuid } from 'uuid';
 
@@ -11,6 +12,8 @@ import { isPathInside } from '../util/isPathInside';
 import { writeWindowsZoneIdentifier } from '../util/windowsZoneIdentifier';
 import OS from '../util/os/osMain';
 import { getRelativePath, createName } from '../util/attachmentPath';
+import { toHex } from '../Bytes';
+import { getRandomBytes } from '../Crypto';
 
 export * from '../util/ensureAttachmentIsReencryptable';
 export * from '../../app/attachments';
@@ -19,6 +22,8 @@ type FSAttrType = {
   set: (path: string, attribute: string, value: string) => Promise<void>;
 };
 
+const GET_UNUSED_FILENAME_MAX_ATTEMPTS = 100;
+
 let xattr: FSAttrType | undefined;
 
 try {
@@ -267,3 +272,33 @@ export const saveAttachmentToDisk = async ({
     name: fileBasename,
   };
 };
+
+export const getUnusedFilename = ({
+  filename,
+  baseDir,
+}: {
+  filename: string;
+  baseDir?: string;
+}): string => {
+  if (baseDir == null || !existsSync(join(baseDir, filename))) {
+    return filename;
+  }
+
+  const { ext, name: mainFilename } = pathParse(filename);
+  for (let n = 1; n < GET_UNUSED_FILENAME_MAX_ATTEMPTS; n += 1) {
+    const nextFilename = `${mainFilename}-${n}${ext}`;
+    if (!existsSync(join(baseDir, nextFilename))) {
+      return nextFilename;
+    }
+  }
+
+  const randomSuffix = toHex(getRandomBytes(4));
+  const randomNextFilename = `${mainFilename}-${randomSuffix}${ext}`;
+  if (!existsSync(join(baseDir, randomNextFilename))) {
+    return randomNextFilename;
+  }
+
+  throw new Error(
+    'getUnusedFilename() failed to get next unused filename after max attempts'
+  );
+};