Skip to content

Commit c3f866e

Browse files
mjbvzmjbvz
authored
Merge pull request microsoft#259608 from mjbvz/overseas-cobra
Move replaceWithPlaintext into domSanitize
2 parents 1468416 + 12b2a9c commit c3f866e

File tree

3 files changed

+139
-63
lines changed

3 files changed

+139
-63
lines changed

‎src/vs/base/browser/domSanitize.ts

Lines changed: 59 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -184,9 +184,15 @@ export interface DomSanitizerConfig {
184184
readonly override?: readonly string[];
185185
};
186186

187+
/**
188+
* If set, replaces unsupported tags with their plaintext representation instead of removing them.
189+
*
190+
* For example, <p><bad>"text"</bad></p> becomes <p>"<bad>text</bad>"</p>.
191+
*/
192+
readonly replaceWithPlaintext?: boolean;
193+
187194
// TODO: move these into more controlled api
188195
readonly _do_not_use_hooks?: {
189-
readonly uponSanitizeElement?: UponSanitizeElementCb;
190196
readonly uponSanitizeAttribute?: UponSanitizeAttributeCb;
191197
};
192198
}
@@ -238,8 +244,8 @@ export function sanitizeHtml(untrusted: string, config?: DomSanitizerConfig): Tr
238244
config?.allowedLinkProtocols?.override ?? [Schemas.http, Schemas.https],
239245
config?.allowedMediaProtocols?.override ?? [Schemas.http, Schemas.https]));
240246

241-
if (config?._do_not_use_hooks?.uponSanitizeElement) {
242-
store.add(addDompurifyHook('uponSanitizeElement', config?._do_not_use_hooks.uponSanitizeElement));
247+
if (config?.replaceWithPlaintext) {
248+
store.add(addDompurifyHook('uponSanitizeElement', replaceWithPlainTextHook));
243249
}
244250

245251
if (config?._do_not_use_hooks?.uponSanitizeAttribute) {
@@ -255,6 +261,56 @@ export function sanitizeHtml(untrusted: string, config?: DomSanitizerConfig): Tr
255261
}
256262
}
257263

264+
const selfClosingTags = ['area', 'base', 'br', 'col', 'command', 'embed', 'hr', 'img', 'input', 'keygen', 'link', 'meta', 'param', 'source', 'track', 'wbr'];
265+
266+
function replaceWithPlainTextHook(element: Element, data: dompurify.SanitizeElementHookEvent, _config: dompurify.Config) {
267+
if (!data.allowedTags[data.tagName] && data.tagName !== 'body') {
268+
const replacement = convertTagToPlaintext(element);
269+
if (element.nodeType === Node.COMMENT_NODE) {
270+
// Workaround for https://github.com/cure53/DOMPurify/issues/1005
271+
// The comment will be deleted in the next phase. However if we try to remove it now, it will cause
272+
// an exception. Instead we insert the text node before the comment.
273+
element.parentElement?.insertBefore(replacement, element);
274+
} else {
275+
element.parentElement?.replaceChild(replacement, element);
276+
}
277+
}
278+
}
279+
280+
export function convertTagToPlaintext(element: Element): DocumentFragment {
281+
let startTagText: string;
282+
let endTagText: string | undefined;
283+
if (element.nodeType === Node.COMMENT_NODE) {
284+
startTagText = `<!--${element.textContent}-->`;
285+
} else {
286+
const tagName = element.tagName.toLowerCase();
287+
const isSelfClosing = selfClosingTags.includes(tagName);
288+
const attrString = element.attributes.length ?
289+
' ' + Array.from(element.attributes)
290+
.map(attr => `${attr.name}="${attr.value}"`)
291+
.join(' ')
292+
: '';
293+
startTagText = `<${tagName}${attrString}>`;
294+
if (!isSelfClosing) {
295+
endTagText = `</${tagName}>`;
296+
}
297+
}
298+
299+
const fragment = document.createDocumentFragment();
300+
const textNode = element.ownerDocument.createTextNode(startTagText);
301+
fragment.appendChild(textNode);
302+
while (element.firstChild) {
303+
fragment.appendChild(element.firstChild);
304+
}
305+
306+
const endTagTextNode = endTagText ? element.ownerDocument.createTextNode(endTagText) : undefined;
307+
if (endTagTextNode) {
308+
fragment.appendChild(endTagTextNode);
309+
}
310+
311+
return fragment;
312+
}
313+
258314
/**
259315
* Sanitizes the given `value` and reset the given `node` with it.
260316
*/

‎src/vs/base/browser/markdownRenderer.ts

