Skip to content
Merged
Show file tree
Hide file tree
Changes from 11 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/gold-kids-move.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@rocket.chat/meteor": patch
"@rocket.chat/core-services": patch
"@rocket.chat/i18n": patch
---

Changes OEmbed URL processing. Now, the process is asynchronously and has a configurable timeout for each request. Additionally, the `API_EmbedIgnoredHosts` setting now accepts wildcard domains.
4 changes: 2 additions & 2 deletions apps/meteor/app/api/server/v1/chat.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Message } from '@rocket.chat/core-services';
import type { IMessage, IThreadMainMessage } from '@rocket.chat/core-typings';
import { MessageTypes } from '@rocket.chat/message-types';
import { Messages, Users, Rooms, Subscriptions } from '@rocket.chat/models';
Expand Down Expand Up @@ -48,7 +49,6 @@ import { executeUpdateMessage } from '../../../lib/server/methods/updateMessage'
import { applyAirGappedRestrictionsValidation } from '../../../license/server/airGappedRestrictionsWrapper';
import { pinMessage, unpinMessage } from '../../../message-pin/server/pinMessage';
import { starMessage } from '../../../message-star/server/starMessage';
import { OEmbed } from '../../../oembed/server/server';
import { executeSetReaction } from '../../../reactions/server/setReaction';
import { settings } from '../../../settings/server';
import { followMessage } from '../../../threads/server/methods/followMessage';
Expand Down Expand Up @@ -914,7 +914,7 @@ API.v1.addRoute(
throw new Meteor.Error('error-not-allowed', 'Not allowed');
}

const { urlPreview } = await OEmbed.parseUrl(url);
const { urlPreview } = await Message.parseOEmbedUrl(url);
urlPreview.ignoreParse = true;

return API.v1.success({ urlPreview });
Expand Down
5 changes: 1 addition & 4 deletions apps/meteor/app/lib/server/functions/sendMessage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { hasPermissionAsync } from '../../../authorization/server/functions/hasP
import { FileUpload } from '../../../file-upload/server';
import { settings } from '../../../settings/server';
import { afterSaveMessage } from '../lib/afterSaveMessage';
import { notifyOnRoomChangedById, notifyOnMessageChange } from '../lib/notifyListener';
import { notifyOnRoomChangedById } from '../lib/notifyListener';
import { validateCustomMessageFields } from '../lib/validateCustomMessageFields';

// TODO: most of the types here are wrong, but I don't want to change them now
Expand Down Expand Up @@ -286,11 +286,8 @@ export const sendMessage = async function (user: any, message: any, room: any, u
void Apps.getBridges()?.getListenerBridge().messageEvent(messageEvent, message);
}

// TODO: is there an opportunity to send returned data to notifyOnMessageChange?
await afterSaveMessage(message, room, user);

void notifyOnMessageChange({ id: message._id });

void notifyOnRoomChangedById(message.rid);

return message;
Expand Down
11 changes: 2 additions & 9 deletions apps/meteor/app/lib/server/functions/updateMessage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { Meteor } from 'meteor/meteor';
import { parseUrlsInMessage } from './parseUrlsInMessage';
import { settings } from '../../../settings/server';
import { afterSaveMessage } from '../lib/afterSaveMessage';
import { notifyOnRoomChangedById, notifyOnMessageChange } from '../lib/notifyListener';
import { notifyOnRoomChangedById } from '../lib/notifyListener';
import { validateCustomMessageFields } from '../lib/validateCustomMessageFields';

