Skip to content

Commit 1edb801

Browse files
jmcphersjmcphers
authored
Render <!DOCTYPE> html to the Viewer pane (#6274)
This change addresses an issue in which HTML that contains a `<!DOCTYPE>` tag but _not_ an `<html>` tag will break the Console. The underlying problem is that the naive HTML parser tries to create a React element for the `!DOCTYPE` tag, which creates an exception that causes the whole console component to fail to render. There are three layers of protection added in this fix: - First, HTML returned by runtimes is inspected for `!DOCTYPE` tags. If any are found, the HTML is rendered in the Viewer pane (where it can be rendered safely in an iframe) instead of in the console. - Second, the HTML renderer now safely skips any `!TAGNAME` tags. - Third, some exception handling in the HTML renderer helps guard against exceptions taking down the whole Console. This is probably not the last issue we'll have around the heuristic way we try to render HTML returned from kernels, which is homespun and has a hard time with these kinds of edge cases. Addresses #6258. ### Release Notes <!-- Optionally, replace `N/A` with text to be included in the next release notes. The `N/A` bullets are ignored. If you refer to one or more Positron issues, these issues are used to collect information about the feature or bugfix, such as the relevant language pack as determined by Github labels of type `lang: `. The note will automatically be tagged with the language. These notes are typically filled by the Positron team. If you are an external contributor, you may ignore this section. --> #### New Features - N/A #### Bug Fixes - Fix issue causing some Polars query plans to break the Console when printed (#6258) ### QA Notes Note that this issue is easiest to reproduce in a browser-based environment. Printing these objects should now result in the query plan to show up in the Viewer, like this: <img width="454" alt="image" src="https://github.com/user-attachments/assets/c32d8d6e-6e7b-4439-a22f-a37dd905207f" /> Test tags: `@:console`
1 parent bab2b0c commit 1edb801

File tree

3 files changed

+34
-8
lines changed

3 files changed

+34
-8
lines changed

‎src/vs/base/browser/positron/renderHtml.tsx

+27-4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*---------------------------------------------------------------------------------------------
2-
* Copyright (C) 2024 Posit Software, PBC. All rights reserved.
2+
* Copyright (C) 2024-2025 Posit Software, PBC. All rights reserved.
33
* Licensed under the Elastic License 2.0. See LICENSE.txt for license information.
44
*--------------------------------------------------------------------------------------------*/
55

@@ -24,6 +24,11 @@ interface HTMLRendererOptions {
2424
/**
2525
* Renders HTML to React elements.
2626
*
27+
* Since throwing an exception here will cause the entire React render to fail,
28+
* this component renders any errors instead of throwing them. This means that
29+
* rendering invalid HTML may produce a rendered error instead of the expected
30+
* content.
31+
*
2732
* @param html A string of untrusted HTML.
2833
* @param opts Options for rendering the HTML.
2934
* @returns A React element containing the rendered HTML.
@@ -35,10 +40,22 @@ export const renderHtml = (html: string, opts: HTMLRendererOptions = {}): React.
3540
//
3641
// Because this code must run in a very strict security context, we cannot
3742
// use parsers that rely on `innerHTML` or `DOMParser`.
38-
const parsedContent = parseHtml(html);
43+
let parsedContent = [];
44+
try {
45+
parsedContent = parseHtml(html);
46+
} catch (e) {
47+
// If the HTML is invalid, render an error message.
48+
const errorMessage = e instanceof Error ? e.message : e.toString();
49+
return <div className='error'>{errorMessage}</div>;
50+
}
3951

4052
// If there are component over-rides, use those to render the applicable elements.
4153
function createElement(name: string, attrs: React.DOMAttributes<HTMLElement>, children?: (React.ReactNode | string)[] | React.ReactNode | string) {
54+
// Don't try to create elements for tags that start with ! (such as
55+
// <!DOCTYPE html>).
56+
if (name && name[0] === '!') {
57+
return undefined;
58+
}
4259
const Component = opts.componentOverrides?.[name] || name;
4360
return React.createElement(Component, attrs, children);
4461
}
@@ -78,7 +95,6 @@ export const renderHtml = (html: string, opts: HTMLRendererOptions = {}): React.
7895
}
7996
}
8097
} else if (node.type === 'tag') {
81-
// Create a React element for the tag.
8298
return createElement(node.name!, node.attrs!);
8399
} else {
84100
// We don't render other types of nodes.
@@ -87,7 +103,14 @@ export const renderHtml = (html: string, opts: HTMLRendererOptions = {}): React.
87103
};
88104

89105
// Render all the nodes.
90-
const renderedNodes = parsedContent.map(renderNode);
106+
let renderedNodes = [];
107+
try {
108+
renderedNodes = parsedContent.map(renderNode);
109+
} catch (e) {
110+
// Show any errors that occur during rendering.
111+
const errorMessage = e instanceof Error ? e.message : e.toString();
112+
return <div className='error'>{errorMessage}</div>;
113+
}
91114

92115
return <div>{renderedNodes}</div>;
93116
};

‎src/vs/workbench/api/browser/positron/mainThreadLanguageRuntime.ts

+5-3
Original file line numberDiff line numberDiff line change
@@ -817,10 +817,12 @@ class ExtHostLanguageRuntimeSessionAdapter implements ILanguageRuntimeSession {
817817
if (mimeTypes.includes('text/html')) {
818818
// Check to see if there are any tags that look like they belong in
819819
// a standalone HTML document.
820-
if (/<(script|html|body|iframe)/.test(message.data['text/html'])) {
820+
const htmlContent = message.data['text/html'];
821+
if (/<(script|html|body|iframe|!DOCTYPE)/.test(htmlContent)) {
821822
// This looks like standalone HTML.
822-
if (message.data['text/html'].includes('<table')) {
823-
// Tabular data? Probably best in the Viewer pane.
823+
if (htmlContent.includes('<table') ||
824+
htmlContent.includes('<!DOCTYPE')) {
825+
// Tabular data or document? Probably best in the Viewer pane.
824826
return RuntimeOutputKind.ViewerWidget;
825827
} else {
826828
// Guess that anything else is a plot.

‎src/vs/workbench/services/positronConsole/browser/positronConsoleService.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -1545,7 +1545,8 @@ class PositronConsoleInstance extends Disposable implements IPositronConsoleInst
15451545
if (htmlContent.indexOf('<script') >= 0 ||
15461546
htmlContent.indexOf('<body') >= 0 ||
15471547
htmlContent.indexOf('<html') >= 0 ||
1548-
htmlContent.indexOf('<iframe') >= 0) {
1548+
htmlContent.indexOf('<iframe') >= 0 ||
1549+
htmlContent.indexOf('<!doctype') >= 0) {
15491550
// We only want to render HTML fragments for now; if it has
15501551
// scripts or looks like it is a self-contained document,
15511552
// hard pass. In the future, we'll need to render those in a

0 commit comments

Comments
 (0)