Skip to content

Commit

Permalink
feat: Use application/octet-stream as a fallback media type to avoi…
Browse files Browse the repository at this point in the history
…d "Unknown media type" errors (#32471)
  • Loading branch information
matheusbsilva137 committed Jun 19, 2024
1 parent 3039968 commit 02dd875
Show file tree
Hide file tree
Showing 11 changed files with 100 additions and 16 deletions.
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
---

Removed "Unknown media type" errors on the client side by using `application/octet-stream` as a fallback media type (MIME type) for all files
6 changes: 4 additions & 2 deletions 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 { getMimeType } from '../../../utils/lib/mimeTypes';

type UploadResult<K> = {
file: Readable & { truncated: boolean };
fieldname: string;
Expand Down Expand Up @@ -61,7 +63,7 @@ export async function getUploadFormData<
function onFile(
fieldname: string,
file: Readable & { truncated: boolean },
{ filename, encoding, mimeType: mimetype }: { filename: string; encoding: string; mimeType: string },
{ filename, encoding }: { filename: string; encoding: string },
) {
if (options.field && fieldname !== options.field) {
file.resume();
Expand All @@ -83,7 +85,7 @@ export async function getUploadFormData<
file,
filename,
encoding,
mimetype,
mimetype: getMimeType(filename),
fieldname,
fields,
fileBuffer: Buffer.concat(fileChunks),
Expand Down
7 changes: 6 additions & 1 deletion apps/meteor/app/utils/lib/mimeTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,9 @@ const getExtension = (param: string): string => {
return !extension || typeof extension === 'boolean' ? '' : extension;
};

export { mime, getExtension };
const getMimeType = (fileName: string): string => {
const fileMimeType = mime.lookup(fileName);
return typeof fileMimeType === 'string' ? fileMimeType : 'application/octet-stream';
};

export { mime, getExtension, getMimeType };
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export const useFileUploadDropTarget = (): readonly [

const t = useTranslation();

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

const onFileDrop = useMutableCallback(async (files: File[]) => {
const { mime } = await import('../../../../../app/utils/lib/mimeTypes');

const { getMimeType } = await import('../../../../../app/utils/lib/mimeTypes');
const getUniqueFiles = () => {
const uniqueFiles: File[] = [];
const st: Set<number> = new Set();
Expand All @@ -55,7 +54,7 @@ export const useFileUploadDropTarget = (): readonly [
const uniqueFiles = getUniqueFiles();

const uploads = Array.from(uniqueFiles).map((file) => {
Object.defineProperty(file, 'type', { value: mime.lookup(file.name) });
Object.defineProperty(file, 'type', { value: getMimeType(file.name) });
return file;
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ 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 fileInputRef = useFileInput(fileInputProps);
const chat = useChat();

Expand All @@ -23,10 +23,10 @@ export const useFileUploadAction = (disabled: boolean): GenericMenuItemProps =>
};

const handleUploadChange = async () => {
const { mime } = await import('../../../../../../../app/utils/lib/mimeTypes');
const { getMimeType } = await import('../../../../../../../app/utils/lib/mimeTypes');
const filesToUpload = Array.from(fileInputRef?.current?.files ?? []).map((file) => {
Object.defineProperty(file, 'type', {
value: mime.lookup(file.name),
value: getMimeType(file.name),
});
return file;
});
Expand Down
1 change: 1 addition & 0 deletions apps/meteor/server/settings/file-upload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export const createFileUploadSettings = () =>
type: 'string',
public: true,
i18nDescription: 'FileUpload_MediaTypeBlackListDescription',
alert: 'FileUpload_MediaTypeBlackList_Alert',
});

await this.add('FileUpload_ProtectFiles', true, {
Expand Down
1 change: 1 addition & 0 deletions apps/meteor/tests/data/interactions.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
export const targetUser = 'rocket.cat';
export const imgURL = './public/images/logo/1024x1024.png';
export const lstURL = './tests/e2e/fixtures/files/lst-test.lst';
export const drawioURL = './tests/e2e/fixtures/files/diagram.drawio';
export const svgLogoURL = './public/images/logo/logo.svg';
export const svgLogoFileName = 'logo.svg';
21 changes: 21 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,7 @@ test.describe.serial('file-upload', () => {
let targetChannel: string;

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

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

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

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

test('expect send drawio (unknown media type) file succesfully', async ({ page }) => {
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 (unknown media type) when the default media type is blocked', async ({ api, page }) => {
await setSettingValueById(api, 'FileUpload_MediaTypeBlackList', 'application/octet-stream');

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>
47 changes: 41 additions & 6 deletions apps/meteor/tests/end-to-end/api/09-rooms.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { after, afterEach, before, beforeEach, describe, it } from 'mocha';
import { sleep } from '../../../lib/utils/sleep';
import { getCredentials, api, request, credentials } from '../../data/api-data.js';
import { sendSimpleMessage, deleteMessage } from '../../data/chat.helper';
import { imgURL, lstURL, svgLogoFileName, svgLogoURL } from '../../data/interactions';
import { drawioURL, imgURL, lstURL, svgLogoFileName, svgLogoURL } from '../../data/interactions';
import { getSettingValueById, updateEEPermission, updatePermission, updateSetting } from '../../data/permissions.helper';
import { createRoom, deleteRoom } from '../../data/rooms.helper';
import { deleteTeam } from '../../data/teams.helper';
Expand Down Expand Up @@ -183,8 +183,8 @@ describe('[Rooms]', function () {
});
});

it('should upload a LST file to room', (done) => {
request
it('should upload a LST file to room', () => {
return request
.post(api(`rooms.upload/${testChannel._id}`))
.set(credentials)
.attach('file', lstURL)
Expand All @@ -200,12 +200,33 @@ describe('[Rooms]', function () {
expect(res.body.message).to.have.property('files');
expect(res.body.message.files).to.be.an('array').of.length(1);
expect(res.body.message.files[0]).to.have.property('name', 'lst-test.lst');
})
.end(done);
expect(res.body.message.files[0]).to.have.property('type', 'text/plain');
});
});

it('should upload a DRAWIO file (unknown media type) to room', () => {
return request
.post(api(`rooms.upload/${testChannel._id}`))
.set(credentials)
.attach('file', drawioURL)
.expect('Content-Type', 'application/json')
.expect(200)
.expect((res) => {
expect(res.body).to.have.property('success', true);
expect(res.body).to.have.property('message');
expect(res.body.message).to.have.property('attachments');
expect(res.body.message.attachments).to.be.an('array').of.length(1);
expect(res.body.message.attachments[0]).to.have.property('format', 'DRAWIO');
expect(res.body.message.attachments[0]).to.have.property('title', 'diagram.drawio');
expect(res.body.message).to.have.property('files');
expect(res.body.message.files).to.be.an('array').of.length(1);
expect(res.body.message.files[0]).to.have.property('name', 'diagram.drawio');
expect(res.body.message.files[0]).to.have.property('type', 'application/octet-stream');
});
});

it('should not allow uploading a blocked media type to a room', async () => {
await updateSetting('FileUpload_MediaTypeBlackList', 'application/octet-stream');
await updateSetting('FileUpload_MediaTypeBlackList', 'text/plain');
await request
.post(api(`rooms.upload/${testChannel._id}`))
.set(credentials)
Expand All @@ -218,6 +239,20 @@ describe('[Rooms]', function () {
});
});

it('should not allow uploading an unknown media type to a room if the default one is blocked', async () => {
await updateSetting('FileUpload_MediaTypeBlackList', 'application/octet-stream');
await request
.post(api(`rooms.upload/${testChannel._id}`))
.set(credentials)
.attach('file', drawioURL)
.expect('Content-Type', 'application/json')
.expect(400)
.expect((res) => {
expect(res.body).to.have.property('success', false);
expect(res.body).to.have.property('errorType', 'error-invalid-file-type');
});
});

it('should be able to get the file', async () => {
await request.get(fileNewUrl).set(credentials).expect('Content-Type', 'image/png').expect(200);
await request.get(fileOldUrl).set(credentials).expect('Content-Type', 'image/png').expect(200);
Expand Down
1 change: 1 addition & 0 deletions packages/i18n/src/locales/en.i18n.json
Original file line number Diff line number Diff line change
Expand Up @@ -2383,6 +2383,7 @@
"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.",
"FileUpload_MediaTypeBlackList_Alert": "The default media type for unknown file extensions is \"application/octet-stream\", to work only with known file extensions you can add it to the \"Blocked Media Types\" list.",
"FileUpload_MediaTypeWhiteList": "Accepted Media Types",
"FileUpload_MediaTypeWhiteListDescription": "Comma-separated list of media types. Leave it blank for accepting all media types.",
"FileUpload_ProtectFiles": "Protect Uploaded Files",
Expand Down

0 comments on commit 02dd875

Please sign in to comment.