Skip to content

Commit

Permalink
Add MarkdownString.supportsHtml
Browse files Browse the repository at this point in the history
Fixes #40607

This change introduces a new `supportsHtml` property on `MarkdownString` that can be used to enable rendering of a safe subset of tags and attributes inside of markdown strings

For backwards compatability, `supportsHtml` will default to false and must be explicitly enabled by extensions

This PR will need to go in after we adopt dompurify (#131950) which should provide better control over how we actually go about sanitizing rendered html
  • Loading branch information
mjbvz committed Sep 3, 2021
1 parent aec6ee0 commit d007982
Show file tree
Hide file tree
Showing 7 changed files with 84 additions and 17 deletions.
28 changes: 16 additions & 12 deletions src/vs/base/browser/markdownRenderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,17 +217,21 @@ export function renderMarkdown(markdown: IMarkdownString, options: MarkdownRende
}));
}

// Use our own sanitizer so that we can let through only spans.
// Otherwise, we'd be letting all html be rendered.
// If we want to allow markdown permitted tags, then we can delete sanitizer and sanitize.
// We always pass the output through insane after this so that we don't rely on
// marked for sanitization.
markedOptions.sanitizer = (html: string): string => {
const match = markdown.isTrusted ? html.match(/^(<span[^>]+>)|(<\/\s*span>)$/) : undefined;
return match ? html : '';
};
markedOptions.sanitize = true;
markedOptions.silent = true;
if (!markdown.supportHtml) {
// TODO: Can we deprecated this in favor of 'supportHtml'?

// Use our own sanitizer so that we can let through only spans.
// Otherwise, we'd be letting all html be rendered.
// If we want to allow markdown permitted tags, then we can delete sanitizer and sanitize.
// We always pass the output through insane after this so that we don't rely on
// marked for sanitization.
markedOptions.sanitizer = (html: string): string => {
const match = markdown.isTrusted ? html.match(/^(<span[^>]+>)|(<\/\s*span>)$/) : undefined;
return match ? html : '';
};
markedOptions.sanitize = true;
markedOptions.silent = true;
}

markedOptions.renderer = renderer;

