Differentiate download and decryption errors when showing images (#9562)

package.json

@@ -202,6 +202,7 @@
     "jest-raw-loader": "^1.0.1",
     "matrix-mock-request": "^2.5.0",
     "matrix-web-i18n": "^1.3.0",
+    "node-fetch": "2",
     "postcss-scss": "^4.0.4",
     "raw-loader": "^4.0.2",
     "react-test-renderer": "^17.0.2",

src/components/views/messages/MImageBody.tsx

@@ -39,6 +39,7 @@ import { blobIsAnimated, mayBeAnimated } from '../../../utils/Image';
 import { presentableTextForFile } from "../../../utils/FileUtils";
 import { createReconnectedListener } from '../../../utils/connection';
 import MediaProcessingError from './shared/MediaProcessingError';
+import { DecryptError, DownloadError } from "../../../utils/DecryptFile";
 
 enum Placeholder {
     NoImage,
@@ -258,7 +259,15 @@ export default class MImageBody extends React.Component<IBodyProps, IState> {
                 ]));
             } catch (error) {
                 if (this.unmounted) return;
-                logger.warn("Unable to decrypt attachment: ", error);
+
+                if (error instanceof DecryptError) {
+                    logger.error("Unable to decrypt attachment: ", error);
+                } else if (error instanceof DownloadError) {
+                    logger.error("Unable to download attachment to decrypt it: ", error);
+                } else {
+                    logger.error("Error encountered when downloading encrypted attachment: ", error);
+                }
+
                 // Set a placeholder image when we can't decrypt the image.
                 this.setState({ error });
             }
