Skip to content

Commit

Permalink
Fix #2693 and sanitization in Live Docs (#2709)
Browse files Browse the repository at this point in the history
  • Loading branch information
fonsp authored Nov 14, 2023
1 parent 4726a20 commit 912f31f
Show file tree
Hide file tree
Showing 7 changed files with 28 additions and 13 deletions.
12 changes: 11 additions & 1 deletion frontend/components/BottomRightPanel.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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}
Expand Down
5 changes: 3 additions & 2 deletions frontend/components/CellOutput.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion frontend/components/Editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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}
Expand Down
10 changes: 7 additions & 3 deletions frontend/components/LiveDocsTab.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,18 @@ 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))

// This is all in a single state object so that we can update multiple field simultaneously
let [state, set_state] = useState({
shown_query: null,
searched_query: null,
body: "<p>Welcome to the <b>Live docs</b>! Keep this little window open while you work on the notebook, and you will get documentation of everything you type!</p><p>You can also type a query above.</p><hr><p><em>Still stuck? Here are <a href='https://julialang.org/about/help/' target='_blank'>some tips</a>.</em></p>",
body: `<p>Welcome to the <b>Live docs</b>! Keep this little window open while you work on the notebook, and you will get documentation of everything you type!</p><p>You can also type a query above.</p><hr><p><em>Still stuck? Here are <a target="_blank" href="https://julialang.org/about/help/">some tips</a>.</em></p>`,
loading: false,
})
let update_state = (mutation) => set_state(immer((state) => mutation(state)))
Expand Down Expand Up @@ -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`
Expand Down
4 changes: 2 additions & 2 deletions frontend/components/TreeView.js
Original file line number Diff line number Diff line change
@@ -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:
Expand All @@ -24,7 +24,7 @@ export const SimpleOutputBody = ({ mime, body, cell_id, persist_js_state, saniti
case "text/plain":
return html`<pre class="no-block">${body}</pre>`
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 })
Expand Down
4 changes: 2 additions & 2 deletions test/frontend/__tests__/safe_preview.js
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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("")
Expand Down
4 changes: 2 additions & 2 deletions test/frontend/fixtures/safe_preview.jl
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ one
"""

# ╔═╡ ef63b97e-700d-11ee-2997-7bf929019c2d
html"""
[[html"""
<div class="zo">
i should not be red
</div>
Expand All @@ -31,7 +31,7 @@ return html`<div style="color: red;">DANGER</div>`
color: red;
}
</style>
"""
"""]]

# ╔═╡ 99e2bfea-4e5d-4d94-bd96-77be7b04811d
html"three"
Expand Down

0 comments on commit 912f31f

Please sign in to comment.