fix: upload of malformed jpegs with Message_Attachments_Strip_Exif disabled fails silently #35839

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

Merged

kodiakhq merged 22 commits into develop from SUP-769-Image-Upload-Display-Error-Message-on-Failure-Silent-Fail-for-Specific-JPEGs-ufs-VipsJpeg-errors

May 9, 2025

.changeset/rare-mails-whisper.md

-Original file line number
+Diff line change
@@ -0,0 +1,5 @@
+    ---
+    "@rocket.chat/meteor": patch
+    ---
+    Fixes an issue where the upload of malformed images failed silently with Message_Attachments_Strip_Exif disabled

apps/meteor/client/lib/chats/uploads.ts

            
                      Original file line number
                      Diff line number
                      Diff line change
                  
    @@ -49,14 +49,13 @@ const send = async (
  
    ): Promise<void> => {

    	const id = Random.id();

    	updateUploads((uploads) => [

    		...uploads,

    		{

    			id,

    			name: fileContent?.raw.name || file.name,

    			percentage: 0,

    		},

    	]);

    	const upload: Upload = {

    		id,

    		name: fileContent?.raw.name || file.name,

    		percentage: 0,

    	};

    	updateUploads((uploads) => [...uploads, upload]);

    	try {

    		await new Promise((resolve, reject) => {

    @@ -114,20 +113,28 @@ const send = async (
  
    			);

    			xhr.onload = async () => {

    				if (xhr.readyState === xhr.DONE && xhr.status === 200) {

    					const result = JSON.parse(xhr.responseText);

    					let content;

    					if (getContent) {

    						content = await getContent(result.file._id, result.file.url);

    				if (xhr.readyState === xhr.DONE) {

    					if (xhr.status === 400) {

    						const error = JSON.parse(xhr.responseText);

    						updateUploads((uploads) => [...uploads, { ...upload, error: new Error(error.error) }]);

    						return;

    					}

    					await sdk.rest.post(`/v1/rooms.mediaConfirm/${rid}/${result.file._id}`, {

    						msg,

    						tmid,

    						description,

    						t,

    						content,

    					});

    					if (xhr.status === 200) {

    						const result = JSON.parse(xhr.responseText);

    						let content;

    						if (getContent) {

    							content = await getContent(result.file._id, result.file.url);

    						}

    						await sdk.rest.post(`/v1/rooms.mediaConfirm/${rid}/${result.file._id}`, {

    							msg,

    							tmid,

    							description,

    							t,

    							content,

    						});

    					}

    				}

    			};

apps/meteor/client/views/room/body/UploadProgress/UploadProgressIndicator.tsx

            
                      Original file line number
                      Diff line number
                      Diff line change
                  
    @@ -1,7 +1,7 @@
  
    import { css } from '@rocket.chat/css-in-js';

    import { Box, Button, Palette } from '@rocket.chat/fuselage';

    import type { ReactElement } from 'react';

    import { useCallback } from 'react';

    import { useCallback, useMemo } from 'react';

    import { useTranslation } from 'react-i18next';

    import type { Upload } from '../../../../lib/chats/Upload';

    @@ -34,6 +34,14 @@ const UploadProgressIndicator = ({ id, name, percentage, error, onClose }: Uploa
  
    		onClose?.(id);

    	}, [id, onClose]);

    	const uploadProgressTitle = useMemo(() => {

    		if (error) {

    			return `${error} ${name}`;

    		}

    		return `[${percentage}%] ${t('Uploading_file__fileName__', { fileName: name })}`;

    	}, [error, name, percentage, t]);

    	return (

    		<Box

    			pb={4}

    @@ -52,8 +60,8 @@ const UploadProgressIndicator = ({ id, name, percentage, error, onClose }: Uploa
  
    			bg='surface-tint'

    			className={customClass}

    		>

    			<Box withTruncatedText zIndex={2} borderRadius={4}>

    				[{percentage}%] {name}

    			<Box role='status' withTruncatedText zIndex={2} borderRadius={4}>

    				{uploadProgressTitle}

    			</Box>

    			<Button zIndex={3} small onClick={handleCloseClick}>

    				{t('Cancel')}

apps/meteor/tests/e2e/fixtures/files/bad-orientation.jpeg

Sorry, something went wrong. Reload?

Sorry, we cannot display this file.

Sorry, this file is invalid so it cannot be displayed.

apps/meteor/tests/e2e/image-upload.spec.ts

dougfabris marked this conversation as resolved.

Show resolved Hide resolved

-Original file line number
+Diff line change
@@ -0,0 +1,58 @@
+    import { Users } from './fixtures/userStates';
+    import { HomeChannel } from './page-objects';
+    import { createTargetChannel, getSettingValueById, setSettingValueById } from './utils';
+    import { test, expect } from './utils/test';
+    test.use({ storageState: Users.user1.state });
+    test.describe('image-upload', () => {
+    	let settingDefaultValue: unknown;
+    	let poHomeChannel: HomeChannel;
+    	let targetChannel: string;
+    	test.beforeAll(async ({ api }) => {
+    		settingDefaultValue = await getSettingValueById(api, 'Message_Attachments_Strip_Exif');
+    		targetChannel = await createTargetChannel(api, { members: ['user1'] });
+    	});
+    	test.beforeEach(async ({ page }) => {
+    		poHomeChannel = new HomeChannel(page);
+    		await page.goto('/home');
+    		await poHomeChannel.sidenav.openChat(targetChannel);
+    	});
+    	test.afterAll(async ({ api }) => {
+    		await setSettingValueById(api, 'Message_Attachments_Strip_Exif', settingDefaultValue);
+    		expect((await api.post('/channels.delete', { roomName: targetChannel })).status()).toBe(200);
+    	});
+    	test.describe('strip exif disabled', () => {
+    		test.beforeAll(async ({ api }) => {
+    			await setSettingValueById(api, 'Message_Attachments_Strip_Exif', false);
+    		});
+    		test('should show error indicator when upload fails', async () => {
+    			await poHomeChannel.content.sendFileMessage('bad-orientation.jpeg');
+    			await poHomeChannel.content.fileNameInput.fill('bad-orientation.jpeg');
+    			await poHomeChannel.content.descriptionInput.fill('bad-orientation_description');
+    			await poHomeChannel.content.btnModalConfirm.click();
+    			await expect(poHomeChannel.statusUploadIndicator).toContainText('Error:');
+    		});
+    	});
+    	test.describe('strip exif enabled', () => {
+    		test.beforeAll(async ({ api }) => {
+    			await setSettingValueById(api, 'Message_Attachments_Strip_Exif', true);
+    		});
+    		test('should succeed upload of bad-orientation.jpeg', async () => {
+    			await poHomeChannel.content.sendFileMessage('bad-orientation.jpeg');
+    			await poHomeChannel.content.fileNameInput.fill('bad-orientation.jpeg');
+    			await poHomeChannel.content.descriptionInput.fill('bad-orientation_description');
+    			await poHomeChannel.content.btnModalConfirm.click();
+    			await expect(poHomeChannel.content.getFileDescription).toHaveText('bad-orientation_description');
+    		});
+    	});
+    });

apps/meteor/tests/e2e/page-objects/home-channel.ts

-Original file line number
+Diff line change
@@ Expand Up / @@ -132,4 +132,8 @@ export class HomeChannel { @@
     	get btnJoinRoom(): Locator {
     		return this.page.getByRole('button', { name: 'Join' });
     	}
+    	get statusUploadIndicator(): Locator {
+    		return this.page.getByRole('main').getByRole('status');
+    	}
     }

packages/i18n/src/locales/en.i18n.json

-Original file line number
+Diff line change
@@ Expand Up / @@ -5295,6 +5295,7 @@ @@
       "Upload_private_app": "Upload private app",
       "Upload_user_avatar": "Upload avatar",
       "Uploading_file": "Uploading file...",
+      "Uploading_file__fileName__": "Uploading file {{fileName}}",
       "Uploads": "Uploads",
       "Uptime": "Uptime",
       "Usage": "Usage",
@@ Expand Down @@

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: upload of malformed jpegs with Message_Attachments_Strip_Exif disabled fails silently #35839

Uh oh!

Diff view

Diff view

There are no files selected for viewing

Uh oh!

Uh oh!