From 8b7264a91d63da706fb44f9a3dd34cb4c804e7e0 Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Fri, 3 Sep 2021 12:33:00 -0700 Subject: [PATCH] Add MarkdownString.supportsHtml (#132182) 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 compatibility, `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 --- src/vs/base/browser/markdownRenderer.ts | 26 ++++++++------ src/vs/base/common/htmlContent.ts | 8 +++-- .../test/browser/markdownRenderer.test.ts | 34 +++++++++++++++++++ src/vs/monaco.d.ts | 1 + src/vs/vscode.proposed.d.ts | 17 ++++++++++ .../api/common/extHostTypeConverters.ts | 3 +- src/vs/workbench/api/common/extHostTypes.ts | 10 ++++-- 7 files changed, 83 insertions(+), 16 deletions(-) diff --git a/src/vs/base/browser/markdownRenderer.ts b/src/vs/base/browser/markdownRenderer.ts index 150af9d33212d..449a951e7769a 100644 --- a/src/vs/base/browser/markdownRenderer.ts +++ b/src/vs/base/browser/markdownRenderer.ts @@ -211,17 +211,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 dompurify after this so that we don't rely on - // marked for sanitization. - markedOptions.sanitizer = (html: string): string => { - const match = markdown.isTrusted ? html.match(/^(]+>)|(<\/\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 dompurify after this so that we don't rely on + // marked for sanitization. + markedOptions.sanitizer = (html: string): string => { + const match = markdown.isTrusted ? html.match(/^(]+>)|(<\/\s*span>)$/) : undefined; + return match ? html : ''; + }; + markedOptions.sanitize = true; + markedOptions.silent = true; + } markedOptions.renderer = renderer; diff --git a/src/vs/base/common/htmlContent.ts b/src/vs/base/common/htmlContent.ts index 4f558c1fb9877..2ee678a744a7c 100644 --- a/src/vs/base/common/htmlContent.ts +++ b/src/vs/base/common/htmlContent.ts @@ -11,6 +11,7 @@ export interface IMarkdownString { readonly value: string; readonly isTrusted?: boolean; readonly supportThemeIcons?: boolean; + readonly supportHtml?: boolean; uris?: { [href: string]: UriComponents }; } @@ -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') { @@ -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) => ' '.repeat(g1.length)) - .replace(/^>/gm, '\\>') + .replace(/\>/gm, '\\>') .replace(/\n/g, newlineStyle === MarkdownStringTextNewlineStyle.Break ? '\\\n' : '\n\n'); return this; diff --git a/src/vs/base/test/browser/markdownRenderer.test.ts b/src/vs/base/test/browser/markdownRenderer.test.ts index c2bb2b0cc5303..322e5239dee04 100644 --- a/src/vs/base/test/browser/markdownRenderer.test.ts +++ b/src/vs/base/test/browser/markdownRenderer.test.ts @@ -139,4 +139,38 @@ suite('MarkdownRenderer', () => { assert.strictEqual(result, expected); }); }); + + suite('supportHtml', () => { + test('supportHtml is disabled by default', () => { + const mds = new MarkdownString(undefined, {}); + mds.appendMarkdown('abc'); + + const result = renderMarkdown(mds); + assert.strictEqual(result.innerHTML, `

abc

`); + }); + + test('Renders html when supportHtml=true', () => { + const mds = new MarkdownString(undefined, { supportHtml: true }); + mds.appendMarkdown('abc'); + + const result = renderMarkdown(mds); + assert.strictEqual(result.innerHTML, `

abc

`); + }); + + test('Should not include scripts even when supportHtml=true', () => { + const mds = new MarkdownString(undefined, { supportHtml: true }); + mds.appendMarkdown('abc'); + + const result = renderMarkdown(mds); + assert.strictEqual(result.innerHTML, `

abc

`); + }); + + test('Should not render html appended as text', () => { + const mds = new MarkdownString(undefined, { supportHtml: true }); + mds.appendText('abc'); + + const result = renderMarkdown(mds); + assert.strictEqual(result.innerHTML, `

a<b>b</b>c

`); + }); + }); }); diff --git a/src/vs/monaco.d.ts b/src/vs/monaco.d.ts index 4e4412710cd37..8afb59a098414 100644 --- a/src/vs/monaco.d.ts +++ b/src/vs/monaco.d.ts @@ -409,6 +409,7 @@ declare namespace monaco { readonly value: string; readonly isTrusted?: boolean; readonly supportThemeIcons?: boolean; + readonly supportHtml?: boolean; uris?: { [href: string]: UriComponents; }; diff --git a/src/vs/vscode.proposed.d.ts b/src/vs/vscode.proposed.d.ts index 0af615740f280..726c4de121a79 100644 --- a/src/vs/vscode.proposed.d.ts +++ b/src/vs/vscode.proposed.d.ts @@ -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 } diff --git a/src/vs/workbench/api/common/extHostTypeConverters.ts b/src/vs/workbench/api/common/extHostTypeConverters.ts index 3e9b18aafa955..dfb5840aba91f 100644 --- a/src/vs/workbench/api/common/extHostTypeConverters.ts +++ b/src/vs/workbench/api/common/extHostTypeConverters.ts @@ -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 { @@ -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; } diff --git a/src/vs/workbench/api/common/extHostTypes.ts b/src/vs/workbench/api/common/extHostTypes.ts index 0cbcae57fcc89..93f779b643e8f 100644 --- a/src/vs/workbench/api/common/extHostTypes.ts +++ b/src/vs/workbench/api/common/extHostTypes.ts @@ -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; @@ -1357,8 +1365,6 @@ export class MarkdownString implements vscode.MarkdownString { this.#delegate.appendCodeblock(language ?? '', value); return this; } - - } @es5ClassCompat