From 912f31fa4bd4560ae593c4b2a08d2fb000e38eb1 Mon Sep 17 00:00:00 2001 From: Fons van der Plas Date: Tue, 14 Nov 2023 14:20:14 +0100 Subject: [PATCH] Fix #2693 and sanitization in Live Docs (#2709) --- frontend/components/BottomRightPanel.js | 12 +++++++++++- frontend/components/CellOutput.js | 5 +++-- frontend/components/Editor.js | 2 +- frontend/components/LiveDocsTab.js | 10 +++++++--- frontend/components/TreeView.js | 4 ++-- test/frontend/__tests__/safe_preview.js | 4 ++-- test/frontend/fixtures/safe_preview.jl | 4 ++-- 7 files changed, 28 insertions(+), 13 deletions(-) diff --git a/frontend/components/BottomRightPanel.js b/frontend/components/BottomRightPanel.js index 25bbbb2649..f7ade801c4 100644 --- a/frontend/components/BottomRightPanel.js +++ b/frontend/components/BottomRightPanel.js @@ -21,9 +21,18 @@ export const open_bottom_right_panel = (/** @type {PanelTabName} */ tab) => wind * connected: boolean, * backend_launch_phase: number?, * backend_launch_logs: string?, + * sanitize_html?: boolean, * }} props */ -export let BottomRightPanel = ({ desired_doc_query, on_update_doc_query, notebook, connected, backend_launch_phase, backend_launch_logs }) => { +export let BottomRightPanel = ({ + desired_doc_query, + on_update_doc_query, + notebook, + connected, + backend_launch_phase, + backend_launch_logs, + sanitize_html = true, +}) => { let container_ref = useRef() const focus_docs_on_open_ref = useRef(false) @@ -125,6 +134,7 @@ export let BottomRightPanel = ({ desired_doc_query, on_update_doc_query, noteboo desired_doc_query=${desired_doc_query} on_update_doc_query=${on_update_doc_query} notebook=${notebook} + sanitize_html=${sanitize_html} />` : open_tab === "process" ? html`<${ProcessTab} diff --git a/frontend/components/CellOutput.js b/frontend/components/CellOutput.js index c1000af7dd..007f096582 100644 --- a/frontend/components/CellOutput.js +++ b/frontend/components/CellOutput.js @@ -493,7 +493,7 @@ let declarative_shadow_dom_polyfill = (template) => { } } -export let RawHTMLContainer = ({ body, className = "", persist_js_state = false, last_run_timestamp, sanitize_html = true }) => { +export let RawHTMLContainer = ({ body, className = "", persist_js_state = false, last_run_timestamp, sanitize_html = true, sanitize_html_message = true }) => { let pluto_actions = useContext(PlutoActionsContext) let pluto_bonds = useContext(PlutoBondsContext) let js_init_set = useContext(PlutoJSInitializingContext) @@ -525,13 +525,14 @@ export let RawHTMLContainer = ({ body, className = "", persist_js_state = false, let html_content_to_set = sanitize_html ? DOMPurify.sanitize(body, { FORBID_TAGS: ["style"], + ADD_ATTR: ["target"], }) : body // Actually "load" the html container.innerHTML = html_content_to_set - if (html_content_to_set !== body) { + if (sanitize_html_message && html_content_to_set !== body) { // DOMPurify also resolves HTML entities, which can give a false positive. To fix this, we use DOMParser to parse both strings, and we compare the innerHTML of the resulting documents. const parser = new DOMParser() const p1 = parser.parseFromString(body, "text/html") diff --git a/frontend/components/Editor.js b/frontend/components/Editor.js index f04aa98dea..4aa9bef518 100644 --- a/frontend/components/Editor.js +++ b/frontend/components/Editor.js @@ -29,7 +29,6 @@ import { slider_server_actions, nothing_actions } from "../common/SliderServerCl import { ProgressBar } from "./ProgressBar.js" import { NonCellOutput } from "./NonCellOutput.js" import { IsolatedCell } from "./Cell.js" -import { RawHTMLContainer } from "./CellOutput.js" import { RecordingPlaybackUI, RecordingUI } from "./RecordingUI.js" import { HijackExternalLinksToOpenInNewTab } from "./HackySideStuff/HijackExternalLinksToOpenInNewTab.js" import { FrontMatterInput } from "./FrontmatterInput.js" @@ -1630,6 +1629,7 @@ patch: ${JSON.stringify( backend_launch_phase=${this.state.backend_launch_phase} backend_launch_logs=${this.state.backend_launch_logs} notebook=${this.state.notebook} + sanitize_html=${status.sanitize_html} /> <${Popup} notebook=${this.state.notebook} diff --git a/frontend/components/LiveDocsTab.js b/frontend/components/LiveDocsTab.js index 156297445f..b26c1e27bc 100644 --- a/frontend/components/LiveDocsTab.js +++ b/frontend/components/LiveDocsTab.js @@ -12,9 +12,10 @@ import { cl } from "../common/ClassTable.js" * desired_doc_query: string?, * on_update_doc_query: (query: string) => void, * notebook: import("./Editor.js").NotebookData, + * sanitize_html?: boolean, * }} props */ -export let LiveDocsTab = ({ focus_on_open, desired_doc_query, on_update_doc_query, notebook }) => { +export let LiveDocsTab = ({ focus_on_open, desired_doc_query, on_update_doc_query, notebook, sanitize_html = true }) => { let pluto_actions = useContext(PlutoActionsContext) let live_doc_search_ref = useRef(/** @type {HTMLInputElement?} */ (null)) @@ -22,7 +23,7 @@ export let LiveDocsTab = ({ focus_on_open, desired_doc_query, on_update_doc_quer let [state, set_state] = useState({ shown_query: null, searched_query: null, - body: "

