Skip to content

Commit e38ee3f

Browse files
authored
safer html interpolation (#229)
* fix #223; safer html interpolation * eslint quotes * avoidEscape
1 parent 0c0c358 commit e38ee3f

File tree

14 files changed

+161
-110
lines changed

14 files changed

+161
-110
lines changed

.eslintrc.json

+3-2
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,9 @@
4444
"no-sparse-arrays": 0,
4545
"no-unexpected-multiline": 0,
4646
"object-shorthand": ["warn", "always"],
47-
"prettier/prettier": ["warn"],
48-
"sort-imports": ["warn", {"ignoreDeclarationSort": true}]
47+
"prettier/prettier": ["warn", {"embeddedLanguageFormatting": "off"}],
48+
"sort-imports": ["warn", {"ignoreDeclarationSort": true}],
49+
"quotes": ["warn", "double", {"avoidEscape": true}]
4950
},
5051
"overrides": [
5152
{

.prettierrc

+2-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
{
22
"bracketSpacing": false,
33
"printWidth": 120,
4-
"trailingComma": "none"
4+
"trailingComma": "none",
5+
"embeddedLanguageFormatting": "off"
56
}

public/client.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,7 @@ for (const summary of document.querySelectorAll("#observablehq-sidebar summary")
337337
}
338338

339339
const copyButton = document.createElement("template");
340-
copyButton.innerHTML = `<button title="Copy code" class="observablehq-pre-copy"><svg width="16" height="16" viewBox="0 0 16 16" fill="none" stroke="currentColor" stroke-width="2"><path d="M2 6C2 5.44772 2.44772 5 3 5H10C10.5523 5 11 5.44772 11 6V13C11 13.5523 10.5523 14 10 14H3C2.44772 14 2 13.5523 2 13V6Z M4 2.00004L12 2.00001C13.1046 2 14 2.89544 14 4.00001V12"></path></svg></button>`;
340+
copyButton.innerHTML = '<button title="Copy code" class="observablehq-pre-copy"><svg width="16" height="16" viewBox="0 0 16 16" fill="none" stroke="currentColor" stroke-width="2"><path d="M2 6C2 5.44772 2.44772 5 3 5H10C10.5523 5 11 5.44772 11 6V13C11 13.5523 10.5523 14 10 14H3C2.44772 14 2 13.5523 2 13V6Z M4 2.00004L12 2.00001C13.1046 2 14 2.89544 14 4.00001V12"></path></svg></button>'; // prettier-ignore
341341

342342
enableCopyButtons();
343343

src/auth.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ export async function login(effects = defaultEffects) {
5656
await effects.waitForEnter();
5757
await effects.openUrlInBrowser(url.toString());
5858
} else {
59-
effects.log(`Open this link in your browser to continue authentication:`);
59+
effects.log("Open this link in your browser to continue authentication:");
6060
effects.log(`\n\t${url.toString()}\n`);
6161
}
6262
return server; // for testing

src/build.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ export async function build(context: CommandContext = makeCommandContext()) {
109109
}
110110
}
111111

112-
const USAGE = `Usage: observable build [--root dir] [--output dir]`;
112+
const USAGE = "Usage: observable build [--root dir] [--output dir]";
113113

114114
function makeCommandContext(): CommandContext {
115115
const {values} = parseArgs({

src/html.ts

+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
import he from "he";
2+
3+
/**
4+
* Denotes a string that contains HTML source; when interpolated into an html
5+
* tagged template literal, it will not be escaped. Use Html.unsafe to denote
6+
* dynamic strings that are known to be safe.
7+
*/
8+
export class Html {
9+
private constructor(readonly html: string) {}
10+
static unsafe(html: string): Html {
11+
return new Html(html);
12+
}
13+
toString() {
14+
return this.html;
15+
}
16+
}
17+
18+
export function html(strings: TemplateStringsArray, ...values: unknown[]): Html {
19+
const parts: string[] = [];
20+
for (let i = 0; i < strings.length; ++i) {
21+
parts.push(strings[i]);
22+
if (i < values.length) {
23+
const value = values[i];
24+
if (value == null) continue;
25+
if (typeof value[Symbol.iterator] === "function") {
26+
for (const v of value as Iterable<unknown>) {
27+
if (v == null) continue;
28+
parts.push(v instanceof Html ? v.html : he.escape(String(v)));
29+
}
30+
} else {
31+
parts.push(value instanceof Html ? value.html : he.escape(String(value)));
32+
}
33+
}
34+
}
35+
return Html.unsafe(parts.join(""));
36+
}
37+
38+
html.unsafe = Html.unsafe;

src/preview.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ function handleWatch(socket: WebSocket, options: {root: string; resolver: CellRe
329329
}
330330
}
331331

332-
const USAGE = `Usage: observable preview [--root dir] [--hostname host] [--port port]`;
332+
const USAGE = "Usage: observable preview [--root dir] [--hostname host] [--port port]";
333333

334334
interface CommandContext {
335335
root: string;

src/render.ts

+67-94
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import {dirname, join} from "node:path";
22
import {parseHTML} from "linkedom";
33
import {type Config, type Page, type Section} from "./config.js";
44
import {computeHash} from "./hash.js";
5+
import {type Html, html} from "./html.js";
56
import {resolveImport} from "./javascript/imports.js";
67
import {type FileReference, type ImportReference} from "./javascript.js";
78
import {type CellPiece, type ParseResult, parseMarkdown} from "./markdown.js";
@@ -39,7 +40,7 @@ export function renderServerless(source: string, options: RenderOptions): Render
3940
};
4041
}
4142

42-
export function renderDefineCell(cell) {
43+
export function renderDefineCell(cell): string {
4344
const {id, inline, inputs, outputs, files, body, databases} = cell;
4445
return `define({${Object.entries({id, inline, inputs, outputs, files, databases})
4546
.filter((arg) => arg[1] !== undefined)
@@ -55,79 +56,71 @@ function render(
5556
parseResult: ParseResult,
5657
{path, pages, title, toc, preview, hash, resolver}: RenderOptions & RenderInternalOptions
5758
): string {
58-
const table = tableOfContents(parseResult, toc);
59-
return `<!DOCTYPE html>
60-
<meta charset="utf-8">${path === "/404" ? `\n<base href="/">` : ""}
59+
const pageTocConfig = parseResult.data?.toc;
60+
const tocLabel = pageTocConfig?.label ?? toc?.label;
61+
const tocHeaders = (pageTocConfig?.show ?? toc?.show) && findHeaders(parseResult);
62+
return String(html`<!DOCTYPE html>
63+
<meta charset="utf-8">${path === "/404" ? html`\n<base href="/">` : ""}
6164
<meta name="viewport" content="width=device-width, initial-scale=1, maximum-scale=1">
6265
${
6366
parseResult.title || title
64-
? `<title>${[parseResult.title, parseResult.title === title ? null : title]
67+
? html`<title>${[parseResult.title, parseResult.title === title ? null : title]
6568
.filter((title): title is string => !!title)
66-
.map((title) => escapeData(title))
6769
.join(" | ")}</title>\n`
6870
: ""
6971
}<link rel="preconnect" href="https://fonts.gstatic.com" crossorigin>
7072
<link rel="stylesheet" type="text/css" href="https://fonts.googleapis.com/css2?family=Source+Serif+Pro:ital,wght@0,400;0,600;0,700;1,400;1,600;1,700&display=swap">
71-
<link rel="stylesheet" type="text/css" href="${escapeDoubleQuoted(relativeUrl(path, "/_observablehq/style.css"))}">
72-
${Array.from(getImportPreloads(parseResult, path))
73-
.map((href) => `<link rel="modulepreload" href="${escapeDoubleQuoted(relativeUrl(path, href))}">`)
74-
.join("\n")}
75-
<script type="module">
73+
<link rel="stylesheet" type="text/css" href="${relativeUrl(path, "/_observablehq/style.css")}">${Array.from(
74+
getImportPreloads(parseResult, path),
75+
(href) => html`\n<link rel="modulepreload" href="${relativeUrl(path, href)}">`
76+
)}
77+
<script type="module">${html.unsafe(`
7678
7779
import {${preview ? "open, " : ""}define} from ${JSON.stringify(relativeUrl(path, "/_observablehq/client.js"))};
7880
7981
${preview ? `open({hash: ${JSON.stringify(hash)}, eval: (body) => (0, eval)(body)});\n` : ""}${parseResult.cells
8082
.map(resolver)
8183
.map(renderDefineCell)
82-
.join("")}
84+
.join("")}`)}
8385
</script>
8486
${pages.length > 0 ? sidebar(title, pages, path) : ""}
85-
${table}<div id="observablehq-center">
87+
${tocHeaders?.length > 0 ? tableOfContents(tocHeaders, tocLabel) : ""}<div id="observablehq-center">
8688
<main id="observablehq-main" class="observablehq">
87-
${parseResult.html}</main>
89+
${html.unsafe(parseResult.html)}</main>
8890
${footer(path, {pages, title})}
8991
</div>
90-
`;
92+
`);
9193
}
9294

93-
function sidebar(title: string | undefined, pages: (Page | Section)[], path: string): string {
94-
return `<input id="observablehq-sidebar-toggle" type="checkbox">
95+
function sidebar(title: string | undefined, pages: (Page | Section)[], path: string): Html {
96+
return html`<input id="observablehq-sidebar-toggle" type="checkbox">
9597
<nav id="observablehq-sidebar">
9698
<ol>
97-
<li class="observablehq-link${path === "/index" ? " observablehq-link-active" : ""}"><a href="${escapeDoubleQuoted(
98-
relativeUrl(path, "/")
99-
)}">${escapeData(title ?? "Home")}</a></li>
99+
<li class="observablehq-link${path === "/index" ? " observablehq-link-active" : ""}"><a href="${relativeUrl(
100+
path,
101+
"/"
102+
)}">${title ?? "Home"}</a></li>
100103
</ol>
101-
<ol>${pages
102-
.map((p, i) =>
103-
"pages" in p
104-
? `${i > 0 && "path" in pages[i - 1] ? "</ol>" : ""}
104+
<ol>${pages.map((p, i) =>
105+
"pages" in p
106+
? html`${i > 0 && "path" in pages[i - 1] ? html`</ol>` : ""}
105107
<details${p.open === undefined || p.open ? " open" : ""}>
106-
<summary>${escapeData(p.name)}</summary>
107-
<ol>${p.pages
108-
.map(
109-
(p) => `
110-
${renderListItem(p, path)}`
111-
)
112-
.join("")}
108+
<summary>${p.name}</summary>
109+
<ol>${p.pages.map((p) => renderListItem(p, path))}
113110
</ol>
114111
</details>`
115-
: "path" in p
116-
? `${
117-
i === 0
118-
? `
119-
`
120-
: !("path" in pages[i - 1])
121-
? `
112+
: "path" in p
113+
? html`${
114+
i === 0
115+
? ""
116+
: !("path" in pages[i - 1])
117+
? html`
122118
</ol>
123-
<ol>
124-
`
125-
: `
126-
`
127-
}${renderListItem(p, path)}`
128-
: null
129-
)
130-
.join("")}
119+
<ol>`
120+
: ""
121+
}${renderListItem(p, path)}`
122+
: ""
123+
)}
131124
</ol>
132125
</nav>
133126
<script>{
@@ -138,38 +131,34 @@ function sidebar(title: string | undefined, pages: (Page | Section)[], path: str
138131
}</script>`;
139132
}
140133

141-
function tableOfContents(parseResult: ParseResult, toc: RenderOptions["toc"]) {
142-
const pageTocConfig = parseResult.data?.toc;
143-
const headers =
144-
(pageTocConfig?.show ?? toc?.show) &&
145-
Array.from(parseHTML(parseResult.html).document.querySelectorAll("h2"))
146-
.map((node) => ({
147-
label: node.textContent,
148-
href: node.firstElementChild?.getAttribute("href")
149-
}))
150-
.filter((d) => d.label && d.href);
151-
return headers?.length
152-
? `<aside id="observablehq-toc">
134+
interface Header {
135+
label: string;
136+
href: string;
137+
}
138+
139+
function findHeaders(parseResult: ParseResult): Header[] {
140+
return Array.from(parseHTML(parseResult.html).document.querySelectorAll("h2"))
141+
.map((node) => ({label: node.textContent, href: node.firstElementChild?.getAttribute("href")}))
142+
.filter((d): d is Header => !!d.label && !!d.href);
143+
}
144+
145+
function tableOfContents(headers: Header[], label = "Contents"): Html {
146+
return html`<aside id="observablehq-toc">
153147
<nav>
154-
<div>${escapeData(pageTocConfig?.label ?? toc?.label ?? "Contents")}</div>
155-
<ol>
156-
${headers
157-
.map(
158-
({label, href}) =>
159-
`<li class="observablehq-secondary-link"><a href="${escapeDoubleQuoted(href)}">${escapeData(label)}</a></li>`
160-
)
161-
.join("\n")}
148+
<div>${label}</div>
149+
<ol>${headers.map(
150+
({label, href}) => html`\n<li class="observablehq-secondary-link"><a href="${href}">${label}</a></li>`
151+
)}
162152
</ol>
163153
</nav>
164154
</aside>
165-
`
166-
: "";
155+
`;
167156
}
168157

169-
function renderListItem(p: Page, path: string): string {
170-
return `<li class="observablehq-link${
158+
function renderListItem(p: Page, path: string): Html {
159+
return html`\n <li class="observablehq-link${
171160
p.path === path ? " observablehq-link-active" : ""
172-
}"><a href="${escapeDoubleQuoted(relativeUrl(path, prettyPath(p.path)))}">${escapeData(p.name)}</a></li>`;
161+
}"><a href="${relativeUrl(path, prettyPath(p.path))}">${p.name}</a></li>`;
173162
}
174163

