fix: custom emoji update issues (#32991)

Co-authored-by: Kevin Aleman <11577696+KevLehman@users.noreply.github.com> Co-authored-by: Tasso Evangelista <2263066+tassoevan@users.noreply.github.com>
RocketChat · Nov 14, 2024 · 80e36bf · 80e36bf
1 parent 7eb7941
commit 80e36bf
Show file tree

Hide file tree

Showing 14 changed files with 200 additions and 74 deletions.
diff --git a/.changeset/green-papayas-thank.md b/.changeset/green-papayas-thank.md
@@ -0,0 +1,7 @@
+---
+'@rocket.chat/model-typings': patch
+'@rocket.chat/core-typings': patch
+'@rocket.chat/meteor': patch
+---
+
+Fixes an issue where updating custom emojis didn’t work as expected, ensuring that uploaded emojis now update correctly and display without any caching problems.
diff --git a/apps/meteor/app/api/server/lib/getUploadFormData.ts b/apps/meteor/app/api/server/lib/getUploadFormData.ts
@@ -17,6 +17,14 @@ type UploadResult<K> = {
 	fields: K;
 };
 
+type UploadResultWithOptionalFile<K> =
+	| UploadResult<K>
+	| ({
+			[P in keyof Omit<UploadResult<K>, 'fields'>]: undefined;
+	  } & {
+			fields: K;
+	  });
+
 export async function getUploadFormData<
 	T extends string,
 	K extends Record<string, string> = Record<string, string>,
@@ -27,8 +35,37 @@ export async function getUploadFormData<
 		field?: T;
 		validate?: V;
 		sizeLimit?: number;
+		fileOptional: true;
+	},
+): Promise<UploadResultWithOptionalFile<K>>;
+
+export async function getUploadFormData<
+	T extends string,
+	K extends Record<string, string> = Record<string, string>,
+	V extends ValidateFunction<K> = ValidateFunction<K>,
+>(
+	{ request }: { request: Request },
+	options?: {
+		field?: T;
+		validate?: V;
+		sizeLimit?: number;
+		fileOptional?: false | undefined;
+	},
+): Promise<UploadResult<K>>;
+
+export async function getUploadFormData<
+	T extends string,
+	K extends Record<string, string> = Record<string, string>,
+	V extends ValidateFunction<K> = ValidateFunction<K>,
+>(
+	{ request }: { request: Request },
+	options: {
+		field?: T;
+		validate?: V;
+		sizeLimit?: number;
+		fileOptional?: boolean;
 	} = {},
-): Promise<UploadResult<K>> {
+): Promise<UploadResultWithOptionalFile<K>> {
 	const limits = {
 		files: 1,
 		...(options.sizeLimit && options.sizeLimit > -1 && { fileSize: options.sizeLimit }),
@@ -37,9 +74,9 @@ export async function getUploadFormData<
 	const bb = busboy({ headers: request.headers, defParamCharset: 'utf8', limits });
 	const fields = Object.create(null) as K;
 
-	let uploadedFile: UploadResult<K> | undefined;
+	let uploadedFile: UploadResultWithOptionalFile<K> | undefined;
 
-	let returnResult = (_value: UploadResult<K>) => {
+	let returnResult = (_value: UploadResultWithOptionalFile<K>) => {
 		// noop
 	};
 	let returnError = (_error?: Error | string | null | undefined) => {
@@ -48,10 +85,22 @@ export async function getUploadFormData<
 
 	function onField(fieldname: keyof K, value: K[keyof K]) {
 		fields[fieldname] = value;
+		uploadedFile = {
+			fields,
+			encoding: undefined,
+			filename: undefined,
+			fieldname: undefined,
+			mimetype: undefined,
+			fileBuffer: undefined,
+			file: undefined,
+		};
 	}
 
 	function onEnd() {
 		if (!uploadedFile) {
+			return returnError(new MeteorError('No file or fields were uploaded'));
+		}
+		if (!('file' in uploadedFile) && !options.fileOptional) {
 			return returnError(new MeteorError('No file uploaded'));
 		}
 		if (options.validate !== undefined && !options.validate(fields)) {
@@ -121,7 +170,7 @@ export async function getUploadFormData<
 
 	request.pipe(bb);
 
-	return new Promise((resolve, reject) => {
+	return new Promise<UploadResultWithOptionalFile<K>>((resolve, reject) => {
 		returnResult = resolve;
 		returnError = reject;
 	});

diff --git a/apps/meteor/app/api/server/v1/emoji-custom.ts b/apps/meteor/app/api/server/v1/emoji-custom.ts
@@ -1,9 +1,11 @@
 import { Media } from '@rocket.chat/core-services';
+import type { IEmojiCustom } from '@rocket.chat/core-typings';
 import { EmojiCustom } from '@rocket.chat/models';
 import { isEmojiCustomList } from '@rocket.chat/rest-typings';
 import { Meteor } from 'meteor/meteor';
 
 import { SystemLogger } from '../../../../server/lib/logger/system';
+import type { EmojiData } from '../../../emoji-custom/server/lib/insertOrUpdateEmoji';
 import { insertOrUpdateEmoji } from '../../../emoji-custom/server/lib/insertOrUpdateEmoji';
 import { uploadEmojiCustomWithBuffer } from '../../../emoji-custom/server/lib/uploadEmojiCustom';
 import { settings } from '../../../settings/server';
@@ -114,16 +116,15 @@ API.v1.addRoute(
 			fields.extension = extension;
 
 			try {
-				await Meteor.callAsync('insertOrUpdateEmoji', {
-					...fields,
-					newFile: true,
-					aliases: fields.aliases || '',
-				});
-				await Meteor.callAsync('uploadEmojiCustom', fileBuffer, mimetype, {
+				const emojiData = await insertOrUpdateEmoji(this.userId, {
 					...fields,
 					newFile: true,
 					aliases: fields.aliases || '',
+					name: fields.name,
+					extension: fields.extension,
 				});
+
+				await uploadEmojiCustomWithBuffer(this.userId, fileBuffer, mimetype, emojiData);
 			} catch (e) {
 				SystemLogger.error(e);
 				return API.v1.failure();
@@ -143,7 +144,7 @@ API.v1.addRoute(
 				{
 					request: this.request,
 				},
-				{ field: 'emoji', sizeLimit: settings.get('FileUpload_MaxFileSize') },
+				{ field: 'emoji', sizeLimit: settings.get('FileUpload_MaxFileSize'), fileOptional: true },
 			);
 
 			const { fields, fileBuffer, mimetype } = emoji;
@@ -152,41 +153,40 @@ API.v1.addRoute(
 				throw new Meteor.Error('The required "_id" query param is missing.');
 			}
 
-			const emojiToUpdate = await EmojiCustom.findOneById(fields._id);
+			const emojiToUpdate = await EmojiCustom.findOneById<Pick<IEmojiCustom, 'name' | 'extension'>>(fields._id, {
+				projection: { name: 1, extension: 1 },
+			});
 			if (!emojiToUpdate) {
 				throw new Meteor.Error('Emoji not found.');
 			}
 
-			fields.previousName = emojiToUpdate.name;
-			fields.previousExtension = emojiToUpdate.extension;
-			fields.aliases = fields.aliases || '';
-			const newFile = Boolean(emoji && fileBuffer.length);
+			const emojiData: EmojiData = {
+				previousName: emojiToUpdate.name,
+				previousExtension: emojiToUpdate.extension,
+				aliases: fields.aliases || '',
+				name: fields.name,
+				extension: fields.extension,
+				_id: fields._id,
+				newFile: false,
+			};
 
-			if (fields.newFile) {
+			const isNewFile = fileBuffer?.length && !!mimetype;
+			if (isNewFile) {
+				emojiData.newFile = isNewFile;
 				const isUploadable = await Media.isImage(fileBuffer);
 				if (!isUploadable) {
 					throw new Meteor.Error('emoji-is-not-image', "Emoji file provided cannot be uploaded since it's not an image");
 				}
 
 				const [, extension] = mimetype.split('/');
-				fields.extension = extension;
+				emojiData.extension = extension;
 			} else {
-				fields.extension = emojiToUpdate.extension;
+				emojiData.extension = emojiToUpdate.extension;
 			}
 
-			const emojiData = {
-				name: fields.name,
-				_id: fields._id,
-				aliases: fields.aliases,
-				extension: fields.extension,
-				previousName: fields.previousName,
-				previousExtension: fields.previousExtension,
-				newFile,
-			};
-
-			await insertOrUpdateEmoji(this.userId, emojiData);
-			if (fields.newFile) {
-				await uploadEmojiCustomWithBuffer(this.userId, fileBuffer, mimetype, emojiData);
+			const updatedEmojiData = await insertOrUpdateEmoji(this.userId, emojiData);
+			if (isNewFile) {
+				await uploadEmojiCustomWithBuffer(this.userId, fileBuffer, mimetype, updatedEmojiData);
 			}
 			return API.v1.success();
 		},

diff --git a/apps/meteor/app/emoji-custom/client/lib/emojiCustom.ts b/apps/meteor/app/emoji-custom/client/lib/emojiCustom.ts
@@ -1,7 +1,6 @@
 import type { IEmoji } from '@rocket.chat/core-typings';
 import { escapeRegExp } from '@rocket.chat/string-helpers';
 import { Meteor } from 'meteor/meteor';
-import { Session } from 'meteor/session';
 
 import { onLoggedIn } from '../../../../client/lib/loggedIn';
 import { emoji, removeFromRecent, replaceEmojiInRecent } from '../../../emoji/client';
@@ -18,16 +17,12 @@ const isSetNotNull = (fn: () => unknown) => {
 	return value !== null && value !== undefined;
 };
 
-const getEmojiUrlFromName = (name: string, extension: string) => {
-	if (name == null) {
+const getEmojiUrlFromName = (name: string, extension: string, etag?: string) => {
+	if (!name) {
 		return;
 	}
 
-	const key = `emoji_random_${name}` as const;
-
-	const random = (Session as unknown as { keys: Record<string, any> }).keys[key] ?? 0;
-
-	return getURL(`/emoji-custom/${encodeURIComponent(name)}.${extension}?_dc=${random}`);
+	return getURL(`/emoji-custom/${encodeURIComponent(name)}.${extension}${etag ? `?etag=${etag}` : ''}`);
 };
 
 export const deleteEmojiCustom = (emojiData: IEmoji) => {
@@ -126,6 +121,7 @@ const customRender = (html: string) => {
 		return `<span class="emoji" style="background-image:url(${getEmojiUrlFromName(
 			emojiAlias,
 			dataCheck.extension!,
+			dataCheck.etag,
 		)});" data-emoji="${emojiAlias}" title="${shortname}">${shortname}</span>`;
 	});
 

diff --git a/apps/meteor/app/emoji-custom/server/lib/insertOrUpdateEmoji.ts b/apps/meteor/app/emoji-custom/server/lib/insertOrUpdateEmoji.ts
@@ -2,16 +2,15 @@ import { api } from '@rocket.chat/core-services';
 import { EmojiCustom } from '@rocket.chat/models';
 import limax from 'limax';
 import { Meteor } from 'meteor/meteor';
-import _ from 'underscore';
 
 import { trim } from '../../../../lib/utils/stringUtils';
 import { hasPermissionAsync } from '../../../authorization/server/functions/hasPermission';
 import { RocketChatFileEmojiCustomInstance } from '../startup/emoji-custom';
 
-type EmojiData = {
+export type EmojiData = {
 	_id?: string;
 	name: string;
-	aliases: string;
+	aliases?: string;
 	extension: string;
 	previousName?: string;
 	previousExtension?: string;
@@ -33,7 +32,6 @@ export async function insertOrUpdateEmoji(userId: string | null, emojiData: Emoj
 	}
 
 	emojiData.name = limax(emojiData.name, { replacement: '_' });
-	emojiData.aliases = limax(emojiData.aliases, { replacement: '_' });
 
 	// allow all characters except colon, whitespace, comma, >, <, &, ", ', /, \, (, )
 	// more practical than allowing specific sets of characters; also allows foreign languages
@@ -42,7 +40,7 @@ export async function insertOrUpdateEmoji(userId: string | null, emojiData: Emoj
 
 	// silently strip colon; this allows for uploading :emojiname: as emojiname
 	emojiData.name = emojiData.name.replace(/:/g, '');
-	emojiData.aliases = emojiData.aliases.replace(/:/g, '');
+	emojiData.aliases = emojiData.aliases?.replace(/:/g, '');
 
 	if (nameValidation.test(emojiData.name)) {
 		throw new Meteor.Error('error-input-is-not-a-valid-field', `${emojiData.name} is not a valid name`, {
@@ -61,7 +59,11 @@ export async function insertOrUpdateEmoji(userId: string | null, emojiData: Emoj
 				field: 'Alias_Set',
 			});
 		}
-		aliases = _.without(emojiData.aliases.split(/[\s,]/).filter(Boolean), emojiData.name);
+		aliases = emojiData.aliases
+			.split(/\s*,\s*/)
+			.filter(Boolean)
+			.map((alias) => limax(alias, { replacement: '_' }))
+			.filter((alias) => alias !== emojiData.name);
 	}
 
 	emojiData.extension = emojiData.extension === 'svg+xml' ? 'png' : emojiData.extension;
@@ -119,7 +121,8 @@ export async function insertOrUpdateEmoji(userId: string | null, emojiData: Emoj
 		const rs = await RocketChatFileEmojiCustomInstance.getFileWithReadStream(
 			encodeURIComponent(`${emojiData.previousName}.${emojiData.previousExtension}`),
 		);
-		if (rs !== null) {
+
+		if (rs) {
 			await RocketChatFileEmojiCustomInstance.deleteFile(encodeURIComponent(`${emojiData.name}.${emojiData.extension}`));
 			const ws = RocketChatFileEmojiCustomInstance.createWriteStream(
 				encodeURIComponent(`${emojiData.name}.${emojiData.previousExtension}`),

diff --git a/apps/meteor/app/emoji-custom/server/lib/uploadEmojiCustom.ts b/apps/meteor/app/emoji-custom/server/lib/uploadEmojiCustom.ts
@@ -1,11 +1,14 @@
 import { api, Media } from '@rocket.chat/core-services';
+import { EmojiCustom } from '@rocket.chat/models';
+import { Random } from '@rocket.chat/random';
 import limax from 'limax';
 import { Meteor } from 'meteor/meteor';
 import sharp from 'sharp';
 
 import { hasPermissionAsync } from '../../../authorization/server/functions/hasPermission';
 import { RocketChatFile } from '../../../file/server';
 import { RocketChatFileEmojiCustomInstance } from '../startup/emoji-custom';
+import type { EmojiData } from './insertOrUpdateEmoji';
 
 const getFile = async (file: Buffer, extension: string) => {
 	if (extension !== 'svg+xml') {
@@ -15,35 +18,35 @@ const getFile = async (file: Buffer, extension: string) => {
 	return sharp(file).png().toBuffer();
 };
 
-type EmojiData = {
-	_id?: string;
-	name: string;
-	aliases?: string | string[];
-	extension: string;
-	previousName?: string;
-	previousExtension?: string;
-	newFile?: boolean;
-};
+export type EmojiDataWithAliases = Omit<EmojiData, 'aliases'> & { aliases?: string | string[] };
 
-export async function uploadEmojiCustom(userId: string | null, binaryContent: string, contentType: string, emojiData: EmojiData) {
+export async function uploadEmojiCustom(
+	userId: string | null,
+	binaryContent: string,
+	contentType: string,
+	emojiData: EmojiDataWithAliases,
+) {
 	return uploadEmojiCustomWithBuffer(userId, Buffer.from(binaryContent, 'binary'), contentType, emojiData);
 }
 
 export async function uploadEmojiCustomWithBuffer(
 	userId: string | null,
 	buffer: Buffer,
 	contentType: string,
-	emojiData: EmojiData,
+	emojiData: EmojiDataWithAliases,
 ): Promise<void> {
 	// technically, since this method doesnt have any datatype validations, users can
 	// upload videos as emojis. The FE won't play them, but they will waste space for sure.
 	if (!userId || !(await hasPermissionAsync(userId, 'manage-emoji'))) {
 		throw new Meteor.Error('not_authorized');
 	}
 
+	if (!Array.isArray(emojiData.aliases)) {
+		// delete aliases for notification purposes. here, it is a string or undefined rather than an array
+		delete emojiData.aliases;
+	}
+
 	emojiData.name = limax(emojiData.name, { replacement: '_' });
-	// delete aliases for notification purposes. here, it is a string rather than an array
-	delete emojiData.aliases;
 
 	const file = await getFile(buffer, emojiData.extension);
 	emojiData.extension = emojiData.extension === 'svg+xml' ? 'png' : emojiData.extension;
@@ -67,8 +70,10 @@ export async function uploadEmojiCustomWithBuffer(
 			encodeURIComponent(`${emojiData.name}.${emojiData.extension}`),
 			contentType,
 		);
-		ws.on('end', () => {
-			setTimeout(() => api.broadcast('emoji.updateCustom', emojiData), 500);
+		ws.on('end', async () => {
+			const etag = Random.hexString(6);
+			await EmojiCustom.setETagByName(emojiData.name, etag);
+			setTimeout(() => api.broadcast('emoji.updateCustom', { ...emojiData, etag }), 500);
 			resolve();
 		});
 

diff --git a/apps/meteor/app/emoji/lib/rocketchat.ts b/apps/meteor/app/emoji/lib/rocketchat.ts
@@ -31,13 +31,15 @@ export type EmojiPackages = {
 					aliases?: string[];
 					aliasOf?: undefined;
 					extension?: string;
+					etag?: string;
 			  }
 			| {
 					emojiPackage: string;
 					aliasOf: string;
 					extension?: undefined;
 					aliases?: undefined;
 					shortnames?: undefined;
+					etag?: string;
 			  };
 	};
 };