Skip to content
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

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

Draft
wants to merge 5 commits into
base: develop
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all 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
19 changes: 18 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,18 @@ 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 file;
}

/**
* Component for setting or removing an avatar on something (eg. a user or a room)
*/
Expand Down Expand Up @@ -139,7 +153,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
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