fix: File Upload description bypassing message maximum character limi…

…t setting (#33218)
RocketChat · Nov 18, 2024 · 6c83bf0 · 6c83bf0
1 parent b56d4c5
commit 6c83bf0
Show file tree

Hide file tree

Showing 7 changed files with 154 additions and 28 deletions.
diff --git a/.changeset/lemon-foxes-carry.md b/.changeset/lemon-foxes-carry.md
@@ -0,0 +1,6 @@
+---
+"@rocket.chat/meteor": patch
+"@rocket.chat/i18n": patch
+---
+
+Fixes message character limit not being applied to file upload descriptions
diff --git a/apps/meteor/app/api/server/v1/rooms.ts b/apps/meteor/app/api/server/v1/rooms.ts
@@ -196,6 +196,10 @@ API.v1.addRoute(
 			const fileStore = FileUpload.getStore('Uploads');
 			const uploadedFile = await fileStore.insert(details, fileBuffer);
 
+			if ((fields.description?.length ?? 0) > settings.get<number>('Message_MaxAllowedSize')) {
+				throw new Meteor.Error('error-message-size-exceeded');
+			}
+
 			uploadedFile.description = fields.description;
 
 			delete fields.description;
@@ -299,6 +303,10 @@ API.v1.addRoute(
 				throw new Meteor.Error('invalid-file');
 			}
 
+			if ((this.bodyParams.description?.length ?? 0) > settings.get<number>('Message_MaxAllowedSize')) {
+				throw new Meteor.Error('error-message-size-exceeded');
+			}
+
 			file.description = this.bodyParams.description;
 			delete this.bodyParams.description;
 

diff --git a/apps/meteor/client/views/room/modals/FileUploadModal/FileUploadModal.spec.tsx b/apps/meteor/client/views/room/modals/FileUploadModal/FileUploadModal.spec.tsx
@@ -0,0 +1,36 @@
+import { mockAppRoot } from '@rocket.chat/mock-providers';
+import { render, screen } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
+import React from 'react';
+
+import FileUploadModal from './FileUploadModal';
+
+const defaultProps = {
+	onClose: () => undefined,
+	file: new File([], 'testing.png'),
+	fileName: 'Testing',
+	fileDescription: '',
+	onSubmit: () => undefined,
+	invalidContentType: false,
+	showDescription: true,
+};
+
+it('should show Undo request button when roomOpen is true and transcriptRequest exist', async () => {
+	render(<FileUploadModal {...defaultProps} />, {
+		legacyRoot: true,
+		wrapper: mockAppRoot()
+			.withTranslations('en', 'core', {
+				Cannot_upload_file_character_limit: 'Cannot upload file, description is over the {{count}} character limit',
+				Send: 'Send',
+				Upload_file_description: 'File description',
+			})
+			.withSetting('Message_MaxAllowedSize', 10)
+			.build(),
+	});
+
+	const input = await screen.findByRole('textbox', { name: 'File description' });
+	expect(input).toBeInTheDocument();
+	await userEvent.type(input, '12345678910');
+
+	expect(screen.getByText('Cannot upload file, description is over the 10 character limit')).toBeInTheDocument();
+});
diff --git a/apps/meteor/client/views/room/modals/FileUploadModal/FileUploadModal.tsx b/apps/meteor/client/views/room/modals/FileUploadModal/FileUploadModal.tsx
@@ -1,9 +1,9 @@
 import { Modal, Box, Field, FieldGroup, FieldLabel, FieldRow, FieldError, TextInput, Button } from '@rocket.chat/fuselage';
-import { useAutoFocus } from '@rocket.chat/fuselage-hooks';
 import { useToastMessageDispatch, useTranslation, useSetting } from '@rocket.chat/ui-contexts';
 import fileSize from 'filesize';
-import type { ReactElement, ChangeEvent, FormEventHandler, ComponentProps } from 'react';
-import React, { memo, useState, useEffect } from 'react';
+import type { ReactElement, ComponentProps } from 'react';
+import React, { memo, useEffect } from 'react';
+import { useForm } from 'react-hook-form';
 
 import FilePreview from './FilePreview';
 
@@ -26,31 +26,21 @@ const FileUploadModal = ({
 	invalidContentType,
 	showDescription = true,
 }: FileUploadModalProps): ReactElement => {
-	const [name, setName] = useState<string>(fileName);
-	const [description, setDescription] = useState<string>(fileDescription || '');
+	const {
+		register,
+		handleSubmit,
+		formState: { errors, isValid },
+	} = useForm({ mode: 'onChange', defaultValues: { name: fileName, description: fileDescription } });
+
 	const t = useTranslation();
 	const dispatchToastMessage = useToastMessageDispatch();
+	const maxMsgSize = useSetting('Message_MaxAllowedSize', 5000);
 	const maxFileSize = useSetting('FileUpload_MaxFileSize', 104857600);
 
-	const ref = useAutoFocus<HTMLInputElement>();
-
-	const handleName = (e: ChangeEvent<HTMLInputElement>): void => {
-		setName(e.currentTarget.value);
-	};
-
-	const handleDescription = (e: ChangeEvent<HTMLInputElement>): void => {
-		setDescription(e.currentTarget.value);
-	};
-
-	const handleSubmit: FormEventHandler<HTMLFormElement> = (e): void => {
-		e.preventDefault();
-		if (!name) {
-			return dispatchToastMessage({
-				type: 'error',
-				message: t('Required_field', { field: t('Upload_file_name') }),
-			});
-		}
+	const isDescriptionValid = (description: string) =>
+		description.length >= maxMsgSize ? t('Cannot_upload_file_character_limit', { count: maxMsgSize }) : true;
 
+	const submit = ({ name, description }: { name: string; description?: string }): void => {
 		// -1 maxFileSize means there is no limit
 		if (maxFileSize > -1 && (file.size || 0) > maxFileSize) {
 			onClose();
@@ -83,7 +73,7 @@ const FileUploadModal = ({
 	}, [file, dispatchToastMessage, invalidContentType, t, onClose]);
 
 	return (
-		<Modal wrapperFunction={(props: ComponentProps<typeof Box>) => <Box is='form' onSubmit={handleSubmit} {...props} />}>
+		<Modal wrapperFunction={(props: ComponentProps<typeof Box>) => <Box is='form' onSubmit={handleSubmit(submit)} {...props} />}>
 			<Box display='flex' flexDirection='column' height='100%'>
 				<Modal.Header>
 					<Modal.Title>{t('FileUpload')}</Modal.Title>
@@ -97,16 +87,26 @@ const FileUploadModal = ({
 						<Field>
 							<FieldLabel>{t('Upload_file_name')}</FieldLabel>
 							<FieldRow>
-								<TextInput value={name} onChange={handleName} />
+								<TextInput
+									{...register('name', {
+										required: t('error-the-field-is-required', { field: t('Name') }),
+									})}
+								/>
 							</FieldRow>
-							{!name && <FieldError>{t('Required_field', { field: t('Upload_file_name') })}</FieldError>}
+							<FieldError>{errors.name?.message}</FieldError>
 						</Field>
 						{showDescription && (
 							<Field>
 								<FieldLabel>{t('Upload_file_description')}</FieldLabel>
 								<FieldRow>
-									<TextInput value={description} onChange={handleDescription} placeholder={t('Description')} ref={ref} />
+									<TextInput
+										{...register('description', {
+											validate: (value) => isDescriptionValid(value || ''),
+										})}
+										aria-label={t('Upload_file_description')}
+									/>
 								</FieldRow>
+								<FieldError>{errors.description?.message}</FieldError>
 							</Field>
 						)}
 					</FieldGroup>
@@ -116,7 +116,7 @@ const FileUploadModal = ({
 						<Button secondary onClick={onClose}>
 							{t('Cancel')}
 						</Button>
-						<Button primary type='submit'>
+						<Button primary type='submit' disabled={!isValid}>
 							{t('Send')}
 						</Button>
 					</Modal.FooterControllers>

diff --git a/apps/meteor/tests/e2e/file-upload.spec.ts b/apps/meteor/tests/e2e/file-upload.spec.ts
@@ -76,6 +76,7 @@ test.describe.serial('file-upload', () => {
 		await expect(poHomeChannel.content.btnModalConfirm).not.toBeVisible();
 	});
 });
+
 test.describe('file-upload-not-member', () => {
 	let poHomeChannel: HomeChannel;
 	let targetChannel: string;

diff --git a/apps/meteor/tests/end-to-end/api/rooms.ts b/apps/meteor/tests/end-to-end/api/rooms.ts
@@ -550,7 +550,81 @@ describe('[Rooms]', () => {
 					expect(res.body.message.files[0]).to.have.property('name', 'lst-test.lst');
 				});
 		});
+		describe('/rooms.media - Max allowed size', () => {
+			before(async () => updateSetting('Message_MaxAllowedSize', 10));
+			after(async () => updateSetting('Message_MaxAllowedSize', 5000));
+			it('should allow uploading a file with description under the max character limit', async () => {
+				await request
+					.post(api(`rooms.media/${testChannel._id}`))
+					.set(credentials)
+					.attach('file', imgURL)
+					.expect('Content-Type', 'application/json')
+					.expect(200)
+					.expect((res) => {
+						expect(res.body).to.have.property('success', true);
+						expect(res.body).to.have.property('file');
+						expect(res.body.file).to.have.property('_id');
+						expect(res.body.file).to.have.property('url');
+
+						fileNewUrl = res.body.file.url;
+						fileOldUrl = res.body.file.url.replace('/file-upload/', '/ufs/GridFS:Uploads/');
+						fileId = res.body.file._id;
+					});
+
+				await request
+					.post(api(`rooms.mediaConfirm/${testChannel._id}/${fileId}`))
+					.set(credentials)
+					.send({
+						description: '123456789',
+					})
+					.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('image_type', 'image/png');
+						expect(res.body.message.attachments[0]).to.have.property('title', '1024x1024.png');
+						expect(res.body.message).to.have.property('files');
+						expect(res.body.message.files).to.be.an('array').of.length(2);
+						expect(res.body.message.files[0]).to.have.property('type', 'image/png');
+						expect(res.body.message.files[0]).to.have.property('name', '1024x1024.png');
+					});
+			});
 
+			it('should not allow uploading a file with description over the max character limit', async () => {
+				await request
+					.post(api(`rooms.media/${testChannel._id}`))
+					.set(credentials)
+					.attach('file', imgURL)
+					.expect('Content-Type', 'application/json')
+					.expect(200)
+					.expect((res) => {
+						expect(res.body).to.have.property('success', true);
+						expect(res.body).to.have.property('file');
+						expect(res.body.file).to.have.property('_id');
+						expect(res.body.file).to.have.property('url');
+
+						fileNewUrl = res.body.file.url;
+						fileOldUrl = res.body.file.url.replace('/file-upload/', '/ufs/GridFS:Uploads/');
+						fileId = res.body.file._id;
+					});
+
+				await request
+					.post(api(`rooms.mediaConfirm/${testChannel._id}/${fileId}`))
+					.set(credentials)
+					.send({
+						description: '12345678910',
+					})
+					.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-message-size-exceeded');
+					});
+			});
+		});
 		it('should not allow uploading a blocked media type to a room', async () => {
 			await updateSetting('FileUpload_MediaTypeBlackList', 'text/plain');
 			await request

diff --git a/packages/i18n/src/locales/en.i18n.json b/packages/i18n/src/locales/en.i18n.json
@@ -955,6 +955,7 @@
   "Cannot_open_conversation_with_yourself": "Cannot Direct Message with yourself",
   "Cannot_share_your_location": "Cannot share your location...",
   "Cannot_disable_while_on_call": "Can't change status during calls ",
+  "Cannot_upload_file_character_limit": "Cannot upload file, description is over the {{count}} character limit",
   "Cant_join": "Can't join",
   "CAS": "CAS",
   "CAS_Description": "Central Authentication Service allows members to use one set of credentials to sign in to multiple sites over multiple protocols.",