Fix empty link bug and proper error logging in highlightMentions.js (#12228)

* fix(chat) - Don't insert user link in url-less link and properly log source-mapping failures

* fix(chat) - Add link to markdown spec and list of known issues

* fix(chat) - Log regular expression as String
This commit is contained in:
Bart Enkelaar
2020-05-31 18:09:43 +02:00
committed by GitHub
parent 61d970db86
commit 1c00d7de5b
2 changed files with 34 additions and 12 deletions

View File

@@ -1,4 +1,5 @@
import mongoose from 'mongoose'; import mongoose from 'mongoose';
import highlightMentions from '../../../../website/server/libs/highlightMentions'; import highlightMentions from '../../../../website/server/libs/highlightMentions';
describe('highlightMentions', () => { describe('highlightMentions', () => {
@@ -101,12 +102,28 @@ describe('highlightMentions', () => {
expect(result[0]).to.equal('http://www.medium.com/@user/blog [@user](/profile/111)'); expect(result[0]).to.equal('http://www.medium.com/@user/blog [@user](/profile/111)');
}); });
// https://spec.commonmark.org/0.29/#example-483
it('doesn\'t highlight user in a link without url', async () => {
const text = '[@user2]()';
const result = await highlightMentions(text);
expect(result[0]).to.equal(text);
});
// https://github.com/HabitRPG/habitica/issues/12217 // https://github.com/HabitRPG/habitica/issues/12217
it('doesn\'t highlight user in link with url-escapable characters', async () => { it('doesn\'t highlight user in link with url-escapable characters', async () => {
const text = '[test](https://habitica.fandom.com/ru/@wiki/Снаряжение)'; const text = '[test](https://habitica.fandom.com/ru/@wiki/Снаряжение)';
const result = await highlightMentions(text); const result = await highlightMentions(text);
expect(result[0]).to.equal(text); expect(result[0]).to.equal(text);
}); });
// https://github.com/HabitRPG/habitica/issues/12223
it('matches a link in between two the same links', async () => {
const text = '[here](http://habitica.wikia.com/wiki/The_Keep:Pirate_Cove/FAQ)\n@user\n[hier](http://habitica.wikia.com/wiki/The_Keep:Pirate_Cove/FAQ)';
const result = await highlightMentions(text);
expect(result[0]).to.equal('[here](http://habitica.wikia.com/wiki/The_Keep:Pirate_Cove/FAQ)\n[@user](/profile/111)\n[hier](http://habitica.wikia.com/wiki/The_Keep:Pirate_Cove/FAQ)');
});
}); });
describe('exceptions in code blocks', () => { describe('exceptions in code blocks', () => {
@@ -149,14 +166,6 @@ describe('highlightMentions', () => {
expect(result[0]).to.equal('[@user](/profile/111) `@user`'); expect(result[0]).to.equal('[@user](/profile/111) `@user`');
}); });
it('matches a link in between two the same links', async () => {
const text = '[here](http://habitica.wikia.com/wiki/The_Keep:Pirate_Cove/FAQ)\n@user\n[hier](http://habitica.wikia.com/wiki/The_Keep:Pirate_Cove/FAQ)';
const result = await highlightMentions(text);
expect(result[0]).to.equal('[here](http://habitica.wikia.com/wiki/The_Keep:Pirate_Cove/FAQ)\n[@user](/profile/111)\n[hier](http://habitica.wikia.com/wiki/The_Keep:Pirate_Cove/FAQ)');
});
}); });
it('github issue 12118, method crashes when square brackets are used', async () => { it('github issue 12118, method crashes when square brackets are used', async () => {

View File

@@ -2,6 +2,7 @@ import escapeRegExp from 'lodash/escapeRegExp';
import habiticaMarkdown from 'habitica-markdown'; import habiticaMarkdown from 'habitica-markdown';
import { model as User } from '../models/user'; import { model as User } from '../models/user';
import logger from './logger';
const mentionRegex = /\B@[-\w]+/g; const mentionRegex = /\B@[-\w]+/g;
const ignoreTokenTypes = ['code_block', 'code_inline', 'fence', 'link_open']; const ignoreTokenTypes = ['code_block', 'code_inline', 'fence', 'link_open'];
@@ -70,8 +71,17 @@ function withOptionalIndentation (content) {
return content.split('\n').map(line => `\\s*${line}`).join('\n'); return content.split('\n').map(line => `\\s*${line}`).join('\n');
} }
// This is essentially a workaround around the fact that markdown-it doesn't /* This is essentially a workaround around the fact that markdown-it doesn't
// provide sourcemap functionality and is the most brittle part of this code. * provide sourcemap functionality and is the most brittle part of this code.
*
* Known errors (Not supported markdown link variants):
* - [a](<b)c>) https://spec.commonmark.org/0.29/#example-489
* - [link](\(foo\)) https://spec.commonmark.org/0.29/#example-492
* - [link](foo(and(bar))) https://spec.commonmark.org/0.29/#example-493
* - [link](foo\(and\(bar\)) https://spec.commonmark.org/0.29/#example-494
* - [link](<foo(and(bar)>) https://spec.commonmark.org/0.29/#example-495
* - [link](foo\)\:) https://spec.commonmark.org/0.29/#example-496
*/
function toSourceMapRegex (token) { function toSourceMapRegex (token) {
const { type, content, markup } = token; const { type, content, markup } = token;
const contentRegex = escapeRegExp(content); const contentRegex = escapeRegExp(content);
@@ -86,7 +96,7 @@ function toSourceMapRegex (token) {
} else if (type === 'link_open') { } else if (type === 'link_open') {
const texts = token.textContents.map(escapeRegExp); const texts = token.textContents.map(escapeRegExp);
regexStr = markup === 'linkify' || markup === 'autolink' ? texts[0] regexStr = markup === 'linkify' || markup === 'autolink' ? texts[0]
: `\\[[^\\]]*${texts.join('[^\\]]*')}[^\\]]*\\]\\([^)]+\\)`; : `\\[[^\\]]*${texts.join('[^\\]]*')}[^\\]]*\\]\\([^)]*\\)`;
} else { } else {
throw new Error(`No source mapping regex defined for ignore blocks of type ${type}`); throw new Error(`No source mapping regex defined for ignore blocks of type ${type}`);
} }
@@ -112,7 +122,10 @@ function findTextBlocks (text) {
const match = targetText.match(regex); const match = targetText.match(regex);
if (!match) { if (!match) {
// Should not happen, but insert to handle bugs gracefully logger.error(
new Error('Failed to match source-mapping regex to find ignore block'),
{ text, targetText, regex: String(regex) },
);
return; return;
} }