Expand Down Expand Up @@ -293,7 +297,7 @@ function getInsaneOptions(options: { readonly isTrusted?: boolean }): InsaneOpti
// Since we have our own sanitize function for marked, it's possible we missed some tag so let insane make sure.
// HTML tags that can result from markdown are from reading https://spec.commonmark.org/0.29/
// HTML table tags that can result from markdown are from https://github.github.com/gfm/#tables-extension-
allowedTags: ['ul', 'li', 'p', 'code', 'blockquote', 'ol', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'hr', 'em', 'pre', 'table', 'thead', 'tbody', 'tr', 'th', 'td', 'div', 'del', 'a', 'strong', 'br', 'img', 'span'],
allowedTags: ['ul', 'li', 'p', 'code', 'blockquote', 'ol', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'hr', 'em', 'pre', 'table', 'thead', 'tbody', 'tr', 'th', 'td', 'div', 'del', 'a', 'b', 'i', 'strong', 'br', 'img', 'span'],
allowedAttributes: {
'a': ['href', 'name', 'target', 'data-href'],
'img': ['src', 'title', 'alt', 'width', 'height'],
Expand Down
8 changes: 6 additions & 2 deletions src/vs/base/common/htmlContent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ export interface IMarkdownString {
readonly value: string;
readonly isTrusted?: boolean;
readonly supportThemeIcons?: boolean;
readonly supportHtml?: boolean;
uris?: { [href: string]: UriComponents };
}

Expand All @@ -24,10 +25,11 @@ export class MarkdownString implements IMarkdownString {
public value: string;
public isTrusted?: boolean;
public supportThemeIcons?: boolean;
public supportHtml?: boolean;

constructor(
value: string = '',
isTrustedOrOptions: boolean | { isTrusted?: boolean, supportThemeIcons?: boolean } = false,
isTrustedOrOptions: boolean | { isTrusted?: boolean, supportThemeIcons?: boolean, supportHtml?: boolean } = false,
) {
this.value = value;
if (typeof this.value !== 'string') {
Expand All @@ -37,17 +39,19 @@ export class MarkdownString implements IMarkdownString {
if (typeof isTrustedOrOptions === 'boolean') {
this.isTrusted = isTrustedOrOptions;
this.supportThemeIcons = false;
this.supportHtml = false;
}
else {
this.isTrusted = isTrustedOrOptions.isTrusted ?? undefined;
this.supportThemeIcons = isTrustedOrOptions.supportThemeIcons ?? false;
this.supportHtml = isTrustedOrOptions.supportHtml ?? false;
}
}

appendText(value: string, newlineStyle: MarkdownStringTextNewlineStyle = MarkdownStringTextNewlineStyle.Paragraph): MarkdownString {
this.value += escapeMarkdownSyntaxTokens(this.supportThemeIcons ? escapeIcons(value) : value)
.replace(/([ \t]+)/g, (_match, g1) => '&nbsp;'.repeat(g1.length))
.replace(/^>/gm, '\\>')
.replace(/\>/gm, '\\>')
.replace(/\n/g, newlineStyle === MarkdownStringTextNewlineStyle.Break ? '\\\n' : '\n\n');

return this;
Expand Down
34 changes: 34 additions & 0 deletions src/vs/base/test/browser/markdownRenderer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,4 +131,38 @@ suite('MarkdownRenderer', () => {
assert.strictEqual(result, expected);
});
});

suite('supportHtml', () => {
test('supportHtml is disabled by default', () => {
const mds = new MarkdownString(undefined, {});
mds.appendMarkdown('a<b>b</b>c');

const result = renderMarkdown(mds);
assert.strictEqual(result.innerHTML, `<p>abc</p>`);
});

test('Renders html when supportHtml=true', () => {
const mds = new MarkdownString(undefined, { supportHtml: true });
mds.appendMarkdown('a<b>b</b>c');

const result = renderMarkdown(mds);
assert.strictEqual(result.innerHTML, `<p>a<b>b</b>c</p>`);
});

test('Should not include scripts even when supportHtml=true', () => {
const mds = new MarkdownString(undefined, { supportHtml: true });
mds.appendMarkdown('a<b onclick="alert(1)">b</b><script>alert(2)</script>c');

const result = renderMarkdown(mds);
assert.strictEqual(result.innerHTML, `<p>a<b>b</b>c</p>`);
});

test('Should not render html appended as text', () => {
const mds = new MarkdownString(undefined, { supportHtml: true });
mds.appendText('a<b>b</b>c');

const result = renderMarkdown(mds);
assert.strictEqual(result.innerHTML, `<p>a&lt;b&gt;b&lt;/b&gt;c</p>`);
});
});
});
1 change: 1 addition & 0 deletions src/vs/monaco.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,7 @@ declare namespace monaco {
readonly value: string;
readonly isTrusted?: boolean;
readonly supportThemeIcons?: boolean;
readonly supportHtml?: boolean;
uris?: {
[href: string]: UriComponents;
};
Expand Down
17 changes: 17 additions & 0 deletions src/vs/vscode.proposed.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2947,4 +2947,21 @@ declare module 'vscode' {
}

//#endregion

//#region @mjbvz https://github.com/microsoft/vscode/issues/40607
export interface MarkdownString {
/**
* Indicates that this markdown string can contain raw html tags. Default to false.
*
* When `supportHtml` is false, the markdown renderer will strip out any raw html tags
* that appear in the markdown text. This means you can only use markdown syntax for rendering.
*
* When `supportHtml` is true, the markdown render will also allow a safe subset of html tags
* and attributes to be rendered. See https://github.com/microsoft/vscode/blob/6d2920473c6f13759c978dd89104c4270a83422d/src/vs/base/browser/markdownRenderer.ts#L296
* for a list of all supported tags and attributes.
*/
supportHtml?: boolean;
}

//#endregion
}
3 changes: 2 additions & 1 deletion src/vs/workbench/api/common/extHostTypeConverters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ export namespace MarkdownString {
const { language, value } = markup;
res = { value: '```' + language + '\n' + value + '\n```\n' };
} else if (types.MarkdownString.isMarkdownString(markup)) {
res = { value: markup.value, isTrusted: markup.isTrusted, supportThemeIcons: markup.supportThemeIcons };
res = { value: markup.value, isTrusted: markup.isTrusted, supportThemeIcons: markup.supportThemeIcons, supportHtml: markup.supportHtml };
} else if (typeof markup === 'string') {
res = { value: markup };
} else {
Expand Down Expand Up @@ -345,6 +345,7 @@ export namespace MarkdownString {
export function to(value: htmlContent.IMarkdownString): vscode.MarkdownString {
const result = new types.MarkdownString(value.value, value.supportThemeIcons);
result.isTrusted = value.isTrusted;
result.supportHtml = value.supportHtml;
return result;
}

Expand Down
10 changes: 8 additions & 2 deletions src/vs/workbench/api/common/extHostTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1343,6 +1343,14 @@ export class MarkdownString implements vscode.MarkdownString {
this.#delegate.supportThemeIcons = value;
}

get supportHtml(): boolean | undefined {
return this.#delegate.supportHtml;
}

set supportHtml(value: boolean | undefined) {
this.#delegate.supportHtml = value;
}

appendText(value: string): vscode.MarkdownString {
this.#delegate.appendText(value);
return this;
Expand All @@ -1357,8 +1365,6 @@ export class MarkdownString implements vscode.MarkdownString {
this.#delegate.appendCodeblock(language ?? '', value);
return this;
}


}

@es5ClassCompat
Expand Down

0 comments on commit d007982

Please sign in to comment.