Skip to content

Commit

Permalink
All: Fixes laurent22#2640: HTML code within Markdown was rendered inc…
Browse files Browse the repository at this point in the history
…orrectly in some cases
  • Loading branch information
laurent22 committed Mar 4, 2020
1 parent 80fff62 commit cb2df32
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 1 deletion.
1 change: 1 addition & 0 deletions CliClient/tests/md_to_html/sanitize_2.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<p><a href="#" onclick="">Testing <strong>inline</strong> text</a></p>
1 change: 1 addition & 0 deletions CliClient/tests/md_to_html/sanitize_2.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<a href="#" onclick="alert('ohno')">Testing **inline** text</a>
Original file line number Diff line number Diff line change
@@ -1,6 +1,20 @@
const md5 = require('md5');
const htmlUtils = require('../../htmlUtils');

function getOpenTagName(html:string):string {
const m = html.toLowerCase().match(/<([a-z]+)(\s|>)/);
if (!m || m.length < 2) return null;
return m[1];
}

function isSelfClosedTag(html:string):boolean {
return html.substr(-2) === '/>';
}

function stripOffClosingTag(html:string, tagName:string):string {
return html.substr(0, html.length - tagName.length - 3);
}

// @ts-ignore: Keep the function signature as-is despite unusued arguments
function installRule(markdownIt:any, mdOptions:any, ruleOptions:any, context:any) {
markdownIt.core.ruler.push('sanitize_html', (state:any) => {
Expand All @@ -18,8 +32,51 @@ function installRule(markdownIt:any, mdOptions:any, ruleOptions:any, context:any
const cacheKey = md5(escape(token.content));
let sanitizedContent = context.cache.get(cacheKey);

// For html_inline, the content is only a fragment of HTML, as it will be rendered, but
// it's not necessarily valid HTML. For example this HTML:
//
// <a href="#">Testing</a>
//
// will be rendered as three tokens:
//
// html_inline: <a href="#">
// text: Testing
// html_inline: </a>
//
// The problem for us is that when we pass this HTML fragment to the sanitize function
// it is going to turn it into valid HTML. Thus:
//
// "<a href="#">" becomes "<a href="#"></a>"
// "</a>" becomes ""
//
// So the result would be "<a href="#"></a>Testing"
//
// Because of this, we need to be careful with html_inline:
//
// 0. Check if it's an opening or closing tag - only opening ones need to be processed
// 1. Sanitize the fragment
// 2. Strip off the closing tag that was added
//
// Also self-closing tags need to be handled.
//
// html_block is not a problem as the whole content is valid HTML.

if (!sanitizedContent) {
sanitizedContent = htmlUtils.sanitizeHtml(token.content);
if (token.type === 'html_inline') {
const openTagName = getOpenTagName(token.content);
const isSelfClosed = isSelfClosedTag(token.content);

if (!openTagName) {
sanitizedContent = token.content;
} else {
sanitizedContent = htmlUtils.sanitizeHtml(token.content);
if (!isSelfClosed) {
sanitizedContent = stripOffClosingTag(sanitizedContent, openTagName);
}
}
} else { // html_block
sanitizedContent = htmlUtils.sanitizeHtml(token.content);
}
}

token.content = sanitizedContent;
Expand Down
1 change: 1 addition & 0 deletions gulpfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ const updateIgnoredTypeScriptBuildTask = async function() {
'**/.git/**',
'**/ElectronClient/lib/**',
'**/CliClient/build/lib/**',
'**/CliClient/tests-build/lib/**',
'**/ElectronClient/dist/**',
],
}).map(f => f.substr(__dirname.length + 1));
Expand Down

0 comments on commit cb2df32

Please sign in to comment.