Lines changed: 19 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import { escape } from '../common/strings.js';
2020
import { URI } from '../common/uri.js';
2121
import * as DOM from './dom.js';
2222
import * as domSanitize from './domSanitize.js';
23+
import { convertTagToPlaintext } from './domSanitize.js';
2324
import { DomEmitter } from './event.js';
2425
import { FormattedTextRenderOptions } from './formattedTextRenderer.js';
2526
import { StandardKeyboardEvent } from './keyboardEvent.js';
@@ -211,6 +212,20 @@ export function renderMarkdown(markdown: IMarkdownString, options: MarkdownRende
211212
}));
212213
}
213214

215+
// Remove/disable inputs
216+
for (const input of [...element.getElementsByTagName('input')]) {
217+
if (input.attributes.getNamedItem('type')?.value === 'checkbox') {
218+
input.setAttribute('disabled', '');
219+
} else {
220+
if (options.sanitizerConfig?.replaceWithPlaintext) {
221+
const replacement = convertTagToPlaintext(input);
222+
input.parentElement?.replaceChild(replacement, input);
223+
} else {
224+
input.remove();
225+
}
226+
}
227+
}
228+
214229
return {
215230
element,
216231
dispose: () => {
@@ -412,15 +427,12 @@ function resolveWithBaseUri(baseUri: URI, href: string): string {
412427
}
413428
}
414429

415-
416-
const selfClosingTags = ['area', 'base', 'br', 'col', 'command', 'embed', 'hr', 'img', 'input', 'keygen', 'link', 'meta', 'param', 'source', 'track', 'wbr'];
417-
418430
function sanitizeRenderedMarkdown(
419431
renderedMarkdown: string,
420432
isTrusted: boolean | MarkdownStringTrustedOptions,
421433
options: MarkdownSanitizerConfig = {},
422434
): TrustedHTML {
423-
const sanitizerConfig = getSanitizerOptions(isTrusted, options);
435+
const sanitizerConfig = getDomSanitizerConfig(isTrusted, options);
424436
return domSanitize.sanitizeHtml(renderedMarkdown, sanitizerConfig);
425437
}
426438

@@ -452,6 +464,7 @@ export const allowedMarkdownHtmlAttributes = [
452464
'type',
453465
'width',
454466
'start',
467+
'value',
455468

456469
// Custom markdown attributes
457470
'data-code',
@@ -462,7 +475,7 @@ export const allowedMarkdownHtmlAttributes = [
462475
'class',
463476
];
464477

465-
function getSanitizerOptions(isTrusted: boolean | MarkdownStringTrustedOptions, options: MarkdownSanitizerConfig): domSanitize.DomSanitizerConfig {
478+
function getDomSanitizerConfig(isTrusted: boolean | MarkdownStringTrustedOptions, options: MarkdownSanitizerConfig): domSanitize.DomSanitizerConfig {
466479
const allowedLinkSchemes = [
467480
Schemas.http,
468481
Schemas.https,
@@ -507,6 +520,7 @@ function getSanitizerOptions(isTrusted: boolean | MarkdownStringTrustedOptions,
507520
Schemas.vscodeRemoteResource,
508521
]
509522
},
523+
replaceWithPlaintext: options.replaceWithPlaintext,
510524
_do_not_use_hooks: {
511525
uponSanitizeAttribute: (element, e) => {
512526
if (options.customAttrSanitizer) {
@@ -545,61 +559,6 @@ function getSanitizerOptions(isTrusted: boolean | MarkdownStringTrustedOptions,
545559
e.keepAttr = false;
546560
}
547561
},
548-
uponSanitizeElement: (element, e) => {
549-
let wantsReplaceWithPlaintext = false;
550-
if (e.tagName === 'input') {
551-
if (element.attributes.getNamedItem('type')?.value === 'checkbox') {
552-
element.setAttribute('disabled', '');
553-
} else if (options.replaceWithPlaintext) {
554-
wantsReplaceWithPlaintext = true;
555-
} else {
556-
element.remove();
557-
return;
558-
}
559-
}
560-
561-
if (options.replaceWithPlaintext && (wantsReplaceWithPlaintext || (!e.allowedTags[e.tagName] && e.tagName !== 'body'))) {
562-
if (element.parentElement) {
563-
let startTagText: string;
564-
let endTagText: string | undefined;
565-
if (e.tagName === '#comment') {
566-
startTagText = `<!--${element.textContent}-->`;
567-
} else {
568-
const isSelfClosing = selfClosingTags.includes(e.tagName);
569-
const attrString = element.attributes.length ?
570-
' ' + Array.from(element.attributes)
571-
.map(attr => `${attr.name}="${attr.value}"`)
572-
.join(' ')
573-
: '';
574-
startTagText = `<${e.tagName}${attrString}>`;
575-
if (!isSelfClosing) {
576-
endTagText = `</${e.tagName}>`;
577-
}
578-
}
579-
580-
const fragment = document.createDocumentFragment();
581-
const textNode = element.parentElement.ownerDocument.createTextNode(startTagText);
582-
fragment.appendChild(textNode);
583-
const endTagTextNode = endTagText ? element.parentElement.ownerDocument.createTextNode(endTagText) : undefined;
584-
while (element.firstChild) {
585-
fragment.appendChild(element.firstChild);
586-
}
587-
588-
if (endTagTextNode) {
589-
fragment.appendChild(endTagTextNode);
590-
}
591-
592-
if (element.nodeType === Node.COMMENT_NODE) {
593-
// Workaround for https://github.com/cure53/DOMPurify/issues/1005
594-
// The comment will be deleted in the next phase. However if we try to remove it now, it will cause
595-
// an exception. Instead we insert the text node before the comment.
596-
element.parentElement.insertBefore(fragment, element);
597-
} else {
598-
element.parentElement.replaceChild(fragment, element);
599-
}
600-
}
601-
}
602-
}
603562
}
604563
};
605564
}

‎src/vs/base/test/browser/domSanitize.test.ts

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,4 +113,65 @@ suite('DomSanitize', () => {
113113

114114
assert.ok(str.includes('src="data:image/png;base64,'));
115115
});
116+
117+
suite('replaceWithPlaintext', () => {
118+
119+
test('replaces unsupported tags with plaintext representation', () => {
120+
const html = '<div>safe<script>alert(1)</script>content</div>';
121+
const result = sanitizeHtml(html, {
122+
replaceWithPlaintext: true
123+
});
124+
const str = result.toString();
125+
assert.strictEqual(str, `<div>safe&lt;script&gt;alert(1)&lt;/script&gt;content</div>`);
126+
});
127+
128+
test('handles self-closing tags correctly', () => {
129+
const html = '<div><input type="text"><custom-input /></div>';
130+
const result = sanitizeHtml(html, {
131+
replaceWithPlaintext: true
132+
});
133+
assert.strictEqual(result.toString(), '<div>&lt;input type="text"&gt;&lt;custom-input&gt;&lt;/custom-input&gt;</div>');
134+
});
135+
136+
test('handles tags with attributes', () => {
137+
const html = '<div><unknown-tag class="test" id="myid">content</unknown-tag></div>';
138+
const result = sanitizeHtml(html, {
139+
replaceWithPlaintext: true
140+
});
141+
assert.strictEqual(result.toString(), '<div>&lt;unknown-tag class="test" id="myid"&gt;content&lt;/unknown-tag&gt;</div>');
142+
});
143+
144+
test('handles nested unsupported tags', () => {
145+
const html = '<div><outer><inner>nested</inner></outer></div>';
146+
const result = sanitizeHtml(html, {
147+
replaceWithPlaintext: true
148+
});
149+
assert.strictEqual(result.toString(), '<div>&lt;outer&gt;&lt;inner&gt;nested&lt;/inner&gt;&lt;/outer&gt;</div>');
150+
});
151+
152+
test('handles comments correctly', () => {
153+
const html = '<div><!-- this is a comment -->content</div>';
154+
const result = sanitizeHtml(html, {
155+
replaceWithPlaintext: true
156+
});
157+
assert.strictEqual(result.toString(), '<div>&lt;!-- this is a comment --&gt;content</div>');
158+
});
159+
160+
test('handles empty tags', () => {
161+
const html = '<div><empty></empty></div>';
162+
const result = sanitizeHtml(html, {
163+
replaceWithPlaintext: true
164+
});
165+
assert.strictEqual(result.toString(), '<div>&lt;empty&gt;&lt;/empty&gt;</div>');
166+
});
167+
168+
test('works with custom allowed tags configuration', () => {
169+
const html = '<div><custom>allowed</custom><forbidden>not allowed</forbidden></div>';
170+
const result = sanitizeHtml(html, {
171+
replaceWithPlaintext: true,
172+
allowedTags: { augment: ['custom'] }
173+
});
174+
assert.strictEqual(result.toString(), '<div><custom>allowed</custom>&lt;forbidden&gt;not allowed&lt;/forbidden&gt;</div>');
175+
});
176+
});
116177
});

0 commit comments

Comments
 (0)