From e5c9be67c08e7968735735898fb05a383f445bb9 Mon Sep 17 00:00:00 2001 From: "Pierre H. Lehnen" Date: Thu, 20 Dec 2018 20:38:54 -0200 Subject: [PATCH] [FIX] Nested Markdown blocks not parsed properly (#12998) * Fix markdown tokens not restored properly * Add tests --- packages/rocketchat-markdown/lib/markdown.js | 28 ++++- .../lib/parser/original/code.js | 2 +- .../rocketchat-markdown/tests/client.mocks.js | 14 +++ .../rocketchat-markdown/tests/client.tests.js | 112 ++++++++++-------- 4 files changed, 98 insertions(+), 58 deletions(-) diff --git a/packages/rocketchat-markdown/lib/markdown.js b/packages/rocketchat-markdown/lib/markdown.js index 4d2286473297..a063236c2639 100644 --- a/packages/rocketchat-markdown/lib/markdown.js +++ b/packages/rocketchat-markdown/lib/markdown.js @@ -45,13 +45,31 @@ class MarkdownClass { return parsers.original(message); } - mountTokensBack(message, useHtml = true) { - if (message.tokens && message.tokens.length > 0) { - for (const { token, text, noHtml } of message.tokens) { - message.html = message.html.replace(token, () => (useHtml ? text : noHtml)); // Uses lambda so doesn't need to escape $ + mountTokensBackRecursively(message, tokenList, useHtml = true) { + const missingTokens = []; + + if (tokenList.length > 0) { + for (const { token, text, noHtml } of tokenList) { + if (message.html.indexOf(token) >= 0) { + message.html = message.html.replace(token, () => (useHtml ? text : noHtml)); // Uses lambda so doesn't need to escape $ + } else { + missingTokens.push({ token, text, noHtml }); + } } } + // If there are tokens that were missing from the string, but the last iteration replaced at least one token, then go again + // this is done because one of the tokens may have been hidden by another one + if (missingTokens.length > 0 && missingTokens.length < tokenList.length) { + this.mountTokensBackRecursively(message, missingTokens, useHtml); + } + } + + mountTokensBack(message, useHtml = true) { + if (message.tokens) { + this.mountTokensBackRecursively(message, message.tokens, useHtml); + } + return message; } @@ -60,7 +78,7 @@ class MarkdownClass { } } -const Markdown = new MarkdownClass; +export const Markdown = new MarkdownClass; RocketChat.Markdown = Markdown; // renderMessage already did html escape diff --git a/packages/rocketchat-markdown/lib/parser/original/code.js b/packages/rocketchat-markdown/lib/parser/original/code.js index a208ac561edb..8a7903364767 100644 --- a/packages/rocketchat-markdown/lib/parser/original/code.js +++ b/packages/rocketchat-markdown/lib/parser/original/code.js @@ -9,7 +9,7 @@ import hljs from 'highlight.js'; const inlinecode = (message) => // Support `text` message.html = message.html.replace(/\`([^`\r\n]+)\`([<_*~]|\B|\b|$)/gm, (match, p1, p2) => { - const token = ` =!=${ Random.id() }=!=`; + const token = `=!=${ Random.id() }=!=`; message.tokens.push({ token, diff --git a/packages/rocketchat-markdown/tests/client.mocks.js b/packages/rocketchat-markdown/tests/client.mocks.js index 73a18f2544b0..07eb25de3a19 100644 --- a/packages/rocketchat-markdown/tests/client.mocks.js +++ b/packages/rocketchat-markdown/tests/client.mocks.js @@ -11,6 +11,10 @@ mock('meteor/meteor', { }, }); +mock('meteor/blaze', { + Blaze: {}, +}); + mock('meteor/rocketchat:lib', { RocketChat: { settings: { @@ -18,6 +22,8 @@ mock('meteor/rocketchat:lib', { switch (setting) { case 'Markdown_SupportSchemesForLink': return 'http,https'; + case 'Markdown_Parser': + return 'original'; case 'Markdown_Headers': // case 'Markdown_Marked_GFM': // case 'Markdown_Marked_Tables': @@ -31,6 +37,14 @@ mock('meteor/rocketchat:lib', { } }, }, + callbacks: { + add() { + + }, + priority: { + HIGH: 1, + }, + }, }, }); diff --git a/packages/rocketchat-markdown/tests/client.tests.js b/packages/rocketchat-markdown/tests/client.tests.js index 8f7e5d94c374..c9867c271de6 100644 --- a/packages/rocketchat-markdown/tests/client.tests.js +++ b/packages/rocketchat-markdown/tests/client.tests.js @@ -1,8 +1,10 @@ /* eslint-env mocha */ import 'babel-polyfill'; import assert from 'assert'; +import s from 'underscore.string'; import './client.mocks.js'; import { original } from '../lib/parser/original/original'; +import { Markdown } from '../lib/markdown'; // import {marked} from '../parser/marked/marked'; const wrapper = (text, tag) => `${ tag }${ text }${ tag }`; @@ -11,7 +13,7 @@ const italicWrapper = (text) => wrapper(`${ text }`, '_'); const strikeWrapper = (text) => wrapper(`${ text }`, '~'); const headerWrapper = (text, level) => `${ text }`; const quoteWrapper = (text) => `
>${ text }
`; -const linkWrapped = (link, title) => `${ title }`; +const linkWrapped = (link, title) => `${ s.escapeHTML(title) }`; const inlinecodeWrapper = (text) => wrapper(`${ text }`, '`'); const codeWrapper = (text, lang) => `
\`\`\`
${ text }
\`\`\`
`; @@ -121,62 +123,62 @@ const headersLevel4 = { }; const quote = { - '>Hello': quoteWrapper('Hello'), - '>Rocket.Cat': quoteWrapper('Rocket.Cat'), - '>Hi': quoteWrapper('Hi'), - '> Hello this is dog': quoteWrapper(' Hello this is dog'), - '> Rocket cat says Hello': quoteWrapper(' Rocket cat says Hello'), - '> He said Hello to her': quoteWrapper(' He said Hello to her'), - '> He said Hello to her ': quoteWrapper(' He said Hello to her '), - '<Hello': '<Hello', - '<Rocket.Cat>': '<Rocket.Cat>', - ' >Hi': ' >Hi', - 'Hello > this is dog': 'Hello > this is dog', - 'Roc>ket cat says Hello': 'Roc>ket cat says Hello', - 'He said Hello to her>': 'He said Hello to her>', - '>Hello': '>Hello', - '>Rocket.Cat': '>Rocket.Cat', - '>Hi': '>Hi', - '> Hello this is dog': '> Hello this is dog', - '> Rocket cat says Hello': '> Rocket cat says Hello', - '> He said Hello to her': '> He said Hello to her', - '': '', - ' >Hi': ' >Hi', - 'Hello > this is dog': 'Hello > this is dog', - 'Roc>ket cat says Hello': 'Roc>ket cat says Hello', - 'He said Hello to her>': 'He said Hello to her>', + '>Hello': s.escapeHTML('>Hello'), + '>Rocket.Cat': s.escapeHTML('>Rocket.Cat'), + '>Hi': s.escapeHTML('>Hi'), + '> Hello this is dog': s.escapeHTML('> Hello this is dog'), + '> Rocket cat says Hello': s.escapeHTML('> Rocket cat says Hello'), + '> He said Hello to her': s.escapeHTML('> He said Hello to her'), + '> He said Hello to her ': s.escapeHTML('> He said Hello to her '), + '<Hello': s.escapeHTML('<Hello'), + '<Rocket.Cat>': s.escapeHTML('<Rocket.Cat>'), + ' >Hi': s.escapeHTML(' >Hi'), + 'Hello > this is dog': s.escapeHTML('Hello > this is dog'), + 'Roc>ket cat says Hello': s.escapeHTML('Roc>ket cat says Hello'), + 'He said Hello to her>': s.escapeHTML('He said Hello to her>'), + '>Hello': quoteWrapper('Hello'), + '>Rocket.Cat': quoteWrapper('Rocket.Cat'), + '>Hi': quoteWrapper('Hi'), + '> Hello this is dog': quoteWrapper(' Hello this is dog'), + '> Rocket cat says Hello': quoteWrapper(' Rocket cat says Hello'), + '> He said Hello to her': quoteWrapper(' He said Hello to her'), + '': s.escapeHTML(''), + ' >Hi': s.escapeHTML(' >Hi'), + 'Hello > this is dog': s.escapeHTML('Hello > this is dog'), + 'Roc>ket cat says Hello': s.escapeHTML('Roc>ket cat says Hello'), + 'He said Hello to her>': s.escapeHTML('He said Hello to her>'), }; const link = { - '<http://link|Text>': linkWrapped('http://link', 'Text'), - '<https://open.rocket.chat/|Open Site For Rocket.Chat>': linkWrapped('https://open.rocket.chat/', 'Open Site For Rocket.Chat'), - '<https://open.rocket.chat/ | Open Site For Rocket.Chat>': linkWrapped('https://open.rocket.chat/ ', ' Open Site For Rocket.Chat'), - '<https://rocket.chat/|Rocket.Chat Site>': linkWrapped('https://rocket.chat/', 'Rocket.Chat Site'), - '<https://rocket.chat/docs/developer-guides/testing/#testing|Testing Entry on Rocket.Chat Docs Site>': linkWrapped('https://rocket.chat/docs/developer-guides/testing/#testing', 'Testing Entry on Rocket.Chat Docs Site'), - '<http://linkText>': '<http://linkText>', - '<https:open.rocket.chat/ | Open Site For Rocket.Chat>': '<https:open.rocket.chat/ | Open Site For Rocket.Chat>', - 'https://open.rocket.chat/|Open Site For Rocket.Chat': 'https://open.rocket.chat/|Open Site For Rocket.Chat', - '<www.open.rocket.chat/|Open Site For Rocket.Chat>': '<www.open.rocket.chat/|Open Site For Rocket.Chat>', - '<htps://rocket.chat/|Rocket.Chat Site>': '<htps://rocket.chat/|Rocket.Chat Site>', - '<ttps://rocket.chat/|Rocket.Chat Site>': '<ttps://rocket.chat/|Rocket.Chat Site>', - '<tps://rocket.chat/|Rocket.Chat Site>': '<tps://rocket.chat/|Rocket.Chat Site>', - '<open.rocket.chat/|Open Site For Rocket.Chat>': '<open.rocket.chat/|Open Site For Rocket.Chat>', - '<htts://rocket.chat/docs/developer-guides/testing/#testing|Testing Entry on Rocket.Chat Docs Site>': '<htts://rocket.chat/docs/developer-guides/testing/#testing|Testing Entry on Rocket.Chat Docs Site>', + '<http://link|Text>': s.escapeHTML('<http://link|Text>'), + '<https://open.rocket.chat/|Open Site For Rocket.Chat>': s.escapeHTML('<https://open.rocket.chat/|Open Site For Rocket.Chat>'), + '<https://open.rocket.chat/ | Open Site For Rocket.Chat>': s.escapeHTML('<https://open.rocket.chat/ | Open Site For Rocket.Chat>'), + '<https://rocket.chat/|Rocket.Chat Site>': s.escapeHTML('<https://rocket.chat/|Rocket.Chat Site>'), + '<https://rocket.chat/docs/developer-guides/testing/#testing|Testing Entry on Rocket.Chat Docs Site>': s.escapeHTML('<https://rocket.chat/docs/developer-guides/testing/#testing|Testing Entry on Rocket.Chat Docs Site>'), + '<http://linkText>': s.escapeHTML('<http://linkText>'), + '<https:open.rocket.chat/ | Open Site For Rocket.Chat>': s.escapeHTML('<https:open.rocket.chat/ | Open Site For Rocket.Chat>'), + 'https://open.rocket.chat/|Open Site For Rocket.Chat': s.escapeHTML('https://open.rocket.chat/|Open Site For Rocket.Chat'), + '<www.open.rocket.chat/|Open Site For Rocket.Chat>': s.escapeHTML('<www.open.rocket.chat/|Open Site For Rocket.Chat>'), + '<htps://rocket.chat/|Rocket.Chat Site>': s.escapeHTML('<htps://rocket.chat/|Rocket.Chat Site>'), + '<ttps://rocket.chat/|Rocket.Chat Site>': s.escapeHTML('<ttps://rocket.chat/|Rocket.Chat Site>'), + '<tps://rocket.chat/|Rocket.Chat Site>': s.escapeHTML('<tps://rocket.chat/|Rocket.Chat Site>'), + '<open.rocket.chat/|Open Site For Rocket.Chat>': s.escapeHTML('<open.rocket.chat/|Open Site For Rocket.Chat>'), + '<htts://rocket.chat/docs/developer-guides/testing/#testing|Testing Entry on Rocket.Chat Docs Site>': s.escapeHTML('<htts://rocket.chat/docs/developer-guides/testing/#testing|Testing Entry on Rocket.Chat Docs Site>'), '': linkWrapped('http://link', 'Text'), '': linkWrapped('https://open.rocket.chat/', 'Open Site For Rocket.Chat'), '': linkWrapped('https://open.rocket.chat/ ', ' Open Site For Rocket.Chat'), '': linkWrapped('https://rocket.chat/', 'Rocket.Chat Site'), '': linkWrapped('https://rocket.chat/docs/developer-guides/testing/#testing', 'Testing Entry on Rocket.Chat Docs Site'), - '': '', - '': '', - '': '', - '': '', - '': '', - '': '', - '': '', - '': '', + '': s.escapeHTML(''), + '': s.escapeHTML(''), + '': s.escapeHTML(''), + '': s.escapeHTML(''), + '': s.escapeHTML(''), + '': s.escapeHTML(''), + '': s.escapeHTML(''), + '': s.escapeHTML(''), '[Text](http://link)': linkWrapped('http://link', 'Text'), '[Open Site For Rocket.Chat](https://open.rocket.chat/)': linkWrapped('https://open.rocket.chat/', 'Open Site For Rocket.Chat'), @@ -219,15 +221,19 @@ const code = { '```__code__```': codeWrapper('__code__', 'markdown'), }; +const nested = { + '> some quote\n`window.location.reload();`': `${ quoteWrapper(' some quote') }${ inlinecodeWrapper('window.location.reload();') }`, +}; + const defaultObjectTest = (result, object, objectKey) => assert.equal(result.html, object[objectKey]); const testObject = (object, parser = original, test = defaultObjectTest) => { Object.keys(object).forEach((objectKey) => { describe(objectKey, () => { - const result = parser({ html: objectKey }); - result.tokens.forEach((token) => { - result.html = result.html.replace(token.token, token.text); - }); + const message = { + html: s.escapeHTML(objectKey), + }; + const result = Markdown.mountTokensBack(parser(message)); it(`should be equal to ${ object[objectKey] }`, () => { test(result, object, objectKey); }); @@ -259,6 +265,8 @@ describe('Original', function() { describe('Inline Code', () => testObject(inlinecode)); describe('Code', () => testObject(code)); + + describe('Nested', () => testObject(nested)); }); // describe.only('Marked', function() {