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

Regression: Error setting user avatars and mentioning rooms on Slack Import #24585

Merged
merged 15 commits into from
Feb 28, 2022
Merged
Show file tree
Hide file tree
Changes from all 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
5 changes: 4 additions & 1 deletion app/importer-slack/server/importer.js
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ export class SlackImporter extends Base {

parseMentions(newMessage) {
const mentionsParser = new MentionsParser({
pattern: () => settings.get('UTF8_User_Names_Validation'),
pattern: () => '[0-9a-zA-Z]+',
useRealName: () => settings.get('UI_Use_Real_Name'),
me: () => 'me',
});
Expand Down Expand Up @@ -534,6 +534,9 @@ export class SlackImporter extends Base {
}

_replaceSlackUserIds(members) {
if (!members?.length) {
return [];
}
return members.map((userId) => this._replaceSlackUserId(userId));
}

Expand Down
36 changes: 31 additions & 5 deletions app/importer/server/classes/ImportDataConverter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ export class ImportDataConverter {
}

if (userData.name || userData.username) {
saveUserIdentity({ _id, name: userData.name, username: userData.username });
Meteor.runAsUser(_id, () => saveUserIdentity({ _id, name: userData.name, username: userData.username }));
}

if (userData.importIds.length) {
Expand Down Expand Up @@ -463,9 +463,12 @@ export class ImportDataConverter {
const data = this.findImportedUser(importId);

if (!data) {
throw new Error('importer-message-mentioned-user-not-found');
this._logger.warn(`Mentioned user not found: ${importId}`);
continue;
}

if (!data.username) {
this._logger.debug(importId);
throw new Error('importer-message-mentioned-username-not-found');
}

Expand All @@ -480,6 +483,31 @@ export class ImportDataConverter {
return result;
}

getMentionedChannelData(importId: string): IMentionedChannel | undefined {
// loading the name will also store the id on the cache if it's missing, so this won't run two queries
const name = this.findImportedRoomName(importId);
const _id = this.findImportedRoomId(importId);

if (name && _id) {
return {
name,
_id,
};
}

// If the importId was not found, check if we have a room with that name
const room = Rooms.findOneByNonValidatedName(importId, { fields: { name: 1 } });
if (room) {
this.addRoomToCache(importId, room._id);
this.addRoomNameToCache(importId, room.name);

return {
name: room.name,
_id: room._id,
};
}
}

convertMessageChannels(message: IImportMessage): Array<IMentionedChannel> | undefined {
const { channels } = message;
if (!channels) {
Expand All @@ -488,9 +516,7 @@ export class ImportDataConverter {

const result: Array<IMentionedChannel> = [];
for (const importId of channels) {
// loading the name will also store the id on the cache if it's missing, so this won't run two queries
const name = this.findImportedRoomName(importId);
const _id = this.findImportedRoomId(importId);
const { name, _id } = this.getMentionedChannelData(importId) || {};

if (!_id || !name) {
this._logger.warn(`Mentioned room not found: ${importId}`);
Expand Down
144 changes: 6 additions & 138 deletions app/lib/server/functions/insertMessage.js
Original file line number Diff line number Diff line change
@@ -1,147 +1,15 @@
import { Match, check } from 'meteor/check';

import { Markdown } from '../../../markdown/server';
import { Messages } from '../../../models';

const objectMaybeIncluding = (types) =>
Match.Where((value) => {
Object.keys(types).forEach((field) => {
if (value[field] != null) {
try {
check(value[field], types[field]);
} catch (error) {
error.path = field;
throw error;
}
}
});

return true;
});

const validateAttachmentsFields = (attachmentField) => {
check(
attachmentField,
objectMaybeIncluding({
short: Boolean,
title: String,
value: Match.OneOf(String, Match.Integer, Boolean),
}),
);

if (typeof attachmentField.value !== 'undefined') {
attachmentField.value = String(attachmentField.value);
}
};

const validateAttachmentsActions = (attachmentActions) => {
check(
attachmentActions,
objectMaybeIncluding({
type: String,
text: String,
url: String,
image_url: String,
is_webview: Boolean,
webview_height_ratio: String,
msg: String,
msg_in_chat_window: Boolean,
}),
);
};

const validateAttachment = (attachment) => {
check(
attachment,
objectMaybeIncluding({
color: String,
text: String,
ts: Match.OneOf(String, Number),
thumb_url: String,
button_alignment: String,
actions: [Match.Any],
message_link: String,
collapsed: Boolean,
author_name: String,
author_link: String,
author_icon: String,
title: String,
title_link: String,
title_link_download: Boolean,
image_url: String,
audio_url: String,
video_url: String,
fields: [Match.Any],
}),
);

if (attachment.fields && attachment.fields.length) {
attachment.fields.map(validateAttachmentsFields);
}

if (attachment.actions && attachment.actions.length) {
attachment.actions.map(validateAttachmentsActions);
}
};

const validateBodyAttachments = (attachments) => attachments.map(validateAttachment);
import { Messages } from '../../../models/server';
import { validateMessage, prepareMessageObject } from './sendMessage';
import { parseUrlsInMessage } from './parseUrlsInMessage';

export const insertMessage = function (user, message, rid, upsert = false) {
if (!user || !message || !rid) {
return false;
}

check(
message,
objectMaybeIncluding({
_id: String,
msg: String,
text: String,
alias: String,
emoji: String,
avatar: String,
attachments: [Match.Any],
}),
);

if (Array.isArray(message.attachments) && message.attachments.length) {
validateBodyAttachments(message.attachments);
}

if (!message.ts) {
message.ts = new Date();
}
const { _id, username } = user;
message.u = {
_id,
username,
};
message.rid = rid;

if (!Match.test(message.msg, String)) {
message.msg = '';
}

if (message.ts == null) {
message.ts = new Date();
}

if (message.parseUrls !== false) {
message.html = message.msg;
message = Markdown.code(message);

const urls = message.html.match(
/([A-Za-z]{3,9}):\/\/([-;:&=\+\$,\w]+@{1})?([-A-Za-z0-9\.]+)+:?(\d+)?((\/[-\+=!:~%\/\.@\,\(\)\w]*)?\??([-\+=&!:;%@\/\.\,\w]+)?(?:#([^\s\)]+))?)?/g,
);
if (urls) {
message.urls = [...new Set(urls)].map((url) => ({ url }));
}

message = Markdown.mountTokensBack(message, false);
message.msg = message.html;
delete message.html;
delete message.tokens;
}
validateMessage(message, { _id: rid }, user);
prepareMessageObject(message, rid, user);
parseUrlsInMessage(message);

if (message._id && upsert) {
const { _id } = message;
Expand Down
21 changes: 12 additions & 9 deletions app/lib/server/functions/sendMessage.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ const validateAttachment = (attachment) => {

const validateBodyAttachments = (attachments) => attachments.map(validateAttachment);

const validateMessage = (message, room, user) => {
export const validateMessage = (message, room, user) => {
check(
message,
objectMaybeIncluding({
Expand Down Expand Up @@ -171,13 +171,7 @@ const validateMessage = (message, room, user) => {
}
};

export const sendMessage = function (user, message, room, upsert = false) {
if (!user || !message || !room._id) {
return false;
}

validateMessage(message, room, user);

export const prepareMessageObject = function (message, rid, user) {
if (!message.ts) {
message.ts = new Date();
}
Expand All @@ -192,7 +186,7 @@ export const sendMessage = function (user, message, room, upsert = false) {
username,
name,
};
message.rid = room._id;
message.rid = rid;

if (!Match.test(message.msg, String)) {
message.msg = '';
Expand All @@ -201,6 +195,15 @@ export const sendMessage = function (user, message, room, upsert = false) {
if (message.ts == null) {
message.ts = new Date();
}
};

export const sendMessage = function (user, message, room, upsert = false) {
if (!user || !message || !room._id) {
return false;
}

validateMessage(message, room, user);
prepareMessageObject(message, room._id, user);

if (settings.get('Message_Read_Receipt_Enabled')) {
message.unread = true;
Expand Down