Fix link preview race condition

Co-authored-by: Evan Hahn <69474926+EvanHahn-Signal@users.noreply.github.com>
signalapp · Jun 3, 2021 · f1f3afd · f1f3afd
1 parent 259ee14
commit f1f3afd
Show file tree

Hide file tree

Showing 3 changed files with 77 additions and 2 deletions.
diff --git a/ts/linkPreviews/linkPreviewFetch.ts b/ts/linkPreviews/linkPreviewFetch.ts
@@ -597,5 +597,9 @@ export async function fetchLinkPreviewImage(
     return null;
   }
 
+  if (abortSignal.aborted) {
+    return null;
+  }
+
   return { data, contentType };
 }
diff --git a/ts/test-electron/linkPreviews/linkPreviewFetch_test.ts b/ts/test-electron/linkPreviews/linkPreviewFetch_test.ts
@@ -1354,5 +1354,70 @@ describe('link preview fetching', () => {
         })
       );
     });
+
+    it("doesn't read the image if the request was aborted before reading started", async () => {
+      const abortController = new AbortController();
+
+      const fixture = await readFixture('kitten-1-64-64.jpg');
+
+      const fakeFetch = stub().callsFake(() => {
+        const response = new Response(fixture, {
+          headers: {
+            'Content-Type': 'image/jpeg',
+            'Content-Length': fixture.length.toString(),
+          },
+        });
+        sinon
+          .stub(response, 'arrayBuffer')
+          .rejects(new Error('Should not be called'));
+        sinon.stub(response, 'blob').rejects(new Error('Should not be called'));
+        sinon.stub(response, 'text').rejects(new Error('Should not be called'));
+        sinon.stub(response, 'body').get(() => {
+          throw new Error('Should not be accessed');
+        });
+
+        abortController.abort();
+
+        return response;
+      });
+
+      assert.isNull(
+        await fetchLinkPreviewImage(
+          fakeFetch,
+          'https://example.com/img',
+          abortController.signal
+        )
+      );
+    });
+
+    it('returns null if the request was aborted after the image was read', async () => {
+      const abortController = new AbortController();
+
+      const fixture = await readFixture('kitten-1-64-64.jpg');
+
+      const fakeFetch = stub().callsFake(() => {
+        const response = new Response(fixture, {
+          headers: {
+            'Content-Type': 'image/jpeg',
+            'Content-Length': fixture.length.toString(),
+          },
+        });
+        const oldArrayBufferMethod = response.arrayBuffer.bind(response);
+        sinon.stub(response, 'arrayBuffer').callsFake(async () => {
+          const data = await oldArrayBufferMethod();
+          abortController.abort();
+          return data;
+        });
+        return response;
+      });
+
+      assert.isNull(
+        await fetchLinkPreviewImage(
+          fakeFetch,
+          'https://example.com/img',
+          abortController.signal
+        )
+      );
+    });
   });
 });
diff --git a/ts/views/conversation_view.ts b/ts/views/conversation_view.ts
@@ -4169,14 +4169,13 @@ Whisper.ConversationView = Whisper.View.extend({
       url,
       abortSignal
     );
-    if (!linkPreviewMetadata) {
+    if (!linkPreviewMetadata || abortSignal.aborted) {
       return null;
     }
     const { title, imageHref, description, date } = linkPreviewMetadata;
 
     let image;
     if (
-      !abortSignal.aborted &&
       imageHref &&
       window.Signal.LinkPreviews.isLinkSafeToPreview(imageHref)
     ) {
@@ -4186,6 +4185,9 @@ Whisper.ConversationView = Whisper.View.extend({
           imageHref,
           abortSignal
         );
+        if (abortSignal.aborted) {
+          return null;
+        }
         if (!fullSizeImage) {
           throw new Error('Failed to fetch link preview image');
         }
@@ -4229,6 +4231,10 @@ Whisper.ConversationView = Whisper.View.extend({
       }
     }
 
+    if (abortSignal.aborted) {
+      return null;
+    }
+
     return {
       title,
       url,