KevinGG commented on a change in pull request #12460: URL: https://github.com/apache/beam/pull/12460#discussion_r468764276
########## File path: sdks/python/apache_beam/runners/interactive/extensions/apache-beam-jupyterlab-sidepanel/src/common/HtmlView.tsx ########## @@ -0,0 +1,119 @@ +// Licensed under the Apache License, Version 2.0 (the 'License'); you may not +// use this file except in compliance with the License. You may obtain a copy of +// the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an 'AS IS' BASIS, WITHOUT +// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +// License for the specific language governing permissions and limitations under +// the License. + +import * as React from 'react'; + +export interface IHtmlProvider { + readonly html: string; + readonly script: Array<string>; +} + +interface IHtmlViewProps { + htmlProvider: IHtmlProvider; +} + +interface IHtmlViewState { + innerHtml: string; + script: Array<string>; +} + +/** + * A common HTML viewing component that renders given HTML and executes scripts + * from the given provider. + */ +export class HtmlView extends React.Component<IHtmlViewProps, IHtmlViewState> { + constructor(props: IHtmlViewProps) { + super(props); + this.state = { + innerHtml: props.htmlProvider.html, + script: [] + }; + } + + componentDidMount(): void { + this._updateRenderTimerId = setInterval(() => this.updateRender(), 1000); + } + + componentWillUnmount(): void { + clearInterval(this._updateRenderTimerId); + } + + updateRender(): void { + const currentHtml = this.state.innerHtml; + const htmlToUpdate = this.props.htmlProvider.html; + const currentScript = this.state.script; + const scriptToUpdate = [...this.props.htmlProvider.script]; + if (htmlToUpdate !== currentHtml) { + this.setState({ + innerHtml: htmlToUpdate, + // As long as the html is updated, clear the script state. + script: [] + }); + } + /* Depending on whether this iteration updates the html, the scripts + * are executed differently. + * Html updated: all scripts are new, start execution from index 0; + * Html not updated: only newly added scripts need to be executed. + */ + const currentScriptLength = + htmlToUpdate === currentHtml ? currentScript.length : 0; + if (scriptToUpdate.length > currentScriptLength) { + this.setState( + { + script: scriptToUpdate + }, + // Executes scripts once the state is updated. + () => { + for (let i = currentScriptLength; i < scriptToUpdate.length; ++i) { + new Function(scriptToUpdate[i])(); + } + } + ); + } + } + + render(): React.ReactNode { + return ( + // This injects raw HTML fetched from kernel into JSX. + <div dangerouslySetInnerHTML={{ __html: this.state.innerHtml }} /> Review comment: Yeah, it sounds scary :) [dangerouslySetInnerHTML](https://reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml) is React's replacement for `innerHTML` in the browser DOM. And the warning about using `innerHTML` is [here](https://screenshot.googleplex.com/nJEFJyqWJiG). `iframe` has similar [concerns](https://stackoverflow.com/questions/7289139/why-are-iframes-considered-dangerous-and-a-security-risk#:~:text=If%20you%20control%20the%20content,%2C%20they're%20perfectly%20safe.&text=The%20IFRAME%20element%20may%20be,an%20IFRAME%20on%20hostile%20site.&text=In%20addition%2C%20IFRAME%20element%20may,vulnerability%20which%20can%20be%20exploited). All these concerns are about serving HTML from another domain that might contain malicious code. Here I think it's a valid use case because the HTML and scripts are returned from the kernel generated by the SDK as the user executes codes in their notebooks. They are known and trusted. The purpose of this module is to render the generated visualization. Whether using `dangerouslySetInnerHTML` or `iframe`, to serve the purpose, the HTML has to be rendered and the scripts have to be executed. In the end, either way would do the same thing. The security concern is similar to whether a notebook should execute scripts in an opened ipynb file. By default, Jupyter only renders HTML in the ipynb file (stored as JSON text, identical to this module getting those JSON fields through kernel messaging) and does not execute any script unless the notebook is trusted. A notebook is only trusted after the user executes `jupyter trust notebook.ipynb` or after the user executes the notebook in a kernel. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
