From 9608b9fa9fb174d91e2409e6e60c585c896ab0fa Mon Sep 17 00:00:00 2001 From: Bart Enkelaar Date: Fri, 17 Apr 2020 22:19:11 +0200 Subject: [PATCH] Don't link user in markdown code blocks - Fixes #[11504] (#12069) * Improve whitespacing in highlightMentions.js unit test * Issue 11504 - Don't link users in markdown code blocks Use the markdown-it parser to determine what parts of the message are code block first. Then work from those parser tokens back to content parts that should not be handled. Still convoluted, but can be improved once a "user mention plugin" is added to habitica-markdown. * Issue 11504 - Put functions in JavaScript order and fix linting issues * Issue 11504 - Use includes i.o. multiple or checks and added some context. * Issue 11504 - Add docstring for highlightMentions and simplify fence regex * Issue 11504 - Replace inline recursor with default parameter value --- ...tMentions.js => highlightMentions.test.js} | 60 ++++++++- website/server/controllers/api-v3/chat.js | 2 +- website/server/controllers/api-v3/members.js | 2 +- website/server/libs/highlightMentions.js | 122 +++++++++++++++++- 4 files changed, 171 insertions(+), 15 deletions(-) rename test/api/unit/libs/{highlightMentions.js => highlightMentions.test.js} (51%) diff --git a/test/api/unit/libs/highlightMentions.js b/test/api/unit/libs/highlightMentions.test.js similarity index 51% rename from test/api/unit/libs/highlightMentions.js rename to test/api/unit/libs/highlightMentions.test.js index 6d46e14aee..a68cd9d6d7 100644 --- a/test/api/unit/libs/highlightMentions.js +++ b/test/api/unit/libs/highlightMentions.test.js @@ -1,7 +1,5 @@ import mongoose from 'mongoose'; -import { - highlightMentions, -} from '../../../../website/server/libs/highlightMentions'; +import highlightMentions from '../../../../website/server/libs/highlightMentions'; describe('highlightMentions', () => { beforeEach(() => { @@ -13,9 +11,12 @@ describe('highlightMentions', () => { return this; }, exec () { - return Promise.resolve([{ - auth: { local: { username: 'user' } }, _id: '111', - }, { auth: { local: { username: 'user2' } }, _id: '222' }, { auth: { local: { username: 'user3' } }, _id: '333' }, { auth: { local: { username: 'user-dash' } }, _id: '444' }, { auth: { local: { username: 'user_underscore' } }, _id: '555' }, + return Promise.resolve([ + { auth: { local: { username: 'user' } }, _id: '111' }, + { auth: { local: { username: 'user2' } }, _id: '222' }, + { auth: { local: { username: 'user3' } }, _id: '333' }, + { auth: { local: { username: 'user-dash' } }, _id: '444' }, + { auth: { local: { username: 'user_underscore' } }, _id: '555' }, ]); }, }; @@ -32,29 +33,76 @@ describe('highlightMentions', () => { const result = await highlightMentions(text); expect(result[0]).to.equal(text); }); + it('highlights existing users', async () => { const text = '@user: message'; const result = await highlightMentions(text); expect(result[0]).to.equal('[@user](/profile/111): message'); }); + it('highlights special characters', async () => { const text = '@user-dash: message @user_underscore'; const result = await highlightMentions(text); expect(result[0]).to.equal('[@user-dash](/profile/444): message [@user_underscore](/profile/555)'); }); + it('doesn\'t highlight nonexisting users', async () => { const text = '@nouser message'; const result = await highlightMentions(text); expect(result[0]).to.equal('@nouser message'); }); + it('highlights multiple existing users', async () => { const text = '@user message (@user2) @user3 @user'; const result = await highlightMentions(text); expect(result[0]).to.equal('[@user](/profile/111) message ([@user2](/profile/222)) [@user3](/profile/333) [@user](/profile/111)'); }); + it('doesn\'t highlight more than 5 users', async () => { const text = '@user @user2 @user3 @user4 @user5 @user6'; const result = await highlightMentions(text); expect(result[0]).to.equal(text); }); + + describe('exceptions in code blocks', () => { + it('doesn\'t highlight user in inline code block', async () => { + const text = '`@user`'; + + const result = await highlightMentions(text); + + expect(result[0]).to.equal(text); + }); + + it('doesn\'t highlight user in fenced code block', async () => { + const text = 'Text\n\n```\n// code referencing @user\n```\n\nText'; + + const result = await highlightMentions(text); + + expect(result[0]).to.equal(text); + }); + + it('doesn\'t highlight user in indented code block', async () => { + const text = ' @user'; + + const result = await highlightMentions(text); + + expect(result[0]).to.equal(text); + }); + + it('does highlight user that\'s after in-line code block', async () => { + const text = '`` for @user'; + + const result = await highlightMentions(text); + + expect(result[0]).to.equal('`` for [@user](/profile/111)'); + }); + + it('does highlight same content properly', async () => { + const text = '@user `@user`'; + + const result = await highlightMentions(text); + + expect(result[0]).to.equal('[@user](/profile/111) `@user`'); + }); + }); }); diff --git a/website/server/controllers/api-v3/chat.js b/website/server/controllers/api-v3/chat.js index 60e5284594..5ade8f037a 100644 --- a/website/server/controllers/api-v3/chat.js +++ b/website/server/controllers/api-v3/chat.js @@ -22,7 +22,7 @@ import guildsAllowingBannedWords from '../../libs/guildsAllowingBannedWords'; import { getMatchesByWordArray } from '../../libs/stringUtils'; import bannedSlurs from '../../libs/bannedSlurs'; import apiError from '../../libs/apiError'; -import { highlightMentions } from '../../libs/highlightMentions'; +import highlightMentions from '../../libs/highlightMentions'; const FLAG_REPORT_EMAILS = nconf.get('FLAG_REPORT_EMAIL').split(',').map(email => ({ email, canSend: true })); diff --git a/website/server/controllers/api-v3/members.js b/website/server/controllers/api-v3/members.js index bc2e5b3065..01334d26b4 100644 --- a/website/server/controllers/api-v3/members.js +++ b/website/server/controllers/api-v3/members.js @@ -24,7 +24,7 @@ import { sentMessage } from '../../libs/inbox'; import { sanitizeText as sanitizeMessageText, } from '../../models/message'; -import { highlightMentions } from '../../libs/highlightMentions'; +import highlightMentions from '../../libs/highlightMentions'; const { achievements } = common; diff --git a/website/server/libs/highlightMentions.js b/website/server/libs/highlightMentions.js index b04777ca2d..a50c490593 100644 --- a/website/server/libs/highlightMentions.js +++ b/website/server/libs/highlightMentions.js @@ -1,12 +1,117 @@ +import habiticaMarkdown from 'habitica-markdown'; + import { model as User } from '../models/user'; -const mentionRegex = new RegExp('\\B@[-\\w]+', 'g'); +const mentionRegex = /\B@[-\w]+/g; +const codeTokenTypes = ['code_block', 'code_inline', 'fence']; -export async function highlightMentions (text) { // eslint-disable-line import/prefer-default-export - const mentions = text.match(mentionRegex); +/** + * Container class for text blocks and code blocks combined + * Blocks have the properties `text` and `isCodeBlock` + */ +class TextWithCodeBlocks { + constructor (blocks) { + this.blocks = blocks; + this.textBlocks = blocks.filter(block => !block.isCodeBlock); + this.allText = this.textBlocks.map(block => block.text).join('\n'); + } + + transformTextBlocks (transform) { + this.textBlocks.forEach(block => { + block.text = transform(block.text); + }); + } + + rebuild () { + return this.blocks.map(block => block.text).join(''); + } +} + +/** + * Since tokens have both order and can be nested until infinite depth, + * use a branching recursive algorithm to maintain order and check all tokens. + */ +function findCodeBlocks (tokens, aggregator) { + const result = aggregator || []; + const [head, ...tail] = tokens; + if (!head) { + return result; + } + + if (codeTokenTypes.includes(head.type)) { + result.push(head); + } + + return findCodeBlocks(tail, head.children ? findCodeBlocks(head.children, result) : result); +} + +/** + * Since there are many factors that can prefix lines with indentation in + * markdown, each line from a token's content needs to be prefixed with a + * variable whitespace matcher. + * + * See for example: https://spec.commonmark.org/0.29/#example-224 + */ +function withOptionalIndentation (content) { + return content.split('\n').map(line => `\\s*${line}`).join('\n'); +} + +function createCodeBlockRegex ({ content, type, markup }) { + let regexStr = ''; + + if (type === 'code_block') { + regexStr = withOptionalIndentation(content); + } else if (type === 'fence') { + regexStr = `\\s*${markup}.*\n${withOptionalIndentation(content)}\\s*${markup}`; + } else { // type === code_inline + regexStr = `${markup} ?${content} ?${markup}`; + } + + return new RegExp(regexStr); +} + +/** + * Uses habiticaMarkdown to determine what part of the text are code blocks + * according to the specification here: https://spec.commonmark.org/0.29/ + */ +function findTextAndCodeBlocks (text) { + // For token description see https://markdown-it.github.io/markdown-it/#Token + const tokens = habiticaMarkdown.parse(text); + const codeBlocks = findCodeBlocks(tokens); + + const blocks = []; + let remainingText = text; + codeBlocks.forEach(codeBlock => { + const codeBlockRegex = createCodeBlockRegex(codeBlock); + const match = remainingText.match(codeBlockRegex); + + if (match.index) { + blocks.push({ text: remainingText.substr(0, match.index), isCodeBlock: false }); + } + blocks.push({ text: match[0], isCodeBlock: true }); + + remainingText = remainingText.substr(match.index + match[0].length); + }); + + if (remainingText) { + blocks.push({ text: remainingText, isCodeBlock: false }); + } + return new TextWithCodeBlocks(blocks); +} + +/** + * Replaces `@user` mentions by `[@user](/profile/{user-id})` markup to inject + * a link towards the user's profile page. + * - Only works if there are no more that 5 user mentions + * - Skips mentions in code blocks as defined by https://spec.commonmark.org/0.29/ + */ +export default async function highlightMentions (text) { + const textAndCodeBlocks = findTextAndCodeBlocks(text); + + const mentions = textAndCodeBlocks.allText.match(mentionRegex); let members = []; - if (mentions !== null && mentions.length <= 5) { + if (mentions && mentions.length <= 5) { const usernames = mentions.map(mention => mention.substr(1)); members = await User .find({ 'auth.local.username': { $in: usernames }, 'flags.verifiedUsername': true }) @@ -15,9 +120,12 @@ export async function highlightMentions (text) { // eslint-disable-line import/p .exec(); members.forEach(member => { const { username } = member.auth.local; - // eslint-disable-next-line no-param-reassign - text = text.replace(new RegExp(`@${username}(?![\\-\\w])`, 'g'), `[@${username}](/profile/${member._id})`); + const regex = new RegExp(`@${username}(?![\\-\\w])`, 'g'); + const replacement = `[@${username}](/profile/${member._id})`; + + textAndCodeBlocks.transformTextBlocks(blockText => blockText.replace(regex, replacement)); }); } - return [text, mentions, members]; + + return [textAndCodeBlocks.rebuild(), mentions, members]; }