Deprecate unescaped HTML inside of expressions (#2489)

* feat: implement automatic escaping

* feat: deprecate automatic escaping

* fix: cast unescapeHTML as string

* fix: slot fallback behavior

* fix: unescaped content

* Update escape.ts

* Update escape.ts

* feat: update internal components to use `set:html`

* chore: update compiler

* chore: update changeset

.changeset/chatty-spies-wink.md

@@ -0,0 +1,8 @@
+---
+'astro': minor
+---
+
+Add support for the `set:html` and `set:text` directives. 
+
+With the introduction of these directives, unescaped HTML content in expressions is now deprecated. Please migrate to `set:html` in order to continue injecting unescaped HTML in future versions of Astro—you can use `<Fragment set:html={content}>` to avoid a wrapper element. `set:text` allows you to opt-in to escaping now, but it will soon become the default.
+

.changeset/mighty-lamps-drive.md

@@ -2,4 +2,4 @@
 'astro': patch
 ---
 
-Bug fix for define:vars with the --experimental-static-build flag
+Bug fix for `define:vars` with the --experimental-static-build flag

packages/astro/components/Code.astro

@@ -47,4 +47,4 @@ const _html = highlighter.codeToHtml(code, lang);
 const html = repairShikiTheme(_html);
 ---
 
-{html}
+<Fragment set:html={html} />

packages/astro/components/Markdown.astro

@@ -41,4 +41,4 @@ if (content) {
 html = htmlContent;
 ---
 
-{html ? html : <slot />}
+{html ? <Fragment set:html={html} /> : <slot />}

packages/astro/components/Prism.astro

@@ -46,4 +46,4 @@ if (grammar) {
 }
 ---
 
-<pre class={[className, classLanguage].join(' ')}><code class={classLanguage}>{html}</code></pre>
+<pre class={[className, classLanguage].join(' ')}><code class={classLanguage}><Fragment set:html={html} /></code></pre>

packages/astro/package.json

@@ -55,7 +55,7 @@
     "test:match": "mocha --timeout 15000 -g"
   },
   "dependencies": {
-    "@astrojs/compiler": "^0.10.0",
+    "@astrojs/compiler": "^0.10.1",
     "@astrojs/language-server": "^0.8.6",
     "@astrojs/markdown-remark": "^0.6.0",
     "@astrojs/prism": "0.4.0",

packages/astro/src/runtime/server/escape.ts

@@ -1,3 +1,34 @@
 const entities = { '"': 'quot', '&': 'amp', "'": 'apos', '<': 'lt', '>': 'gt' } as const;
 
-export const escapeHTML = (string: any) => string.replace(/["'&<>]/g, (char: keyof typeof entities) => '&' + entities[char] + ';');
+const warned = new Set<string>();
+export const escapeHTML = (string: any, { deprecated = false }: { deprecated?: boolean } = {}) => {
+	const escaped = string.replace(/["'&<>]/g, (char: keyof typeof entities) => '&' + entities[char] + ';');
+	if (!deprecated) return escaped;
+	if (warned.has(string) || !string.match(/[&<>]/g)) return string;
+	// eslint-disable-next-line no-console
+	console.warn(`Unescaped HTML content found inside expression!
+
+The next minor version of Astro will automatically escape all
+expression content. Please use the \`set:html\` directive.
+
+Expression content:
+${string}`);
+	warned.add(string);
+
+	// Return unescaped content for now. To be removed.
+	return string;
+}
+
+/**
+ * RawString is a "blessed" version of String
+ * that is not subject to escaping.
+ */
+export class UnescapedString extends String {}
+
+/** 
+ * unescapeHTML marks a string as raw, unescaped HTML. 
+ * This should only be generated internally, not a public API.
+ *
+ * Need to cast the return value `as unknown as string` so TS doesn't yell at us.
+ */
+export const unescapeHTML = (str: any) => new UnescapedString(str) as unknown as string;

packages/astro/src/runtime/server/index.ts

@@ -4,9 +4,11 @@ import type { AstroGlobalPartial, SSRResult, SSRElement } from '../../@types/ast
 import shorthash from 'shorthash';
 import { extractDirectives, generateHydrateScript } from './hydration.js';
 import { serializeListValue } from './util.js';
-export { createMetadata } from './metadata.js';
+import { escapeHTML, UnescapedString, unescapeHTML } from './escape.js';
-export { escapeHTML } from './escape.js';
+
 export type { Metadata } from './metadata';
+export { createMetadata } from './metadata.js';
+export { escapeHTML, unescapeHTML } from './escape.js';
 
 const voidElementNames = /^(area|base|br|col|command|embed|hr|img|input|keygen|link|meta|param|source|track|wbr)$/i;
 
@@ -19,22 +21,24 @@ const voidElementNames = /^(area|base|br|col|command|embed|hr|img|input|keygen|l
 // Or maybe type UserValue = any; ?
 async function _render(child: any): Promise<any> {
 	child = await child;
-	if (Array.isArray(child)) {
+	if (child instanceof UnescapedString) {
-		return (await Promise.all(child.map((value) => _render(value)))).join('');
+		return child;
+	} else if (Array.isArray(child)) {
+		return unescapeHTML((await Promise.all(child.map((value) => _render(value)))).join(''));
 	} else if (typeof child === 'function') {
 		// Special: If a child is a function, call it automatically.
 		// This lets you do {() => ...} without the extra boilerplate
 		// of wrapping it in a function and calling it.
 		return _render(child());
 	} else if (typeof child === 'string') {
-		return child;
+		return escapeHTML(child, { deprecated: true });
 	} else if (!child && child !== 0) {
 		// do nothing, safe to ignore falsey values.
 	}
 	// Add a comment explaining why each of these are needed.
 	// Maybe create clearly named function for what this is doing.
 	else if (child instanceof AstroComponent || Object.prototype.toString.call(child) === '[object AstroComponent]') {
-		return await renderAstroComponent(child);
+		return unescapeHTML(await renderAstroComponent(child));
 	} else {
 		return child;
 	}
@@ -62,7 +66,7 @@ export class AstroComponent {
 			const html = htmlParts[i];
 			const expression = expressions[i];
 
-			yield _render(html);
+			yield _render(unescapeHTML(html));
 			yield _render(expression);
 		}
 	}
@@ -88,7 +92,7 @@ export function createComponent(cb: AstroComponentFactory) {
 
 export async function renderSlot(_result: any, slotted: string, fallback?: any) {
 	if (slotted) {
-		return _render(slotted);
+		return await _render(slotted);
 	}
 	return fallback;
 }
@@ -122,12 +126,12 @@ export async function renderComponent(result: SSRResult, displayName: string, Co
 	const children = await renderSlot(result, slots?.default);
 
 	if (Component === Fragment) {
-		return children;
+		return unescapeHTML(children);
 	}
 
 	if (Component && (Component as any).isAstroComponentFactory) {
 		const output = await renderToString(result, Component as any, _props, slots);
-		return output;
+		return unescapeHTML(output);
 	}
 
 	if (Component === null && !_props['client:only']) {
@@ -233,7 +237,7 @@ If you're still stuck, please open an issue on GitHub or join us at https://astr
 	// as a string and the user is responsible for adding a script tag for the component definition.
 	if (!html && typeof Component === 'string') {
 		html = await renderAstroComponent(
-			await render`<${Component}${spreadAttributes(props)}${(children == null || children == '') && voidElementNames.test(Component) ? `/>` : `>${children}</${Component}>`}`
+			await render`<${Component}${spreadAttributes(props)}${unescapeHTML((children == null || children == '') && voidElementNames.test(Component) ? `/>` : `>${children}</${Component}>`)}`
 		);
 	}
 
@@ -248,7 +252,7 @@ If you're still stuck, please open an issue on GitHub or join us at https://astr
 	}
 
 	if (!hydration) {
-		return html.replace(/\<\/?astro-fragment\>/g, '');
+		return unescapeHTML(html.replace(/\<\/?astro-fragment\>/g, ''));
 	}
 
 	// Include componentExport name and componentUrl in hash to dedupe identical islands
@@ -258,7 +262,7 @@ If you're still stuck, please open an issue on GitHub or join us at https://astr
 	// INVESTIGATE: This will likely be a problem in streaming because the `<head>` will be gone at this point.
 	result.scripts.add(await generateHydrateScript({ renderer, result, astroId, props }, metadata as Required<AstroComponentMetadata>));
 
-	return `<astro-root uid="${astroId}">${html ?? ''}</astro-root>`;
+	return unescapeHTML(`<astro-root uid="${astroId}">${html ?? ''}</astro-root>`);
 }
 
 /** Create the Astro.fetchContent() runtime function. */
@@ -336,14 +340,14 @@ Make sure to use the static attribute syntax (\`${key}={value}\`) instead of the
 
 	// support "class" from an expression passed into an element (#782)
 	if (key === 'class:list') {
-		return ` ${key.slice(0, -5)}="${toAttributeString(serializeListValue(value))}"`;
+		return unescapeHTML(` ${key.slice(0, -5)}="${toAttributeString(serializeListValue(value))}"`);
 	}
 
 	// Boolean only needs the key
 	if (value === true && key.startsWith('data-')) {
-		return ` ${key}`;
+		return unescapeHTML(` ${key}`);
 	} else {
-		return ` ${key}="${toAttributeString(value)}"`;
+		return unescapeHTML(` ${key}="${toAttributeString(value)}"`);
 	}
 }
 
@@ -353,7 +357,7 @@ export function spreadAttributes(values: Record<any, any>) {
 	for (const [key, value] of Object.entries(values)) {
 		output += addAttribute(value, key);
 	}
-	return output;
+	return unescapeHTML(output);
 }
 
 // Adds CSS variables to an inline style tag
@@ -378,7 +382,7 @@ export function defineScriptVars(vars: Record<any, any>) {
 export async function renderToString(result: SSRResult, componentFactory: AstroComponentFactory, props: any, children: any) {
 	const Component = await componentFactory(result, props, children);
 	let template = await renderAstroComponent(Component);
-	return template;
+	return unescapeHTML(template);
 }
 
 // Filter out duplicate elements in our set
@@ -431,15 +435,15 @@ export async function renderPage(result: SSRResult, Component: AstroComponentFac
 }
 
 export async function renderAstroComponent(component: InstanceType<typeof AstroComponent>) {
-	let template = '';
+	let template = [];
 
 	for await (const value of component) {
 		if (value || value === 0) {
-			template += value;
+			template.push(value);
 		}
 	}
 
-	return template;
+	return unescapeHTML(await _render(template));
 }
 
 export async function renderHTMLElement(result: SSRResult, constructor: typeof HTMLElement, props: any, slots: any) {

yarn.lock

@@ -130,10 +130,10 @@
     jsonpointer "^5.0.0"
     leven "^3.1.0"
 
-"@astrojs/compiler@^0.10.0":
+"@astrojs/compiler@^0.10.1":
-  version "0.10.0"
+  version "0.10.1"
-  resolved "https://registry.yarnpkg.com/@astrojs/compiler/-/compiler-0.10.0.tgz#6ba2707bf9a91017fd72fb46b1d7865036b71c09"
+  resolved "https://registry.yarnpkg.com/@astrojs/compiler/-/compiler-0.10.1.tgz#69df1a7e4150c1b0b255154ae716dbc8b24c5dd4"
-  integrity sha512-TbeuITyhRGlQowipNX7Q8o5QczVjSxlE68xKh7i1swTxUhM8K/cnCPSyzTsWiuFWY4C5PDI1GREUaD3BHYfzqQ==
+  integrity sha512-SUp5auq6jcLmxyOx8Ovd3ebvwR5wnuqsbIi77Ze/ua3+GMcR++rOWiyqyql887U2ajZMwJfHatOwQl67P7o5gg==
   dependencies:
     typescript "^4.3.5"