From 02dd87574bc0939fb38a428c1fb623c3b752d6ca Mon Sep 17 00:00:00 2001 From: Matheus Barbosa Silva <36537004+matheusbsilva137@users.noreply.github.com> Date: Wed, 19 Jun 2024 08:13:21 -0300 Subject: [PATCH] feat: Use `application/octet-stream` as a fallback media type to avoid "Unknown media type" errors (#32471) --- .changeset/breezy-pens-sing.md | 6 +++ .../app/api/server/lib/getUploadFormData.ts | 6 ++- apps/meteor/app/utils/lib/mimeTypes.ts | 7 ++- .../body/hooks/useFileUploadDropTarget.ts | 7 ++- .../hooks/useFileUploadAction.ts | 6 +-- apps/meteor/server/settings/file-upload.ts | 1 + apps/meteor/tests/data/interactions.ts | 1 + apps/meteor/tests/e2e/file-upload.spec.ts | 21 +++++++++ .../tests/e2e/fixtures/files/diagram.drawio | 13 +++++ apps/meteor/tests/end-to-end/api/09-rooms.js | 47 ++++++++++++++++--- packages/i18n/src/locales/en.i18n.json | 1 + 11 files changed, 100 insertions(+), 16 deletions(-) create mode 100644 .changeset/breezy-pens-sing.md create mode 100644 apps/meteor/tests/e2e/fixtures/files/diagram.drawio diff --git a/.changeset/breezy-pens-sing.md b/.changeset/breezy-pens-sing.md new file mode 100644 index 000000000000..0725999ef62b --- /dev/null +++ b/.changeset/breezy-pens-sing.md @@ -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 diff --git a/apps/meteor/app/api/server/lib/getUploadFormData.ts b/apps/meteor/app/api/server/lib/getUploadFormData.ts index 9b8f69fb3a66..85fc0658542d 100644 --- a/apps/meteor/app/api/server/lib/getUploadFormData.ts +++ b/apps/meteor/app/api/server/lib/getUploadFormData.ts @@ -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 = { file: Readable & { truncated: boolean }; fieldname: string; @@ -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(); @@ -83,7 +85,7 @@ export async function getUploadFormData< file, filename, encoding, - mimetype, + mimetype: getMimeType(filename), fieldname, fields, fileBuffer: Buffer.concat(fileChunks), diff --git a/apps/meteor/app/utils/lib/mimeTypes.ts b/apps/meteor/app/utils/lib/mimeTypes.ts index f2da185f84ba..909a955d6724 100644 --- a/apps/meteor/app/utils/lib/mimeTypes.ts +++ b/apps/meteor/app/utils/lib/mimeTypes.ts @@ -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 }; diff --git a/apps/meteor/client/views/room/body/hooks/useFileUploadDropTarget.ts b/apps/meteor/client/views/room/body/hooks/useFileUploadDropTarget.ts index 2427f7217401..414b91c52493 100644 --- a/apps/meteor/client/views/room/body/hooks/useFileUploadDropTarget.ts +++ b/apps/meteor/client/views/room/body/hooks/useFileUploadDropTarget.ts @@ -29,7 +29,7 @@ export const useFileUploadDropTarget = (): readonly [ const t = useTranslation(); - const fileUploadEnabled = useSetting('FileUpload_Enabled') as boolean; + const fileUploadEnabled = useSetting('FileUpload_Enabled'); const user = useUser(); const fileUploadAllowedForUser = useReactiveValue( useCallback(() => !roomCoordinator.readOnly(room._id, { username: user?.username }), [room._id, user?.username]), @@ -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 = new Set(); @@ -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; }); diff --git a/apps/meteor/client/views/room/composer/messageBox/MessageBoxActionsToolbar/hooks/useFileUploadAction.ts b/apps/meteor/client/views/room/composer/messageBox/MessageBoxActionsToolbar/hooks/useFileUploadAction.ts index aba008a353a5..03229c5dceb3 100644 --- a/apps/meteor/client/views/room/composer/messageBox/MessageBoxActionsToolbar/hooks/useFileUploadAction.ts +++ b/apps/meteor/client/views/room/composer/messageBox/MessageBoxActionsToolbar/hooks/useFileUploadAction.ts @@ -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('FileUpload_Enabled'); const fileInputRef = useFileInput(fileInputProps); const chat = useChat(); @@ -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; }); diff --git a/apps/meteor/server/settings/file-upload.ts b/apps/meteor/server/settings/file-upload.ts index 505520b73e57..90032266651c 100644 --- a/apps/meteor/server/settings/file-upload.ts +++ b/apps/meteor/server/settings/file-upload.ts @@ -23,6 +23,7 @@ export const createFileUploadSettings = () => type: 'string', public: true, i18nDescription: 'FileUpload_MediaTypeBlackListDescription', + alert: 'FileUpload_MediaTypeBlackList_Alert', }); await this.add('FileUpload_ProtectFiles', true, { diff --git a/apps/meteor/tests/data/interactions.ts b/apps/meteor/tests/data/interactions.ts index d14749181193..085d97d4ece3 100644 --- a/apps/meteor/tests/data/interactions.ts +++ b/apps/meteor/tests/data/interactions.ts @@ -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'; diff --git a/apps/meteor/tests/e2e/file-upload.spec.ts b/apps/meteor/tests/e2e/file-upload.spec.ts index b382476c0a2f..0a5d1cfd2512 100644 --- a/apps/meteor/tests/e2e/file-upload.spec.ts +++ b/apps/meteor/tests/e2e/file-upload.spec.ts @@ -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 }); @@ -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); }); @@ -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); }); @@ -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(); + }); }); diff --git a/apps/meteor/tests/e2e/fixtures/files/diagram.drawio b/apps/meteor/tests/e2e/fixtures/files/diagram.drawio new file mode 100644 index 000000000000..a86c2673ab98 --- /dev/null +++ b/apps/meteor/tests/e2e/fixtures/files/diagram.drawio @@ -0,0 +1,13 @@ + + + + + + + + + + + + + diff --git a/apps/meteor/tests/end-to-end/api/09-rooms.js b/apps/meteor/tests/end-to-end/api/09-rooms.js index 72bd5819593e..0809d06b324d 100644 --- a/apps/meteor/tests/end-to-end/api/09-rooms.js +++ b/apps/meteor/tests/end-to-end/api/09-rooms.js @@ -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'; @@ -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) @@ -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) @@ -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); diff --git a/packages/i18n/src/locales/en.i18n.json b/packages/i18n/src/locales/en.i18n.json index 836b534df601..1277a02375bf 100644 --- a/packages/i18n/src/locales/en.i18n.json +++ b/packages/i18n/src/locales/en.i18n.json @@ -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",