export const updateMessage = async function (
Expand Down Expand Up @@ -97,14 +97,7 @@ export const updateMessage = async function (
return;
}

// although this is an "afterSave" kind callback, we know they can extend message's properties
// so we wait for it to run before broadcasting
const data = await afterSaveMessage(msg, room, user);

void notifyOnMessageChange({
id: msg._id,
data,
});
await afterSaveMessage(msg, room, user);

if (room?.lastMessage?._id === msg._id) {
void notifyOnRoomChangedById(message.rid);
Expand Down
9 changes: 7 additions & 2 deletions apps/meteor/app/lib/server/lib/afterSaveMessage.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { Message } from '@rocket.chat/core-services';
import type { IMessage, IUser, IRoom } from '@rocket.chat/core-typings';
import type { Updater } from '@rocket.chat/models';
import { Rooms } from '@rocket.chat/models';
Expand All @@ -6,14 +7,16 @@ import { callbacks } from '../../../../server/lib/callbacks';

export async function afterSaveMessage(message: IMessage, room: IRoom, user: IUser, roomUpdater?: Updater<IRoom>): Promise<IMessage> {
const updater = roomUpdater ?? Rooms.getUpdater();
const data = await callbacks.run('afterSaveMessage', message, { room, user, roomUpdater: updater });
const data: IMessage = (await callbacks.run('afterSaveMessage', message, { room, user, roomUpdater: updater })) as unknown as IMessage;

if (!roomUpdater && updater.hasChanges()) {
await Rooms.updateFromUpdater({ _id: room._id }, updater);
}

void Message.afterSave({ message: data });

// TODO: Fix type - callback configuration needs to be updated
return data as unknown as IMessage;
return data;
}

export function afterSaveMessageAsync(message: IMessage, room: IRoom, user: IUser, roomUpdater: Updater<IRoom> = Rooms.getUpdater()): void {
Expand All @@ -22,4 +25,6 @@ export function afterSaveMessageAsync(message: IMessage, room: IRoom, user: IUse
if (roomUpdater.hasChanges()) {
void Rooms.updateFromUpdater({ _id: room._id }, roomUpdater);
}

void Message.afterSave({ message });
}
2 changes: 0 additions & 2 deletions apps/meteor/app/oembed/server/index.ts

This file was deleted.

1 change: 0 additions & 1 deletion apps/meteor/server/importPackages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ import '../app/message-pin/server';
import '../app/message-star/server';
import '../app/nextcloud/server';
import '../app/oauth2-server-config/server';
import '../app/oembed/server';
import '../app/push-notifications/server';
import '../app/retention-policy/server';
import '../app/slackbridge/server';
Expand Down
12 changes: 0 additions & 12 deletions apps/meteor/server/lib/callbacks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ import type {
ILivechatInquiryRecord,
ILivechatVisitor,
VideoConference,
OEmbedMeta,
OEmbedUrlContent,
IOmnichannelRoom,
ILivechatTag,
ILivechatTagRecord,
Expand Down Expand Up @@ -164,16 +162,6 @@ type ChainedCallbackSignatures = {
BusinessHourBehaviorClass: { new (): IBusinessHourBehavior };
};
'renderMessage': <T extends IMessage & { html: string }>(message: T) => T;
'oembed:beforeGetUrlContent': (data: { urlObj: URL }) => {
urlObj: URL;
headerOverrides?: { [k: string]: string };
};
'oembed:afterParseContent': (data: { url: string; meta: OEmbedMeta; headers: { [k: string]: string }; content: OEmbedUrlContent }) => {
url: string;
meta: OEmbedMeta;
headers: { [k: string]: string };
content: OEmbedUrlContent;
};
'livechat.beforeListTags': () => ILivechatTag[];
'livechat.offlineMessage': (data: { name: string; email: string; message: string; department?: string; host?: string }) => void;
'livechat.leadCapture': (room: IOmnichannelRoom) => IOmnichannelRoom;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,25 +1,26 @@
import type {
OEmbedUrlContentResult,
MessageUrl,
OEmbedUrlWithMetadata,
OEmbedMeta,
IMessage,
MessageAttachment,

Check failure on line 7 in apps/meteor/server/services/messages/hooks/AfterSaveOEmbed.ts

View workflow job for this annotation

GitHub Actions / 🔎 Code Check / Code Lint

'MessageAttachment' is defined but never used
OEmbedMeta,
MessageUrl,
OEmbedUrlContent,
} from '@rocket.chat/core-typings';
import { isOEmbedUrlWithMetadata } from '@rocket.chat/core-typings';
import { Logger } from '@rocket.chat/logger';
import { Messages, OEmbedCache } from '@rocket.chat/models';
import { OEmbedCache, Messages } from '@rocket.chat/models';
import { serverFetch as fetch } from '@rocket.chat/server-fetch';
import { camelCase } from 'change-case';
import he from 'he';
import iconv from 'iconv-lite';
import ipRangeCheck from 'ip-range-check';
import jschardet from 'jschardet';
import { camelCase } from 'lodash';

import { isURL } from '../../../lib/utils/isURL';
import { callbacks } from '../../../server/lib/callbacks';
import { settings } from '../../settings/server';
import { Info } from '../../utils/rocketchat.info';
import { settings } from '../../../../app/settings/server';
import { Info } from '../../../../app/utils/rocketchat.info';
import { isURL } from '../../../../lib/utils/isURL';
import { afterParseUrlContent, beforeGetUrlContent } from '../lib/oembed/providers';

const MAX_EXTERNAL_URL_PREVIEWS = 5;
const log = new Logger('OEmbed');
Expand Down Expand Up @@ -65,7 +66,7 @@
return iconv.decode(body, getCharset(contentType, body));
};

const getUrlContent = async (urlObj: URL, redirectCount = 5): Promise<OEmbedUrlContentResult> => {
const getUrlContent = async (urlObj: URL, redirectCount = 5): Promise<OEmbedUrlContent> => {
const portsProtocol = new Map<string, string>(
Object.entries({
80: 'http:',
Expand All @@ -75,8 +76,40 @@
);

const ignoredHosts = settings.get<string>('API_EmbedIgnoredHosts').replace(/\s/g, '').split(',') || [];
if (urlObj.hostname && (ignoredHosts.includes(urlObj.hostname) || ipRangeCheck(urlObj.hostname, ignoredHosts))) {
throw new Error('invalid host');

const isIgnoredHost = (hostname: string | null): boolean => {
if (!hostname || !ignoredHosts.length) {
return false;
}

const exactHosts = ignoredHosts.filter((h) => !h.includes('*'));
if (exactHosts.includes(hostname) || ipRangeCheck(hostname, exactHosts)) {
return true;
}

return ignoredHosts
.filter((h) => h.includes('*'))
.some((pattern) => {
const validationRegex = /^(?:\*\.)?(?:\*|[a-z0-9-]+)(?:\.(?:\*|[a-z0-9-]+))*$/i;
if (!validationRegex.test(pattern) || pattern === '*') {
return false;
}

const escaped = pattern.replace(/[-/\\^$+?.()|[\]{}]/g, '\\$&');
const source = `^${escaped.replace(/\*/g, '[^.]*')}$`;

try {
const regex = new RegExp(source, 'i');
return regex.test(hostname);
} catch {
// fail safe on invalid patterns
return false;
}
});
};

if (isIgnoredHost(urlObj.hostname)) {
throw new Error('host is ignored');
}

const safePorts = settings.get<string>('API_EmbedSafePorts').replace(/\s/g, '').split(',') || [];
Expand All @@ -91,14 +124,13 @@
throw new Error('invalid/unsafe port');
}

const data = await callbacks.run('oembed:beforeGetUrlContent', {
urlObj,
});
const data = beforeGetUrlContent({ urlObj });

const url = data.urlObj.toString();
const sizeLimit = 250000;

log.debug({ msg: 'Fetching URL for OEmbed', url, redirectCount });
const start = Date.now();
const response = await fetch(
url,
{
Expand All @@ -109,10 +141,12 @@
'Accept-Language': settings.get('Language') || 'en',
...data.headerOverrides,
},
timeout: settings.get<number>('API_EmbedTimeout') * 1000,
size: sizeLimit, // max size of the response body, this was not working as expected so I'm also manually verifying that on the iterator
},
settings.get('Allow_Invalid_SelfSigned_Certs'),
);
const end = Date.now();

let totalSize = 0;
const chunks = [];
Expand All @@ -126,13 +160,14 @@
}
}

log.debug({ msg: 'Obtained response from server', length: totalSize });
log.debug({ msg: 'Obtained response from server', length: totalSize, timeSpent: `${end - start}ms` });
const buffer = Buffer.concat(chunks);

return {
headers: Object.fromEntries(response.headers),
body: toUtf8(response.headers.get('content-type') || 'text/plain', buffer),
statusCode: response.status,
urlObj,
};
};

Expand Down Expand Up @@ -183,7 +218,7 @@
}

log.debug({ msg: 'Fetching URL content', url: urlObj.toString() });
let content: OEmbedUrlContentResult | undefined;
let content: OEmbedUrlContent | undefined;
try {
content = await getUrlContent(urlObj, 5);
} catch (err) {
Expand All @@ -195,7 +230,7 @@
}

log.debug({ msg: 'Parsing metadata for URL', url });
const metas: { [k: string]: string } = {};
const metas: OEmbedMeta = {} as any;

if (content?.body) {
const escapeMeta = (name: string, value: string): string => {
Expand Down Expand Up @@ -233,7 +268,7 @@
if (content && content.statusCode !== 200) {
return;
}
return callbacks.run('oembed:afterParseContent', {
return afterParseUrlContent({
url,
meta: metas,
headers,
Expand Down Expand Up @@ -289,6 +324,10 @@
const rocketUrlParser = async function (message: IMessage): Promise<IMessage> {
log.debug({ msg: 'Parsing message URLs' });

if (!settings.get('API_Embed')) {
return message;
}

if (!Array.isArray(message.urls)) {
return message;
}
Expand All @@ -303,8 +342,6 @@
return message;
}

const attachments: MessageAttachment[] = [];

let changed = false;
for await (const item of message.urls) {
if (item.ignoreParse === true) {
Expand All @@ -318,34 +355,17 @@
changed = changed || foundMeta;
}

if (attachments.length) {
await Messages.setMessageAttachments(message._id, attachments);
}

if (changed === true) {
await Messages.setUrlsById(message._id, message.urls);
}

return message;
};

const OEmbed: {
getUrlMeta: (url: string, withFragment?: boolean) => Promise<OEmbedUrlWithMetadata | undefined | OEmbedUrlContentResult>;
getUrlMetaWithCache: (url: string, withFragment?: boolean) => Promise<OEmbedUrlWithMetadata | OEmbedUrlContentResult | undefined>;
export const OEmbed: {
rocketUrlParser: (message: IMessage) => Promise<IMessage>;
parseUrl: (url: string) => Promise<{ urlPreview: MessageUrl; foundMeta: boolean }>;
} = {
rocketUrlParser,
getUrlMetaWithCache,
getUrlMeta,
parseUrl,
};

settings.watch('API_Embed', (value) => {
if (value) {
return callbacks.add('afterSaveMessage', (message) => OEmbed.rocketUrlParser(message), callbacks.priority.LOW, 'API_Embed');
}
return callbacks.remove('afterSaveMessage', 'API_Embed');
});

export { OEmbed };
Loading
Loading