From 9fa3359477b37adf74013279dc25425879c3bb4e Mon Sep 17 00:00:00 2001
From: Fedor Indutny <79877362+indutny-signal@users.noreply.github.com>
Date: Mon, 22 Mar 2021 11:51:53 -0700
Subject: [PATCH] Mark attachment as corrupted if audio load failed

Sending corrupted audio should not leave user with non-functional
UI. Mark attachment as corrupted and show generic attachment UI for it
instead.
---
 .../conversation/Message.stories.tsx          |  1 +
 ts/components/conversation/Message.tsx        | 12 ++++++
 ts/components/conversation/MessageAudio.tsx   | 19 ++++++++-
 .../conversation/MessageDetail.stories.tsx    |  1 +
 ts/components/conversation/Quote.stories.tsx  |  1 +
 .../conversation/Timeline.stories.tsx         |  1 +
 .../conversation/TimelineItem.stories.tsx     |  1 +
 ts/models/messages.ts                         | 41 +++++++++++++++++++
 ts/state/smart/MessageAudio.tsx               |  1 +
 ts/types/Attachment.ts                        |  2 +
 ts/util/lint/exceptions.json                  |  6 +--
 ts/views/conversation_view.ts                 | 17 ++++++++
 12 files changed, 98 insertions(+), 5 deletions(-)

diff --git a/ts/components/conversation/Message.stories.tsx b/ts/components/conversation/Message.stories.tsx
index 81b4a69e5..0d02671d2 100644
--- a/ts/components/conversation/Message.stories.tsx
+++ b/ts/components/conversation/Message.stories.tsx
@@ -111,6 +111,7 @@ const createProps = (overrideProps: Partial<Props> = {}): Props => ({
   isTapToViewError: overrideProps.isTapToViewError,
   isTapToViewExpired: overrideProps.isTapToViewExpired,
   kickOffAttachmentDownload: action('kickOffAttachmentDownload'),
+  markAttachmentAsCorrupted: action('markAttachmentAsCorrupted'),
   openConversation: action('openConversation'),
   openLink: action('openLink'),
   previews: overrideProps.previews || [],
diff --git a/ts/components/conversation/Message.tsx b/ts/components/conversation/Message.tsx
index fc4dc740f..936a6cd03 100644
--- a/ts/components/conversation/Message.tsx
+++ b/ts/components/conversation/Message.tsx
@@ -90,6 +90,7 @@ export type AudioAttachmentProps = {
   withContentBelow: boolean;
 
   kickOffAttachmentDownload(): void;
+  onCorrupted(): void;
 };
 
 export type PropsData = {
@@ -185,6 +186,10 @@ export type PropsActions = {
     attachment: AttachmentType;
     messageId: string;
   }) => void;
+  markAttachmentAsCorrupted: (options: {
+    attachment: AttachmentType;
+    messageId: string;
+  }) => void;
   showVisualAttachment: (options: {
     attachment: AttachmentType;
     messageId: string;
@@ -686,6 +691,7 @@ export class Message extends React.PureComponent<Props, State> {
       i18n,
       id,
       kickOffAttachmentDownload,
+      markAttachmentAsCorrupted,
       quote,
       showVisualAttachment,
       isSticker,
@@ -773,6 +779,12 @@ export class Message extends React.PureComponent<Props, State> {
             messageId: id,
           });
         },
+        onCorrupted() {
+          markAttachmentAsCorrupted({
+            attachment: firstAttachment,
+            messageId: id,
+          });
+        },
       });
     }
     const { pending, fileName, fileSize, contentType } = firstAttachment;
diff --git a/ts/components/conversation/MessageAudio.tsx b/ts/components/conversation/MessageAudio.tsx
index 7f6861764..7da1ed422 100644
--- a/ts/components/conversation/MessageAudio.tsx
+++ b/ts/components/conversation/MessageAudio.tsx
@@ -25,6 +25,7 @@ export type Props = {
 
   buttonRef: React.RefObject<HTMLButtonElement>;
   kickOffAttachmentDownload(): void;
+  onCorrupted(): void;
 
   activeAudioID: string | undefined;
   setActiveAudioID: (id: string | undefined) => void;
@@ -208,6 +209,7 @@ export const MessageAudio: React.FC<Props> = (props: Props) => {
 
     buttonRef,
     kickOffAttachmentDownload,
+    onCorrupted,
 
     audio,
     audioContext,
@@ -275,14 +277,27 @@ export const MessageAudio: React.FC<Props> = (props: Props) => {
         setPeaks(newPeaks);
         setDuration(Math.max(newDuration, 1e-23));
       } catch (err) {
-        window.log.error('MessageAudio: loadAudio error', err);
+        window.log.error(
+          'MessageAudio: loadAudio error, marking as corrupted',
+          err
+        );
+
+        onCorrupted();
       }
     })();
 
     return () => {
       canceled = true;
     };
