Skip to content

Commit 7b51ba8

Browse files
authored
feat: OEmbed async processing (#38151)
1 parent ccd7694 commit 7b51ba8

File tree

15 files changed

+179
-128
lines changed

15 files changed

+179
-128
lines changed

.changeset/gold-kids-move.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
"@rocket.chat/meteor": patch
3+
"@rocket.chat/core-services": patch
4+
"@rocket.chat/i18n": patch
5+
---
6+
7+
Changes OEmbed URL processing. Now, the processing is done asynchronously and has a configurable timeout for each request. Additionally, the `API_EmbedIgnoredHosts` setting now accepts wildcard domains.

apps/meteor/app/api/server/v1/chat.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { Message } from '@rocket.chat/core-services';
12
import type { IMessage, IThreadMainMessage } from '@rocket.chat/core-typings';
23
import { MessageTypes } from '@rocket.chat/message-types';
34
import { Messages, Users, Rooms, Subscriptions } from '@rocket.chat/models';
@@ -48,7 +49,6 @@ import { executeUpdateMessage } from '../../../lib/server/methods/updateMessage'
4849
import { applyAirGappedRestrictionsValidation } from '../../../license/server/airGappedRestrictionsWrapper';
4950
import { pinMessage, unpinMessage } from '../../../message-pin/server/pinMessage';
5051
import { starMessage } from '../../../message-star/server/starMessage';
51-
import { OEmbed } from '../../../oembed/server/server';
5252
import { executeSetReaction } from '../../../reactions/server/setReaction';
5353
import { settings } from '../../../settings/server';
5454
import { followMessage } from '../../../threads/server/methods/followMessage';
@@ -914,7 +914,7 @@ API.v1.addRoute(
914914
throw new Meteor.Error('error-not-allowed', 'Not allowed');
915915
}
916916

917-
const { urlPreview } = await OEmbed.parseUrl(url);
917+
const { urlPreview } = await Message.parseOEmbedUrl(url);
918918
urlPreview.ignoreParse = true;
919919

920920
return API.v1.success({ urlPreview });

apps/meteor/app/lib/server/functions/sendMessage.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { hasPermissionAsync } from '../../../authorization/server/functions/hasP
1111
import { FileUpload } from '../../../file-upload/server';
1212
import { settings } from '../../../settings/server';
1313
import { afterSaveMessage } from '../lib/afterSaveMessage';
14-
import { notifyOnRoomChangedById, notifyOnMessageChange } from '../lib/notifyListener';
14+
import { notifyOnRoomChangedById } from '../lib/notifyListener';
1515
import { validateCustomMessageFields } from '../lib/validateCustomMessageFields';
1616

1717
// TODO: most of the types here are wrong, but I don't want to change them now
@@ -285,11 +285,8 @@ export const sendMessage = async function (user: any, message: any, room: any, u
285285
void Apps.self?.triggerEvent(messageEvent, message);
286286
}
287287

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

291-
void notifyOnMessageChange({ id: message._id });
292-
293290
void notifyOnRoomChangedById(message.rid);
294291

295292
return message;

apps/meteor/app/lib/server/functions/updateMessage.ts

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { Meteor } from 'meteor/meteor';
77
import { parseUrlsInMessage } from './parseUrlsInMessage';
88
import { settings } from '../../../settings/server';
99
import { afterSaveMessage } from '../lib/afterSaveMessage';
10-
import { notifyOnRoomChangedById, notifyOnMessageChange } from '../lib/notifyListener';
10+
import { notifyOnRoomChangedById } from '../lib/notifyListener';
1111
import { validateCustomMessageFields } from '../lib/validateCustomMessageFields';
1212

1313
export const updateMessage = async function (
@@ -97,14 +97,7 @@ export const updateMessage = async function (
9797
return;
9898
}
9999

100-
// although this is an "afterSave" kind callback, we know they can extend message's properties
101-
// so we wait for it to run before broadcasting
102-
const data = await afterSaveMessage(msg, room, user);
103-
104-
void notifyOnMessageChange({
105-
id: msg._id,
106-
data,
107-
});
100+
await afterSaveMessage(msg, room, user);
108101

109102
if (room?.lastMessage?._id === msg._id) {
110103
void notifyOnRoomChangedById(message.rid);
Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { Message } from '@rocket.chat/core-services';
12
import type { IMessage, IUser, IRoom } from '@rocket.chat/core-typings';
23
import type { Updater } from '@rocket.chat/models';
34
import { Rooms } from '@rocket.chat/models';
@@ -6,14 +7,16 @@ import { callbacks } from '../../../../server/lib/callbacks';
67

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

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

16+
void Message.afterSave({ message: data });
17+
1518
// TODO: Fix type - callback configuration needs to be updated
16-
return data as unknown as IMessage;
19+
return data;
1720
}
1821

1922
export function afterSaveMessageAsync(message: IMessage, room: IRoom, user: IUser, roomUpdater: Updater<IRoom> = Rooms.getUpdater()): void {
@@ -22,4 +25,6 @@ export function afterSaveMessageAsync(message: IMessage, room: IRoom, user: IUse
2225
if (roomUpdater.hasChanges()) {
2326
void Rooms.updateFromUpdater({ _id: room._id }, roomUpdater);
2427
}
28+
29+
void Message.afterSave({ message });
2530
}

apps/meteor/app/oembed/server/index.ts

Lines changed: 0 additions & 2 deletions
This file was deleted.

apps/meteor/server/importPackages.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,6 @@ import '../app/message-pin/server';
4242
import '../app/message-star/server';
4343
import '../app/nextcloud/server';
4444
import '../app/oauth2-server-config/server';
45-
import '../app/oembed/server';
4645
import '../app/push-notifications/server';
4746
import '../app/retention-policy/server';
4847
import '../app/slackbridge/server';

apps/meteor/server/lib/callbacks.ts

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@ import type {
99
ILivechatInquiryRecord,
1010
ILivechatVisitor,
1111
VideoConference,
12-
OEmbedMeta,
13-
OEmbedUrlContent,
1412
IOmnichannelRoom,
1513
ILivechatTag,
1614
ILivechatTagRecord,
@@ -164,16 +162,6 @@ type ChainedCallbackSignatures = {
164162
BusinessHourBehaviorClass: { new (): IBusinessHourBehavior };
165163
};
166164
'renderMessage': <T extends IMessage & { html: string }>(message: T) => T;
167-
'oembed:beforeGetUrlContent': (data: { urlObj: URL }) => {
168-
urlObj: URL;
169-
headerOverrides?: { [k: string]: string };
170-
};
171-
'oembed:afterParseContent': (data: { url: string; meta: OEmbedMeta; headers: { [k: string]: string }; content: OEmbedUrlContent }) => {
172-
url: string;
173-
meta: OEmbedMeta;
174-
headers: { [k: string]: string };
175-
content: OEmbedUrlContent;
176-
};
177165
'livechat.beforeListTags': () => ILivechatTag[];
178166
'livechat.offlineMessage': (data: { name: string; email: string; message: string; department?: string; host?: string }) => void;
179167
'livechat.leadCapture': (room: IOmnichannelRoom) => IOmnichannelRoom;

apps/meteor/app/oembed/server/server.ts renamed to apps/meteor/server/services/messages/hooks/AfterSaveOEmbed.ts

Lines changed: 66 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,25 +1,25 @@
11
import type {
22
OEmbedUrlContentResult,
3+
MessageUrl,
34
OEmbedUrlWithMetadata,
4-
IMessage,
5-
MessageAttachment,
65
OEmbedMeta,
7-
MessageUrl,
6+
IMessage,
7+
OEmbedUrlContent,
88
} from '@rocket.chat/core-typings';
99
import { isOEmbedUrlWithMetadata } from '@rocket.chat/core-typings';
1010
import { Logger } from '@rocket.chat/logger';
11-
import { Messages, OEmbedCache } from '@rocket.chat/models';
11+
import { OEmbedCache, Messages } from '@rocket.chat/models';
1212
import { serverFetch as fetch } from '@rocket.chat/server-fetch';
13-
import { camelCase } from 'change-case';
1413
import he from 'he';
1514
import iconv from 'iconv-lite';
1615
import ipRangeCheck from 'ip-range-check';
1716
import jschardet from 'jschardet';
17+
import { camelCase } from 'lodash';
1818

19-
import { isURL } from '../../../lib/utils/isURL';
20-
import { callbacks } from '../../../server/lib/callbacks';
21-
import { settings } from '../../settings/server';
22-
import { Info } from '../../utils/rocketchat.info';
19+
import { settings } from '../../../../app/settings/server';
20+
import { Info } from '../../../../app/utils/rocketchat.info';
21+
import { isURL } from '../../../../lib/utils/isURL';
22+
import { afterParseUrlContent, beforeGetUrlContent } from '../lib/oembed/providers';
2323

2424
const MAX_EXTERNAL_URL_PREVIEWS = 5;
2525
const log = new Logger('OEmbed');
@@ -65,7 +65,7 @@ const toUtf8 = function (contentType: string, body: Buffer): string {
6565
return iconv.decode(body, getCharset(contentType, body));
6666
};
6767

68-
const getUrlContent = async (urlObj: URL, redirectCount = 5): Promise<OEmbedUrlContentResult> => {
68+
const getUrlContent = async (urlObj: URL, redirectCount = 5): Promise<OEmbedUrlContent> => {
6969
const portsProtocol = new Map<string, string>(
7070
Object.entries({
7171
80: 'http:',
@@ -74,9 +74,48 @@ const getUrlContent = async (urlObj: URL, redirectCount = 5): Promise<OEmbedUrlC
7474
}),
7575
);
7676

77-
const ignoredHosts = settings.get<string>('API_EmbedIgnoredHosts').replace(/\s/g, '').split(',') || [];
78-
if (urlObj.hostname && (ignoredHosts.includes(urlObj.hostname) || ipRangeCheck(urlObj.hostname, ignoredHosts))) {
79-
throw new Error('invalid host');
77+
const ignoredHosts =
78+
settings
79+
.get<string>('API_EmbedIgnoredHosts')
80+
.replace(/\s/g, '')
81+
.split(',')
82+
.filter(Boolean)
83+
.map((host) => host.toLowerCase()) || [];
84+
85+
const isIgnoredHost = (hostname: string | undefined): boolean => {
86+
hostname = hostname?.toLowerCase();
87+
if (!hostname || !ignoredHosts.length) {
88+
return false;
89+
}
90+
91+
const exactHosts = ignoredHosts.filter((h) => !h.includes('*'));
92+
if (exactHosts.includes(hostname) || ipRangeCheck(hostname, exactHosts)) {
93+
return true;
94+
}
95+
96+
return ignoredHosts
97+
.filter((h) => h.includes('*'))
98+
.some((pattern) => {
99+
const validationRegex = /^(?:\*\.)?(?:\*|[a-z0-9-]+)(?:\.(?:\*|[a-z0-9-]+))*$/i;
100+
if (!validationRegex.test(pattern) || pattern === '*') {
101+
return false;
102+
}
103+
104+
const escaped = pattern.replace(/[-/\\^$+?.()|[\]{}]/g, '\\$&');
105+
const source = `^${escaped.replace(/\*/g, '[^.]*')}$`;
106+
107+
try {
108+
const regex = new RegExp(source, 'i');
109+
return regex.test(hostname);
110+
} catch {
111+
// fail safe on invalid patterns
112+
return false;
113+
}
114+
});
115+
};
116+
117+
if (isIgnoredHost(urlObj.hostname)) {
118+
throw new Error('host is ignored');
80119
}
81120

82121
const safePorts = settings.get<string>('API_EmbedSafePorts').replace(/\s/g, '').split(',') || [];
@@ -91,14 +130,13 @@ const getUrlContent = async (urlObj: URL, redirectCount = 5): Promise<OEmbedUrlC
91130
throw new Error('invalid/unsafe port');
92131
}
93132

94-
const data = await callbacks.run('oembed:beforeGetUrlContent', {
95-
urlObj,
96-
});
133+
const data = beforeGetUrlContent({ urlObj });
97134

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

101138
log.debug({ msg: 'Fetching URL for OEmbed', url, redirectCount });
139+
const start = Date.now();
102140
const response = await fetch(
103141
url,
104142
{
@@ -109,10 +147,12 @@ const getUrlContent = async (urlObj: URL, redirectCount = 5): Promise<OEmbedUrlC
109147
'Accept-Language': settings.get('Language') || 'en',
110148
...data.headerOverrides,
111149
},
150+
timeout: settings.get<number>('API_EmbedTimeout') * 1000,
112151
size: sizeLimit, // max size of the response body, this was not working as expected so I'm also manually verifying that on the iterator
113152
},
114153
settings.get('Allow_Invalid_SelfSigned_Certs'),
115154
);
155+
const end = Date.now();
116156

117157
let totalSize = 0;
118158
const chunks = [];
@@ -126,13 +166,14 @@ const getUrlContent = async (urlObj: URL, redirectCount = 5): Promise<OEmbedUrlC
126166
}
127167
}
128168

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

132172
return {
133173
headers: Object.fromEntries(response.headers),
134174
body: toUtf8(response.headers.get('content-type') || 'text/plain', buffer),
135175
statusCode: response.status,
176+
urlObj,
136177
};
137178
};
138179

@@ -183,7 +224,7 @@ const getUrlMeta = async function (
183224
}
184225

185226
log.debug({ msg: 'Fetching URL content', url: urlObj.toString() });
186-
let content: OEmbedUrlContentResult | undefined;
227+
let content: OEmbedUrlContent | undefined;
187228
try {
188229
content = await getUrlContent(urlObj, 5);
189230
} catch (err) {
@@ -195,7 +236,7 @@ const getUrlMeta = async function (
195236
}
196237

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

200241
if (content?.body) {
201242
const escapeMeta = (name: string, value: string): string => {
@@ -233,7 +274,7 @@ const getUrlMeta = async function (
233274
if (content && content.statusCode !== 200) {
234275
return;
235276
}
236-
return callbacks.run('oembed:afterParseContent', {
277+
return afterParseUrlContent({
237278
url,
238279
meta: metas,
239280
headers,
@@ -289,6 +330,10 @@ const insertMaxWidthInOembedHtml = (oembedHtml?: string): string | undefined =>
289330
const rocketUrlParser = async function (message: IMessage): Promise<IMessage> {
290331
log.debug({ msg: 'Parsing message URLs' });
291332

333+
if (!settings.get('API_Embed')) {
334+
return message;
335+
}
336+
292337
if (!Array.isArray(message.urls)) {
293338
return message;
294339
}
@@ -303,8 +348,6 @@ const rocketUrlParser = async function (message: IMessage): Promise<IMessage> {
303348
return message;
304349
}
305350

306-
const attachments: MessageAttachment[] = [];
307-
308351
let changed = false;
309352
for await (const item of message.urls) {
310353
if (item.ignoreParse === true) {
@@ -318,34 +361,17 @@ const rocketUrlParser = async function (message: IMessage): Promise<IMessage> {
318361
changed = changed || foundMeta;
319362
}
320363

321-
if (attachments.length) {
322-
await Messages.setMessageAttachments(message._id, attachments);
323-
}
324-
325364
if (changed === true) {
326365
await Messages.setUrlsById(message._id, message.urls);
327366
}
328367

329368
return message;
330369
};
331370

332-
const OEmbed: {
333-
getUrlMeta: (url: string, withFragment?: boolean) => Promise<OEmbedUrlWithMetadata | undefined | OEmbedUrlContentResult>;
334-
getUrlMetaWithCache: (url: string, withFragment?: boolean) => Promise<OEmbedUrlWithMetadata | OEmbedUrlContentResult | undefined>;
371+
export const OEmbed: {
335372
rocketUrlParser: (message: IMessage) => Promise<IMessage>;
336373
parseUrl: (url: string) => Promise<{ urlPreview: MessageUrl; foundMeta: boolean }>;
337374
} = {
338375
rocketUrlParser,
339-
getUrlMetaWithCache,
340-
getUrlMeta,
341376
parseUrl,
342377
};
343-
344-
settings.watch('API_Embed', (value) => {
345-
if (value) {
346-
return callbacks.add('afterSaveMessage', (message) => OEmbed.rocketUrlParser(message), callbacks.priority.LOW, 'API_Embed');
347-
}
348-
return callbacks.remove('afterSaveMessage', 'API_Embed');
349-
});
350-
351-
export { OEmbed };

0 commit comments

Comments
 (0)