@@ -557,9 +566,16 @@ export default class MImageBody extends React.Component<IBodyProps, IState> {
         const content = this.props.mxEvent.getContent<IMediaEventContent>();
 
         if (this.state.error) {
+            let errorText = _t("Unable to show image due to error");
+            if (this.state.error instanceof DecryptError) {
+                errorText = _t("Error decrypting image");
+            } else if (this.state.error instanceof DownloadError) {
+                errorText = _t("Error downloading image");
+            }
+
             return (
                 <MediaProcessingError className="mx_MImageBody">
-                    { _t("Error decrypting image") }
+                    { errorText }
                 </MediaProcessingError>
             );
         }

src/i18n/strings/en_EN.json

@@ -2312,7 +2312,9 @@
     "Decrypt %(text)s": "Decrypt %(text)s",
     "Invalid file%(extra)s": "Invalid file%(extra)s",
     "Image": "Image",
+    "Unable to show image due to error": "Unable to show image due to error",
     "Error decrypting image": "Error decrypting image",
+    "Error downloading image": "Error downloading image",
     "Show image": "Show image",
     "Join the conference at the top of this room": "Join the conference at the top of this room",
     "Join the conference from the room information card on the right": "Join the conference from the room information card on the right",

src/utils/DecryptFile.ts

@@ -16,11 +16,28 @@ limitations under the License.
 
 // Pull in the encryption lib so that we can decrypt attachments.
 import encrypt from 'matrix-encrypt-attachment';
+import { parseErrorResponse } from 'matrix-js-sdk/src/http-api';
 
 import { mediaFromContent } from "../customisations/Media";
 import { IEncryptedFile, IMediaEventInfo } from "../customisations/models/IMediaEventContent";
 import { getBlobSafeMimeType } from "./blobs";
 
+export class DownloadError extends Error {
+    constructor(e) {
+        super(e.message);
+        this.name = "DownloadError";
+        this.stack = e.stack;
+    }
+}
+
+export class DecryptError extends Error {
+    constructor(e) {
+        super(e.message);
+        this.name = "DecryptError";
+        this.stack = e.stack;
+    }
+}
+
 /**
  * Decrypt a file attached to a matrix event.
  * @param {IEncryptedFile} file The encrypted file information taken from the matrix event.
@@ -30,19 +47,27 @@ import { getBlobSafeMimeType } from "./blobs";
  * @param {IMediaEventInfo} info The info parameter taken from the matrix event.
  * @returns {Promise<Blob>} Resolves to a Blob of the file.
  */
-export function decryptFile(
+export async function decryptFile(
     file: IEncryptedFile,
     info?: IMediaEventInfo,
 ): Promise<Blob> {
     const media = mediaFromContent({ file });
-    // Download the encrypted file as an array buffer.
-    return media.downloadSource().then((response) => {
-        return response.arrayBuffer();
-    }).then((responseData) => {
-        // Decrypt the array buffer using the information taken from
-        // the event content.
-        return encrypt.decryptAttachment(responseData, file);
-    }).then((dataArray) => {
+
+    let responseData: ArrayBuffer;
+    try {
+        // Download the encrypted file as an array buffer.
+        const response = await media.downloadSource();
+        if (!response.ok) {
+            throw parseErrorResponse(response, await response.text());
+        }
+        responseData = await response.arrayBuffer();
+    } catch (e) {
+        throw new DownloadError(e);
+    }
+
+    try {
+        // Decrypt the array buffer using the information taken from the event content.
+        const dataArray = await encrypt.decryptAttachment(responseData, file);
         // Turn the array into a Blob and give it the correct MIME-type.
 
         // IMPORTANT: we must not allow scriptable mime-types into Blobs otherwise
@@ -53,5 +78,7 @@ export function decryptFile(
         mimetype = getBlobSafeMimeType(mimetype);
 
         return new Blob([dataArray], { type: mimetype });
-    });
+    } catch (e) {
+        throw new DecryptError(e);
+    }
 }

src/utils/LazyValue.ts

@@ -48,12 +48,10 @@ export class LazyValue<T> {
         if (this.prom) return this.prom;
         this.prom = this.getFn();
 
-        // Fork the promise chain to avoid accidentally making it return undefined always.
-        this.prom.then(v => {
+        return this.prom.then(v => {
             this.val = v;
             this.done = true;
+            return v;
         });
-
-        return this.prom;
     }
 }

test/components/views/messages/MImageBody-test.tsx

@@ -0,0 +1,99 @@
+/*
+Copyright 2022 The Matrix.org Foundation C.I.C.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+import React from "react";
+import { render, screen } from "@testing-library/react";
+import { EventType, MatrixEvent, Room } from "matrix-js-sdk/src/matrix";
+import fetchMock from "fetch-mock-jest";
+import encrypt from "matrix-encrypt-attachment";
+import { mocked } from "jest-mock";
+
+import MImageBody from "../../../../src/components/views/messages/MImageBody";
+import { RoomPermalinkCreator } from "../../../../src/utils/permalinks/Permalinks";
+import {
+    getMockClientWithEventEmitter,
+    mockClientMethodsCrypto,
+    mockClientMethodsDevice,
+    mockClientMethodsServer,
+    mockClientMethodsUser,
+} from "../../../test-utils";
+import { MediaEventHelper } from "../../../../src/utils/MediaEventHelper";
+
+jest.mock("matrix-encrypt-attachment", () => ({
+    decryptAttachment: jest.fn(),
+}));
+
+describe("<MImageBody/>", () => {
+    const userId = "@user:server";
+    const deviceId = "DEADB33F";
+    const cli = getMockClientWithEventEmitter({
+        ...mockClientMethodsUser(userId),
+        ...mockClientMethodsServer(),
+        ...mockClientMethodsDevice(deviceId),
+        ...mockClientMethodsCrypto(),
+        getRooms: jest.fn().mockReturnValue([]),
+        getIgnoredUsers: jest.fn(),
+        getVersions: jest.fn().mockResolvedValue({
+            unstable_features: {
+                'org.matrix.msc3882': true,
+                'org.matrix.msc3886': true,
+            },
+        }),
+    });
+    const url = "https://server/_matrix/media/r0/download/server/encrypted-image";
+    // eslint-disable-next-line no-restricted-properties
+    cli.mxcUrlToHttp.mockReturnValue(url);
+    const encryptedMediaEvent = new MatrixEvent({
+        room_id: "!room:server",
+        sender: userId,
+        type: EventType.RoomMessage,
+        content: {
+            file: {
+                url: "mxc://server/encrypted-image",
+            },
+        },
+    });
+    const props = {
+        onHeightChanged: jest.fn(),
+        onMessageAllowed: jest.fn(),
+        permalinkCreator: new RoomPermalinkCreator(new Room(encryptedMediaEvent.getRoomId(), cli, cli.getUserId())),
+    };
+
+    it("should show error when encrypted media cannot be downloaded", async () => {
+        fetchMock.getOnce(url, { status: 500 });
+
+        render(<MImageBody
+            {...props}
+            mxEvent={encryptedMediaEvent}
+            mediaEventHelper={new MediaEventHelper(encryptedMediaEvent)}
+        />);
+
+        await screen.findByText("Error downloading image");
+    });
+
+    it("should show error when encrypted media cannot be decrypted", async () => {
+        fetchMock.getOnce(url, "thisistotallyanencryptedpng");
+        mocked(encrypt.decryptAttachment).mockRejectedValue(new Error("Failed to decrypt"));
+
+        render(<MImageBody
+            {...props}
+            mxEvent={encryptedMediaEvent}
+            mediaEventHelper={new MediaEventHelper(encryptedMediaEvent)}
+        />);
+
+        await screen.findByText("Error decrypting image");
+    });
+});