Welcome to the Live docs! Keep this little window open while you work on the notebook, and you will get documentation of everything you type!

You can also type a query above.


Still stuck? Here are some tips.

", + body: `

Welcome to the Live docs! Keep this little window open while you work on the notebook, and you will get documentation of everything you type!

You can also type a query above.


Still stuck? Here are some tips.

`, loading: false, }) let update_state = (mutation) => set_state(immer((state) => mutation(state))) @@ -74,7 +75,10 @@ export let LiveDocsTab = ({ focus_on_open, desired_doc_query, on_update_doc_quer }) } - let docs_element = useMemo(() => html`<${RawHTMLContainer} body=${without_workspace_stuff(state.body)} />`, [state.body]) + let docs_element = useMemo( + () => html`<${RawHTMLContainer} body=${without_workspace_stuff(state.body)} sanitize_html=${sanitize_html} sanitize_html_message=${false} />`, + [state.body, sanitize_html] + ) let no_docs_found = state.loading === false && state.searched_query !== "" && state.searched_query !== state.shown_query return html` diff --git a/frontend/components/TreeView.js b/frontend/components/TreeView.js index ccd1645b63..c15e0c2822 100644 --- a/frontend/components/TreeView.js +++ b/frontend/components/TreeView.js @@ -1,6 +1,6 @@ import { html, useRef, useState, useContext } from "../imports/Preact.js" -import { OutputBody, PlutoImage, RawHTMLContainer } from "./CellOutput.js" +import { OutputBody, PlutoImage } from "./CellOutput.js" import { PlutoActionsContext } from "../common/PlutoContext.js" // this is different from OutputBody because: @@ -24,7 +24,7 @@ export const SimpleOutputBody = ({ mime, body, cell_id, persist_js_state, saniti case "text/plain": return html`
${body}
` case "application/vnd.pluto.tree+object": - return html`<${TreeView} cell_id=${cell_id} body=${body} persist_js_state=${persist_js_state} />` + return html`<${TreeView} cell_id=${cell_id} body=${body} persist_js_state=${persist_js_state} sanitize_html=${sanitize_html} />` break default: return OutputBody({ mime, body, cell_id, persist_js_state, sanitize_html, last_run_timestamp: null }) diff --git a/test/frontend/__tests__/safe_preview.js b/test/frontend/__tests__/safe_preview.js index e8b37b1d03..b6c8995371 100644 --- a/test/frontend/__tests__/safe_preview.js +++ b/test/frontend/__tests__/safe_preview.js @@ -200,7 +200,7 @@ Hello let cell_contents = await getAllCellOutputs(page) expect(cell_contents[0]).toBe("one") - expect(cell_contents[1]).toBe("Scripts and styles not rendered in Safe preview\ni should not be red\ntwo\nsafe") + expect(cell_contents[1]).toBe("Scripts and styles not rendered in Safe preview\n\ni should not be red\n\n\n\ntwo\n\n\nsafe") expect(cell_contents[2]).toBe("three") expect(cell_contents[3]).toBe("Code not executed in Safe preview") expect(cell_contents[4]).toBe("Code not executed in Safe preview") @@ -234,7 +234,7 @@ Hello cell_contents = await getAllCellOutputs(page) expect(cell_contents[0]).toBe("one") - expect(cell_contents[1]).toBe("i should not be red\ntwo\nsafe\nDANGER") + expect(cell_contents[1]).toBe("\ni should not be red\n\ntwo\nsafe\nDANGER") expect(cell_contents[2]).toBe("three") expect(cell_contents[3]).toBe("123") expect(cell_contents[4]).toBe("") diff --git a/test/frontend/fixtures/safe_preview.jl b/test/frontend/fixtures/safe_preview.jl index 7fb6014e80..ffc7439aef 100644 --- a/test/frontend/fixtures/safe_preview.jl +++ b/test/frontend/fixtures/safe_preview.jl @@ -10,7 +10,7 @@ one """ # ╔═╡ ef63b97e-700d-11ee-2997-7bf929019c2d -html""" +[[html"""
i should not be red
@@ -31,7 +31,7 @@ return html`
DANGER
` color: red; } -""" +"""]] # ╔═╡ 99e2bfea-4e5d-4d94-bd96-77be7b04811d html"three"