-  }, [attachment, audioContext, setDuration, setPeaks, state, waveformCache]);
+  }, [
+    attachment,
+    audioContext,
+    setDuration,
+    setPeaks,
+    onCorrupted,
+    state,
+    waveformCache,
+  ]);
 
   // This effect attaches/detaches event listeners to the global <audio/>
   // instance that we reuse from the GlobalAudioContext.
diff --git a/ts/components/conversation/MessageDetail.stories.tsx b/ts/components/conversation/MessageDetail.stories.tsx
index c1d96bb4d..0675d2f22 100644
--- a/ts/components/conversation/MessageDetail.stories.tsx
+++ b/ts/components/conversation/MessageDetail.stories.tsx
@@ -36,6 +36,7 @@ const defaultMessage: MessageProps = {
   isBlocked: false,
   isMessageRequestAccepted: true,
   kickOffAttachmentDownload: action('kickOffAttachmentDownload'),
+  markAttachmentAsCorrupted: action('markAttachmentAsCorrupted'),
   openConversation: () => null,
   openLink: () => null,
   previews: [],
diff --git a/ts/components/conversation/Quote.stories.tsx b/ts/components/conversation/Quote.stories.tsx
index 2a1d1b4d1..30252bafa 100644
--- a/ts/components/conversation/Quote.stories.tsx
+++ b/ts/components/conversation/Quote.stories.tsx
@@ -39,6 +39,7 @@ const defaultMessageProps: MessagesProps = {
   isBlocked: false,
   isMessageRequestAccepted: true,
   kickOffAttachmentDownload: () => null,
+  markAttachmentAsCorrupted: () => null,
   openConversation: () => null,
   openLink: () => null,
   previews: [],
diff --git a/ts/components/conversation/Timeline.stories.tsx b/ts/components/conversation/Timeline.stories.tsx
index 224a07c07..f5bc9ae06 100644
--- a/ts/components/conversation/Timeline.stories.tsx
+++ b/ts/components/conversation/Timeline.stories.tsx
@@ -235,6 +235,7 @@ const actions = () => ({
   showContactDetail: action('showContactDetail'),
   showContactModal: action('showContactModal'),
   kickOffAttachmentDownload: action('kickOffAttachmentDownload'),
+  markAttachmentAsCorrupted: action('markAttachmentAsCorrupted'),
   showVisualAttachment: action('showVisualAttachment'),
   downloadAttachment: action('downloadAttachment'),
   displayTapToViewMessage: action('displayTapToViewMessage'),
diff --git a/ts/components/conversation/TimelineItem.stories.tsx b/ts/components/conversation/TimelineItem.stories.tsx
index a6211b34b..cb6f26c2c 100644
--- a/ts/components/conversation/TimelineItem.stories.tsx
+++ b/ts/components/conversation/TimelineItem.stories.tsx
@@ -47,6 +47,7 @@ const getDefaultProps = () => ({
   deleteMessage: action('deleteMessage'),
   deleteMessageForEveryone: action('deleteMessageForEveryone'),
   kickOffAttachmentDownload: action('kickOffAttachmentDownload'),
+  markAttachmentAsCorrupted: action('markAttachmentAsCorrupted'),
   showMessageDetail: action('showMessageDetail'),
   openConversation: action('openConversation'),
   showContactDetail: action('showContactDetail'),
diff --git a/ts/models/messages.ts b/ts/models/messages.ts
index 5a12fb807..544d51b9c 100644
--- a/ts/models/messages.ts
+++ b/ts/models/messages.ts
@@ -2967,6 +2967,47 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
     return false;
   }
 
+  markAttachmentAsCorrupted(attachment: AttachmentType): void {
+    if (!attachment.path) {
+      throw new Error(
+        "Attachment can't be marked as corrupted because it wasn't loaded"
+      );
+    }
+
+    // We intentionally don't check in quotes/stickers/contacts/... here,
+    // because this function should be called only for something that can
+    // be displayed as a generic attachment.
+    const attachments: ReadonlyArray<AttachmentType> =
+      this.get('attachments') || [];
+
+    let changed = false;
+    const newAttachments = attachments.map(existing => {
+      if (existing.path !== attachment.path) {
+        return existing;
+      }
+      changed = true;
+
+      return {
+        ...existing,
+        isCorrupted: true,
+      };
+    });
+
+    if (!changed) {
+      throw new Error(
+        "Attachment can't be marked as corrupted because it wasn't found"
+      );
+    }
+
+    window.log.info(
+      'markAttachmentAsCorrupted: marking an attachment as corrupted'
+    );
+
+    this.set({
+      attachments: newAttachments,
+    });
+  }
+
   // eslint-disable-next-line class-methods-use-this
   async copyFromQuotedMessage(message: WhatIsThis): Promise<boolean> {
     const { quote } = message;
diff --git a/ts/state/smart/MessageAudio.tsx b/ts/state/smart/MessageAudio.tsx
index ada44720c..1dc21b927 100644
--- a/ts/state/smart/MessageAudio.tsx
+++ b/ts/state/smart/MessageAudio.tsx
@@ -25,6 +25,7 @@ export type Props = {
 
   buttonRef: React.RefObject<HTMLButtonElement>;
   kickOffAttachmentDownload(): void;
+  onCorrupted(): void;
 };
 
 const mapStateToProps = (state: StateType, props: Props) => {
diff --git a/ts/types/Attachment.ts b/ts/types/Attachment.ts
index 456adb452..5ad5034fc 100644
--- a/ts/types/Attachment.ts
+++ b/ts/types/Attachment.ts
@@ -49,6 +49,7 @@ export type AttachmentType = {
     contentType: MIME.MIMEType;
     path: string;
   };
+  isCorrupted?: boolean;
 };
 
 // UI-focused functions
@@ -87,6 +88,7 @@ export function isAudio(
     attachments &&
     attachments[0] &&
     attachments[0].contentType &&
+    !attachments[0].isCorrupted &&
     MIME.isAudio(attachments[0].contentType)
   );
 }
diff --git a/ts/util/lint/exceptions.json b/ts/util/lint/exceptions.json
index c6a8228df..d48e7e29c 100644
--- a/ts/util/lint/exceptions.json
+++ b/ts/util/lint/exceptions.json
@@ -14658,7 +14658,7 @@
     "rule": "React-createRef",
     "path": "ts/components/conversation/Message.tsx",
     "line": "  public focusRef: React.RefObject<HTMLDivElement> = React.createRef();",
-    "lineNumber": 237,
+    "lineNumber": 242,
     "reasonCategory": "usageTrusted",
     "updated": "2021-03-05T19:57:01.431Z",
     "reasonDetail": "Used for managing focus only"
@@ -14667,7 +14667,7 @@
     "rule": "React-createRef",
     "path": "ts/components/conversation/Message.tsx",
     "line": "  public audioButtonRef: React.RefObject<HTMLButtonElement> = React.createRef();",
-    "lineNumber": 239,
+    "lineNumber": 244,
     "reasonCategory": "usageTrusted",
     "updated": "2021-03-05T19:57:01.431Z",
     "reasonDetail": "Used for propagating click from the Message to MessageAudio's button"
@@ -14676,7 +14676,7 @@
     "rule": "React-createRef",
     "path": "ts/components/conversation/Message.tsx",
     "line": "  > = React.createRef();",
-    "lineNumber": 243,
+    "lineNumber": 248,
     "reasonCategory": "usageTrusted",
     "updated": "2021-03-05T19:57:01.431Z",
     "reasonDetail": "Used for detecting clicks outside reaction viewer"
diff --git a/ts/views/conversation_view.ts b/ts/views/conversation_view.ts
index 054bffd20..4ab8aeb78 100644
--- a/ts/views/conversation_view.ts
+++ b/ts/views/conversation_view.ts
@@ -9,6 +9,7 @@ import { MediaItemType } from '../components/LightboxGallery';
 import { MessageType } from '../state/ducks/conversations';
 import { ConversationModel } from '../models/conversations';
 import { MessageModel } from '../models/messages';
+import { assert } from '../util/assert';
 
 type GetLinkPreviewImageResult = {
   data: ArrayBuffer;
@@ -27,6 +28,11 @@ type GetLinkPreviewResult = {
   date: number | null;
 };
 
+type AttachmentOptions = {
+  messageId: string;
+  attachment: AttachmentType;
+};
+
 const FIVE_MINUTES = 1000 * 60 * 5;
 const LINK_PREVIEW_TIMEOUT = 60 * 1000;
 
@@ -756,6 +762,16 @@ Whisper.ConversationView = Whisper.View.extend({
       const message = this.model.messageCollection.get(options.messageId);
       await message.queueAttachmentDownloads();
     };
+    const markAttachmentAsCorrupted = (options: AttachmentOptions) => {
+      if (!this.model.messageCollection) {
+        throw new Error('Message collection does not exist');
+      }
+      const message: MessageModel = this.model.messageCollection.get(
+        options.messageId
+      );
+      assert(message, 'Message not found');
+      message.markAttachmentAsCorrupted(options.attachment);
+    };
     const showVisualAttachment = (options: any) => {
       this.showLightbox(options);
     };
@@ -949,6 +965,7 @@ Whisper.ConversationView = Whisper.View.extend({
         downloadAttachment,
         downloadNewVersion,
         kickOffAttachmentDownload,
+        markAttachmentAsCorrupted,
         loadNewerMessages,
         loadNewestMessages: this.loadNewestMessages.bind(this),
         loadAndScroll: this.loadAndScroll.bind(this),