Skip to content
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

fix: custom emoji update issues #32991

Merged
merged 18 commits into from
Nov 14, 2024
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/green-papayas-thank.md
Original file line number Diff line number Diff line change
@@ -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.
57 changes: 53 additions & 4 deletions apps/meteor/app/api/server/lib/getUploadFormData.ts
abhinavkrin marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand All @@ -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 }),
Expand All @@ -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) => {
Expand All @@ -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)) {
Expand Down Expand Up @@ -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;
});
Expand Down
56 changes: 28 additions & 28 deletions apps/meteor/app/api/server/v1/emoji-custom.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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();
Expand All @@ -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;
Expand All @@ -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();
},
Expand Down
5 changes: 3 additions & 2 deletions apps/meteor/app/emoji-custom/client/lib/emojiCustom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const isSetNotNull = (fn: () => unknown) => {
return value !== null && value !== undefined;
};

const getEmojiUrlFromName = (name: string, extension: string) => {
const getEmojiUrlFromName = (name: string, extension: string, etag?: string) => {
if (name == null) {
return;
}
Expand All @@ -27,7 +27,7 @@ const getEmojiUrlFromName = (name: string, extension: string) => {

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}?_dc=${random}${etag ? `&etag=${etag}` : ''}`);
};

export const deleteEmojiCustom = (emojiData: IEmoji) => {
Expand Down Expand Up @@ -123,6 +123,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>`;
});

Expand Down
17 changes: 10 additions & 7 deletions apps/meteor/app/emoji-custom/server/lib/insertOrUpdateEmoji.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
KevLehman marked this conversation as resolved.
Show resolved Hide resolved
_id?: string;
name: string;
aliases: string;
aliases?: string;
extension: string;
previousName?: string;
previousExtension?: string;
Expand All @@ -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
Expand All @@ -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`, {
Expand All @@ -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;
Expand Down Expand Up @@ -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}`),
Expand Down
35 changes: 20 additions & 15 deletions apps/meteor/app/emoji-custom/server/lib/uploadEmojiCustom.ts
Original file line number Diff line number Diff line change
@@ -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') {
Expand All @@ -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)) {
KevLehman marked this conversation as resolved.
Show resolved Hide resolved
// 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;
Expand All @@ -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);
KevLehman marked this conversation as resolved.
Show resolved Hide resolved
setTimeout(() => api.broadcast('emoji.updateCustom', { ...emojiData, etag }), 500);
resolve();
});

Expand Down
2 changes: 2 additions & 0 deletions apps/meteor/app/emoji/lib/rocketchat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};
};
};
Loading
Loading