Skip to content

Check that the file the user chose has a MIME type of image/* #28467

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Nov 18, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions src/components/views/elements/MiniAvatarUploader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { useTimeout } from "../../../hooks/useTimeout";
import { chromeFileInputFix } from "../../../utils/BrowserWorkarounds";
import AccessibleButton from "./AccessibleButton";
import Spinner from "./Spinner";
import { getFileChanged } from "../settings/AvatarSetting.tsx";

export const AVATAR_SIZE = "52px";

Expand Down Expand Up @@ -72,11 +73,12 @@ const MiniAvatarUploader: React.FC<IProps> = ({
onClick?.(ev);
}}
onChange={async (ev): Promise<void> => {
if (!ev.target.files?.length) return;
setBusy(true);
const file = ev.target.files[0];
const { content_uri: uri } = await cli.uploadContent(file);
await setAvatarUrl(uri);
const file = getFileChanged(ev);
if (file) {
const { content_uri: uri } = await cli.uploadContent(file);
await setAvatarUrl(uri);
}
setBusy(false);
}}
accept="image/*"
Expand Down
20 changes: 19 additions & 1 deletion src/components/views/settings/AvatarSetting.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import { chromeFileInputFix } from "../../../utils/BrowserWorkarounds";
import { useId } from "../../../utils/useId";
import AccessibleButton from "../elements/AccessibleButton";
import BaseAvatar from "../avatars/BaseAvatar";
import Modal from "../../../Modal.tsx";
import ErrorDialog from "../dialogs/ErrorDialog.tsx";

interface MenuProps {
trigger: ReactNode;
Expand Down Expand Up @@ -103,6 +105,19 @@ interface IProps {
placeholderName: string;
}

export function getFileChanged(e: React.ChangeEvent<HTMLInputElement>): File | null {
if (!e.target.files?.length) return null;
const file = e.target.files[0];
if (!file.type.startsWith("image/")) {
Modal.createDialog(ErrorDialog, {
title: _t("upload_failed_title"),
description: _t("upload_file|not_image"),
});
return null;
}
return file;
}

/**
* Component for setting or removing an avatar on something (eg. a user or a room)
*/
Expand Down Expand Up @@ -139,7 +154,10 @@ const AvatarSetting: React.FC<IProps> = ({

const onFileChanged = useCallback(
(e: React.ChangeEvent<HTMLInputElement>) => {
if (e.target.files) onChange?.(e.target.files[0]);
const file = getFileChanged(e);
if (file) {
onChange?.(file);
}
},
[onChange],
);
Expand Down
1 change: 1 addition & 0 deletions src/i18n/strings/en_EN.json
Original file line number Diff line number Diff line change
Expand Up @@ -3742,6 +3742,7 @@
"error_files_too_large": "These files are <b>too large</b> to upload. The file size limit is %(limit)s.",
"error_some_files_too_large": "Some files are <b>too large</b> to be uploaded. The file size limit is %(limit)s.",
"error_title": "Upload Error",
"not_image": "The file you have chosen is not a valid image file.",
"title": "Upload files",
"title_progress": "Upload files (%(current)s of %(total)s)",
"upload_all_button": "Upload all",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
/*
Copyright 2024 New Vector Ltd.

SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only
Please see LICENSE files in the repository root for full details.
*/

import React from "react";
import { render } from "jest-matrix-react";
import userEvent from "@testing-library/user-event";
import { mocked } from "jest-mock";

import MiniAvatarUploader from "../../../../../src/components/views/elements/MiniAvatarUploader.tsx";
import { stubClient, withClientContextRenderOptions } from "../../../../test-utils";

const BASE64_GIF = "R0lGODlhAQABAAAAACw=";
const AVATAR_FILE = new File([Uint8Array.from(atob(BASE64_GIF), (c) => c.charCodeAt(0))], "avatar.gif", {
type: "image/gif",
});

describe("<MiniAvatarUploader />", () => {
it("calls setAvatarUrl when a file is uploaded", async () => {
const cli = stubClient();
mocked(cli.uploadContent).mockResolvedValue({ content_uri: "mxc://example.com/1234" });

const setAvatarUrl = jest.fn();
const user = userEvent.setup();

const { container, findByText } = render(
<MiniAvatarUploader hasAvatar={false} noAvatarLabel="Upload" setAvatarUrl={setAvatarUrl} isUserAvatar />,
withClientContextRenderOptions(cli),
);

await findByText("Upload");
await user.upload(container.querySelector("input")!, AVATAR_FILE);

expect(cli.uploadContent).toHaveBeenCalledWith(AVATAR_FILE);
expect(setAvatarUrl).toHaveBeenCalledWith("mxc://example.com/1234");
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only
Please see LICENSE files in the repository root for full details.
*/
import React from "react";
import { render, screen } from "jest-matrix-react";
import { render, screen, fireEvent } from "jest-matrix-react";
import userEvent from "@testing-library/user-event";

import AvatarSetting from "../../../../../src/components/views/settings/AvatarSetting";
Expand All @@ -16,6 +16,9 @@ const BASE64_GIF = "R0lGODlhAQABAAAAACw=";
const AVATAR_FILE = new File([Uint8Array.from(atob(BASE64_GIF), (c) => c.charCodeAt(0))], "avatar.gif", {
type: "image/gif",
});
const GENERIC_FILE = new File([Uint8Array.from(atob(BASE64_GIF), (c) => c.charCodeAt(0))], "not-avatar.doc", {
type: "application/msword",
});

describe("<AvatarSetting />", () => {
beforeEach(() => {
Expand Down Expand Up @@ -70,4 +73,45 @@ describe("<AvatarSetting />", () => {

expect(onChange).toHaveBeenCalledWith(AVATAR_FILE);
});

it("should noop when selecting no file", async () => {
const onChange = jest.fn();

render(
<AvatarSetting
placeholderId="blee"
placeholderName="boo"
avatar="mxc://example.org/my-avatar"
avatarAltText="Avatar of Peter Fox"
onChange={onChange}
/>,
);

const fileInput = screen.getByAltText("Upload");
// Can't use userEvent.upload here as it doesn't support uploading invalid files
fireEvent.change(fileInput, { target: { files: [] } });

expect(onChange).not.toHaveBeenCalled();
});

it("should show error if user tries to use non-image file", async () => {
const onChange = jest.fn();

render(
<AvatarSetting
placeholderId="blee"
placeholderName="boo"
avatar="mxc://example.org/my-avatar"
avatarAltText="Avatar of Peter Fox"
onChange={onChange}
/>,
);

const fileInput = screen.getByAltText("Upload");
// Can't use userEvent.upload here as it doesn't support uploading invalid files
fireEvent.change(fileInput, { target: { files: [GENERIC_FILE] } });

expect(onChange).not.toHaveBeenCalled();
await expect(screen.findByRole("heading", { name: "Upload Failed" })).resolves.toBeInTheDocument();
});
});
Loading