175164
function prettyPath(path: string): string {
@@ -203,34 +192,18 @@ function getImportPreloads(parseResult: ParseResult, path: string): Iterable<str
203192
return preloads;
204193
}
205194

206-
// TODO Adopt Hypertext Literal?
207-
function escapeDoubleQuoted(value): string {
208-
return `${value}`.replace(/["&]/g, entity);
209-
}
210-
211-
// TODO Adopt Hypertext Literal?
212-
function escapeData(value: string): string {
213-
return `${value}`.replace(/[<&]/g, entity);
214-
}
215-
216-
function entity(character) {
217-
return `&#${character.charCodeAt(0).toString()};`;
218-
}
219-
220-
function footer(path: string, options?: Pick<Config, "pages" | "title">): string {
195+
function footer(path: string, options?: Pick<Config, "pages" | "title">): Html {
221196
const link = pager(path, options);
222-
return `<footer id="observablehq-footer">\n${
223-
link ? `${pagenav(path, link)}\n` : ""
197+
return html`<footer id="observablehq-footer">\n${
198+
link ? html`${pagenav(path, link)}\n` : ""
224199
}<div>© ${new Date().getUTCFullYear()} Observable, Inc.</div>
225200
</footer>`;
226201
}
227202

228-
function pagenav(path: string, {prev, next}: PageLink): string {
229-
return `<nav>${prev ? pagelink(path, prev, "prev") : ""}${next ? pagelink(path, next, "next") : ""}</nav>`;
203+
function pagenav(path: string, {prev, next}: PageLink): Html {
204+
return html`<nav>${prev ? pagelink(path, prev, "prev") : ""}${next ? pagelink(path, next, "next") : ""}</nav>`;
230205
}
231206

232-
function pagelink(path: string, page: Page, rel: "prev" | "next"): string {
233-
return `<a rel="${escapeDoubleQuoted(rel)}" href="${escapeDoubleQuoted(
234-
relativeUrl(path, prettyPath(page.path))
235-
)}"><span>${escapeData(page.name)}</span></a>`;
207+
function pagelink(path: string, page: Page, rel: "prev" | "next"): Html {
208+
return html`<a rel="${rel}" href="${relativeUrl(path, prettyPath(page.path))}"><span>${page.name}</span></a>`;
236209
}

test/html-test.ts

+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
import assert from "node:assert";
2+
import {html} from "../src/html.js";
3+
4+
describe("html(strings, ...values)", () => {
5+
it("returns an instance of Html", () => {
6+
assert.deepStrictEqual(html`<b>hello</b>`, html.unsafe("<b>hello</b>"));
7+
});
8+
it("stringifies to HTML source", () => {
9+
assert.deepStrictEqual(String(html`<b>hello</b>`), "<b>hello</b>");
10+
});
11+
it("escapes interpolated values, if not instanceof Html", () => {
12+
assert.deepStrictEqual(String(html`<b>${"dollar&pound"}</b>`), "<b>dollar&amp;pound</b>");
13+
assert.deepStrictEqual(String(html`<b>${"dollar"}${"&pound"}</b>`), "<b>dollar&amp;pound</b>");
14+
assert.deepStrictEqual(String(html`${"<script>"}alert()${"</script>"}`), "&lt;script&gt;alert()&lt;/script&gt;");
15+
assert.deepStrictEqual(String(html`<span name="${"'a'"}">a</span>`), '<span name="&#x27;a&#x27;">a</span>');
16+
assert.deepStrictEqual(String(html`<span name="${'"b"'}">b</span>`), '<span name="&quot;b&quot;">b</span>');
17+
assert.deepStrictEqual(String(html`<span name="${"`c`"}">c</span>`), '<span name="&#x60;c&#x60;">c</span>');
18+
});
19+
it("is not raw", () => {
20+
assert.deepStrictEqual(String(html`<b>\new{42}</b>`), "<b>\new{42}</b>");
21+
});
22+
it("coerces interpolated values to strings, if not instanceof Html", () => {
23+
assert.deepStrictEqual(String(html`<b>${{toString: () => 42}}</b>`), "<b>42</b>");
24+
});
25+
it("concatenates iterables", () => {
26+
assert.deepStrictEqual(String(html`<b>${[1, 2, 3]}</b>`), "<b>123</b>");
27+
assert.deepStrictEqual(String(html`<b>${new Set([1, 2, 3])}</b>`), "<b>123</b>");
28+
assert.deepStrictEqual(String(html`<b>${["dollar", "&pound"]}</b>`), "<b>dollar&amp;pound</b>");
29+
});
30+
it("ignores null and undefined", () => {
31+
assert.deepStrictEqual(String(html`<b>${null}</b>`), "<b></b>");
32+
assert.deepStrictEqual(String(html`<b>${undefined}</b>`), "<b></b>");
33+
});
34+
it("does not escape interpolated values if instanceof Html", () => {
35+
assert.deepStrictEqual(String(html`<b>${html`dollar&pound`}</b>`), "<b>dollar&pound</b>");
36+
assert.deepStrictEqual(String(html`<b>${html.unsafe("dollar&pound")}</b>`), "<b>dollar&pound</b>");
37+
});
38+
});

test/output/build/config/index.html

+2-2
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
</ol>
2020
<ol>
2121
<li class="observablehq-link observablehq-link-active"><a href="./">Index</a></li>
22-
<li class="observablehq-link"><a href="./one">One&#60;Two</a></li>
22+
<li class="observablehq-link"><a href="./one">One&lt;Two</a></li>
2323
<li class="observablehq-link"><a href="./sub/two">Two</a></li>
2424
</ol>
2525
</nav>
@@ -38,7 +38,7 @@ <h1 id="index" tabindex="-1"><a class="observablehq-header-anchor" href="#index"
3838
</ul>
3939
</main>
4040
<footer id="observablehq-footer">
41-
<nav><a rel="next" href="./one"><span>One&#60;Two</span></a></nav>
41+
<nav><a rel="next" href="./one"><span>One&lt;Two</span></a></nav>
4242
<div>© 2023 Observable, Inc.</div>
4343
</footer>
4444
</div>

test/output/build/config/one.html

+1-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
</ol>
2020
<ol>
2121
<li class="observablehq-link"><a href="./">Index</a></li>
22-
<li class="observablehq-link observablehq-link-active"><a href="./one">One&#60;Two</a></li>
22+
<li class="observablehq-link observablehq-link-active"><a href="./one">One&lt;Two</a></li>
2323
<li class="observablehq-link"><a href="./sub/two">Two</a></li>
2424
</ol>
2525
</nav>

0 commit comments

Comments
 (0)