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

feat: Use application/octet-stream as a fallback media type to avoid "Unknown media type" errors #32471

Merged
merged 19 commits into from
Jun 19, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
64a2c25
feat: Add setting to allow uploading unknown media types
matheusbsilva137 May 22, 2024
8b36f09
test: add end-to-end tests
matheusbsilva137 May 22, 2024
d267286
Create changeset
matheusbsilva137 May 22, 2024
06b92b6
feat: Improve error when uploading unknown media types
matheusbsilva137 May 22, 2024
7de1eab
Merge branch 'feat/setting-allow-upload-unknown-media' of https://git…
matheusbsilva137 May 22, 2024
cd4113c
Update setting's name and description
matheusbsilva137 May 24, 2024
bf5d06e
Update changeset
matheusbsilva137 May 24, 2024
57a36af
fix end-to-end tests
matheusbsilva137 May 24, 2024
9cfb465
Merge branch 'feat/setting-allow-upload-unknown-media' of https://git…
matheusbsilva137 May 24, 2024
a857479
improve: Remove setting and make application/octet-stream the default…
matheusbsilva137 Jun 13, 2024
a35783e
i18n: remove unused translation
matheusbsilva137 Jun 13, 2024
13cd06c
Merge branch 'develop' of https://github.com/RocketChat/Rocket.Chat i…
matheusbsilva137 Jun 13, 2024
fcb6086
i18n: re-add translations
matheusbsilva137 Jun 13, 2024
49a553b
improve: Add alert to media type blocked list setting
matheusbsilva137 Jun 13, 2024
e4d0a91
Update alert
matheusbsilva137 Jun 14, 2024
3f3e6ae
improve: Use helper function to get file MIME type
matheusbsilva137 Jun 17, 2024
93717af
Merge branch 'develop' into feat/setting-allow-upload-unknown-media
matheusbsilva137 Jun 17, 2024
bb81896
Merge branch 'develop' into feat/setting-allow-upload-unknown-media
matheusbsilva137 Jun 17, 2024
2886ac3
Merge branch 'develop' into feat/setting-allow-upload-unknown-media
matheusbsilva137 Jun 19, 2024
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
6 changes: 6 additions & 0 deletions .changeset/breezy-pens-sing.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@rocket.chat/meteor": minor
"@rocket.chat/i18n": minor
---

Added "Reject unknown media types" setting to control whether uploads with unknown media types (MIME types) should be accepted
5 changes: 4 additions & 1 deletion apps/meteor/app/api/server/lib/getUploadFormData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import type { ValidateFunction } from 'ajv';
import busboy from 'busboy';
import type { Request } from 'express';

import { mime } from '../../../utils/lib/mimeTypes';

type UploadResult<K> = {
file: Readable & { truncated: boolean };
fieldname: string;
Expand Down Expand Up @@ -78,12 +80,13 @@ export async function getUploadFormData<
fileChunks.length = 0;
return returnError(new MeteorError('error-file-too-large'));
}
const mimeTypeFromFilename = mime.lookup(filename);

uploadedFile = {
file,
filename,
encoding,
mimetype,
mimetype: typeof mimeTypeFromFilename === 'string' ? mimeTypeFromFilename : mimetype,
matheusbsilva137 marked this conversation as resolved.
Show resolved Hide resolved
matheusbsilva137 marked this conversation as resolved.
Show resolved Hide resolved
fieldname,
fields,
fileBuffer: Buffer.concat(fileChunks),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ export const useFileUploadDropTarget = (): readonly [

const t = useTranslation();

const fileUploadEnabled = useSetting('FileUpload_Enabled') as boolean;
const fileUploadEnabled = useSetting<boolean>('FileUpload_Enabled');
const rejectUnknownMediaTypes = useSetting<boolean>('FileUpload_RejectUnknownMediaTypes');
const user = useUser();
const fileUploadAllowedForUser = useReactiveValue(
useCallback(() => !roomCoordinator.readOnly(room._id, { username: user?.username }), [room._id, user?.username]),
Expand All @@ -55,7 +56,8 @@ export const useFileUploadDropTarget = (): readonly [
const uniqueFiles = getUniqueFiles();

const uploads = Array.from(uniqueFiles).map((file) => {
Object.defineProperty(file, 'type', { value: mime.lookup(file.name) });
const fileMimeType = mime.lookup(file.name);
Object.defineProperty(file, 'type', { value: fileMimeType ?? (rejectUnknownMediaTypes ? undefined : 'application/octet-stream') });
matheusbsilva137 marked this conversation as resolved.
Show resolved Hide resolved
return file;
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ const fileInputProps = { type: 'file', multiple: true };

export const useFileUploadAction = (disabled: boolean): GenericMenuItemProps => {
const t = useTranslation();
const fileUploadEnabled = useSetting('FileUpload_Enabled');
const fileUploadEnabled = useSetting<boolean>('FileUpload_Enabled');
const rejectUnknownMediaTypes = useSetting<boolean>('FileUpload_RejectUnknownMediaTypes');
const fileInputRef = useFileInput(fileInputProps);
const chat = useChat();

Expand All @@ -25,8 +26,9 @@ export const useFileUploadAction = (disabled: boolean): GenericMenuItemProps =>
const handleUploadChange = async () => {
const { mime } = await import('../../../../../../../app/utils/lib/mimeTypes');
const filesToUpload = Array.from(fileInputRef?.current?.files ?? []).map((file) => {
const fileMimeType = mime.lookup(file.name);
Object.defineProperty(file, 'type', {
value: mime.lookup(file.name),
value: fileMimeType ?? (rejectUnknownMediaTypes ? undefined : 'application/octet-stream'),
matheusbsilva137 marked this conversation as resolved.
Show resolved Hide resolved
});
return file;
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,15 @@ const FileUploadModal = ({
};

useEffect(() => {
if (invalidContentType && !file.type) {
matheusbsilva137 marked this conversation as resolved.
Show resolved Hide resolved
dispatchToastMessage({
type: 'error',
message: t('FileUpload_MediaType_Unknown'),
});
onClose();
return;
}

if (invalidContentType) {
dispatchToastMessage({
type: 'error',
Expand Down
5 changes: 5 additions & 0 deletions apps/meteor/server/settings/file-upload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,11 @@ export const createFileUploadSettings = () =>
i18nDescription: 'FileUpload_MediaTypeBlackListDescription',
});

await this.add('FileUpload_RejectUnknownMediaTypes', true, {
type: 'boolean',
public: true,
});

await this.add('FileUpload_ProtectFiles', true, {
type: 'boolean',
public: true,
Expand Down
38 changes: 38 additions & 0 deletions apps/meteor/tests/e2e/file-upload.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { Users } from './fixtures/userStates';
import { HomeChannel } from './page-objects';
import { createTargetChannel } from './utils';
import { setSettingValueById } from './utils/setSettingValueById';
import { expect, test } from './utils/test';

test.use({ storageState: Users.user1.state });
Expand All @@ -10,6 +11,10 @@ test.describe.serial('file-upload', () => {
let targetChannel: string;

test.beforeAll(async ({ api }) => {
await Promise.all([
setSettingValueById(api, 'FileUpload_RejectUnknownMediaTypes', true),
setSettingValueById(api, 'FileUpload_MediaTypeBlackList', 'image/svg+xml'),
]);
targetChannel = await createTargetChannel(api);
});

Expand All @@ -21,6 +26,10 @@ test.describe.serial('file-upload', () => {
});

test.afterAll(async ({ api }) => {
await Promise.all([
setSettingValueById(api, 'FileUpload_RejectUnknownMediaTypes', true),
setSettingValueById(api, 'FileUpload_MediaTypeBlackList', 'image/svg+xml'),
]);
expect((await api.post('/channels.delete', { roomName: targetChannel })).status()).toBe(200);
});

Expand Down Expand Up @@ -54,4 +63,33 @@ test.describe.serial('file-upload', () => {
await expect(poHomeChannel.content.getFileDescription).toHaveText('lst_description');
await expect(poHomeChannel.content.lastMessageFileName).toContainText('lst-test.lst');
});

test('expect not to send drawio file when the "Reject unknown media types" setting is enabled', async ({ api, page }) => {
matheusbsilva137 marked this conversation as resolved.
Show resolved Hide resolved
await setSettingValueById(api, 'FileUpload_RejectUnknownMediaTypes', true);

await page.reload();
await poHomeChannel.content.sendFileMessage('diagram.drawio');
await expect(poHomeChannel.content.btnModalConfirm).not.toBeVisible();
});

test('expect send drawio file succesfully when the "Reject unknown media types" setting is disabled', async ({ api, page }) => {
matheusbsilva137 marked this conversation as resolved.
Show resolved Hide resolved
await setSettingValueById(api, 'FileUpload_RejectUnknownMediaTypes', false);

await page.reload();
await poHomeChannel.content.sendFileMessage('diagram.drawio');
await poHomeChannel.content.descriptionInput.fill('drawio_description');
await poHomeChannel.content.btnModalConfirm.click();

await expect(poHomeChannel.content.getFileDescription).toHaveText('drawio_description');
await expect(poHomeChannel.content.lastMessageFileName).toContainText('diagram.drawio');
});

test('expect not to send drawio file when the "Reject unknown media types" setting is disabled, but the default media type is blocked', async ({ api, page }) => {
matheusbsilva137 marked this conversation as resolved.
Show resolved Hide resolved
await setSettingValueById(api, 'FileUpload_MediaTypeBlackList', 'application/octet-stream')
await setSettingValueById(api, 'FileUpload_RejectUnknownMediaTypes', false);

await page.reload();
await poHomeChannel.content.sendFileMessage('diagram.drawio');
await expect(poHomeChannel.content.btnModalConfirm).not.toBeVisible();
});
});
13 changes: 13 additions & 0 deletions apps/meteor/tests/e2e/fixtures/files/diagram.drawio
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<mxfile host="app.diagrams.net" modified="2024-05-21T16:10:09.295Z" agent="Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/124.0.0.0 Safari/537.36" etag="ZxZRCTHi-kxhlzk7b9_Z" version="24.4.4" type="device">
<diagram name="Página-1" id="9eBILa8281JaQ4yUkDbp">
<mxGraphModel dx="1434" dy="786" grid="1" gridSize="10" guides="1" tooltips="1" connect="1" arrows="1" fold="1" page="1" pageScale="1" pageWidth="827" pageHeight="1169" math="0" shadow="0">
<root>
<mxCell id="0" />
<mxCell id="1" parent="0" />
<mxCell id="dopCU4gkJe7Sfp6IDYO1-1" value="&lt;b&gt;&lt;font style=&quot;font-size: 30px;&quot;&gt;Rocket.Chat&lt;/font&gt;&lt;/b&gt;" style="text;html=1;align=center;verticalAlign=middle;resizable=0;points=[];autosize=1;strokeColor=none;fillColor=none;" vertex="1" parent="1">
<mxGeometry x="314" y="350" width="200" height="50" as="geometry" />
</mxCell>
</root>
</mxGraphModel>
</diagram>
</mxfile>
3 changes: 3 additions & 0 deletions packages/i18n/src/locales/en.i18n.json
Original file line number Diff line number Diff line change
Expand Up @@ -2369,6 +2369,7 @@
"FileUpload_MaxFileSize": "Maximum File Upload Size (in bytes)",
"FileUpload_MaxFileSizeDescription": "Set it to -1 to remove the file size limitation.",
"FileUpload_MediaType_NotAccepted__type__": "Media Type Not Accepted: {{type}}",
"FileUpload_MediaType_Unknown": "Unknown media type not accepted",
"FileUpload_MediaType_NotAccepted": "Media Types Not Accepted",
"FileUpload_MediaTypeBlackList": "Blocked Media Types",
"FileUpload_MediaTypeBlackListDescription": "Comma-separated list of media types. This setting has priority over the Accepted Media Types.",
Expand All @@ -2377,6 +2378,8 @@
"FileUpload_ProtectFiles": "Protect Uploaded Files",
"FileUpload_ProtectFilesDescription": "Only authenticated users will have access",
"FileUpload_ProtectFilesEnabled_JWTNotSet": "Uploaded files are protected, but JWT access is not setup, this is required for Twilio to send media messages. Setup in Settings -> FileUpload",
"FileUpload_RejectUnknownMediaTypes": "Reject unknown media types",
"FileUpload_RejectUnknownMediaTypes_Description": "Uploading unknown media types is blocked by default. When this setting is disabled, the `application/octet-stream` MIME type will be assigned to files with unknown media types so that they can be uploaded.",
"FileUpload_RotateImages": "Rotate images on upload",
"FileUpload_RotateImages_Description": "Enabling this setting may cause image quality loss",
"FileUpload_S3_Acl": "Acl",
Expand Down
Loading