From 99e1665ce18410c4a3266ca6cd489a53b1d7fcf3 Mon Sep 17 00:00:00 2001
From: Dominic Gannaway <trueadm@users.noreply.github.com>
Date: Wed, 28 Feb 2024 09:16:31 +0000
Subject: [PATCH] feat: improve ssr html mismatch validation (#10658)

* feat: improve ssr html mismatch validation

* update types

* Update packages/svelte/src/internal/server/index.js

Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>

* Update packages/svelte/src/compiler/validate-options.js

Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>

* feedback

---------

Co-authored-by: Simon H <5968653+dummdidumm@users.noreply.github.com>
---
 .changeset/hungry-singers-share.md            |   5 +
 .../src/compiler/phases/1-parse/utils/html.js |  41 +----
 .../compiler/phases/2-analyze/validation.js   | 127 +-------------
 .../3-transform/server/transform-server.js    |  18 ++
 packages/svelte/src/constants.js              | 161 ++++++++++++++++++
 packages/svelte/src/internal/server/index.js  |  71 +++++++-
 packages/svelte/src/legacy/legacy-client.js   |   2 +-
 .../svelte/tests/runtime-legacy/shared.ts     |   3 +-
 .../samples/invalid-html-ssr/Component.svelte |   1 +
 .../samples/invalid-html-ssr/_config.js       |  41 +++++
 .../samples/invalid-html-ssr/main.svelte      |   7 +
 packages/svelte/types/index.d.ts              |   2 +-
 12 files changed, 313 insertions(+), 166 deletions(-)
 create mode 100644 .changeset/hungry-singers-share.md
 create mode 100644 packages/svelte/tests/runtime-runes/samples/invalid-html-ssr/Component.svelte
 create mode 100644 packages/svelte/tests/runtime-runes/samples/invalid-html-ssr/_config.js
 create mode 100644 packages/svelte/tests/runtime-runes/samples/invalid-html-ssr/main.svelte

diff --git a/.changeset/hungry-singers-share.md b/.changeset/hungry-singers-share.md
new file mode 100644
index 000000000..e80414411
--- /dev/null
+++ b/.changeset/hungry-singers-share.md
@@ -0,0 +1,5 @@
+---
+"svelte": patch
+---
+
+feat: improve ssr html mismatch validation
diff --git a/packages/svelte/src/compiler/phases/1-parse/utils/html.js b/packages/svelte/src/compiler/phases/1-parse/utils/html.js
index 1c80aa355..3cc9a5a20 100644
--- a/packages/svelte/src/compiler/phases/1-parse/utils/html.js
+++ b/packages/svelte/src/compiler/phases/1-parse/utils/html.js
@@ -1,3 +1,4 @@
+import { interactive_elements } from '../../../../constants.js';
 import entities from './entities.js';
 
 const windows_1252 = [
@@ -121,16 +122,6 @@ function validate_code(code) {
 
 // based on http://developers.whatwg.org/syntax.html#syntax-tag-omission
 
-// while `input` is also an interactive element, it is never moved by the browser, so we don't need to check for it
-export const interactive_elements = new Set([
-	'a',
-	'button',
-	'iframe',
-	'embed',
-	'select',
-	'textarea'
-]);
-
 /** @type {Record<string, Set<string>>} */
 const disallowed_contents = {
 	li: new Set(['li']),
@@ -153,36 +144,6 @@ const disallowed_contents = {
 	th: new Set(['td', 'th', 'tr'])
 };
 
-export const disallowed_parapgraph_contents = [
-	'address',
-	'article',
-	'aside',
-	'blockquote',
-	'details',
-	'div',
-	'dl',
-	'fieldset',
-	'figcapture',
-	'figure',
-	'footer',
-	'form',
-	'h1',
-	'h2',
-	'h3',
-	'h4',
-	'h5',
-	'h6',
-	'header',
-	'hr',
-	'menu',
-	'nav',
-	'ol',
-	'pre',
-	'section',
-	'table',
-	'ul'
-];
-
 for (const interactive_element of interactive_elements) {
 	disallowed_contents[interactive_element] = interactive_elements;
 }
diff --git a/packages/svelte/src/compiler/phases/2-analyze/validation.js b/packages/svelte/src/compiler/phases/2-analyze/validation.js
index 1790e073e..55d523ffd 100644
--- a/packages/svelte/src/compiler/phases/2-analyze/validation.js
+++ b/packages/svelte/src/compiler/phases/2-analyze/validation.js
@@ -1,3 +1,8 @@
+import {
+	disallowed_parapgraph_contents,
+	interactive_elements,
+	is_tag_valid_with_parent
+} from '../../../constants.js';
 import { error } from '../../errors.js';
 import {
 	extract_identifiers,
@@ -8,7 +13,6 @@ import {
 } from '../../utils/ast.js';
 import { warn } from '../../warnings.js';
 import fuzzymatch from '../1-parse/utils/fuzzymatch.js';
-import { disallowed_parapgraph_contents, interactive_elements } from '../1-parse/utils/html.js';
 import { binding_properties } from '../bindings.js';
 import { ContentEditableBindings, EventModifiers, SVGElements } from '../constants.js';
 import { is_custom_element_node } from '../nodes.js';
@@ -226,127 +230,6 @@ function validate_slot_attribute(context, attribute) {
 	}
 }
 
-// https://html.spec.whatwg.org/multipage/syntax.html#generate-implied-end-tags
-const implied_end_tags = ['dd', 'dt', 'li', 'option', 'optgroup', 'p', 'rp', 'rt'];
-
-/**
- * @param {string} tag
- * @param {string} parent_tag
- * @returns {boolean}
- */
-function is_tag_valid_with_parent(tag, parent_tag) {
-	// First, let's check if we're in an unusual parsing mode...
-	switch (parent_tag) {
-		// https://html.spec.whatwg.org/multipage/syntax.html#parsing-main-inselect
-		case 'select':
-			return tag === 'option' || tag === 'optgroup' || tag === '#text';
-		case 'optgroup':
-			return tag === 'option' || tag === '#text';
-		// Strictly speaking, seeing an <option> doesn't mean we're in a <select>
-		// but
-		case 'option':
-			return tag === '#text';
-		// https://html.spec.whatwg.org/multipage/syntax.html#parsing-main-intd
-		// https://html.spec.whatwg.org/multipage/syntax.html#parsing-main-incaption
-		// No special behavior since these rules fall back to "in body" mode for
-		// all except special table nodes which cause bad parsing behavior anyway.
-
-		// https://html.spec.whatwg.org/multipage/syntax.html#parsing-main-intr
-		case 'tr':
-			return (
-				tag === 'th' || tag === 'td' || tag === 'style' || tag === 'script' || tag === 'template'
-			);
-		// https://html.spec.whatwg.org/multipage/syntax.html#parsing-main-intbody
-		case 'tbody':
-		case 'thead':
-		case 'tfoot':
-			return tag === 'tr' || tag === 'style' || tag === 'script' || tag === 'template';
-		// https://html.spec.whatwg.org/multipage/syntax.html#parsing-main-incolgroup
-		case 'colgroup':
-			return tag === 'col' || tag === 'template';
-		// https://html.spec.whatwg.org/multipage/syntax.html#parsing-main-intable
-		case 'table':
-			return (
-				tag === 'caption' ||
-				tag === 'colgroup' ||
-				tag === 'tbody' ||
-				tag === 'tfoot' ||
-				tag === 'thead' ||
-				tag === 'style' ||
-				tag === 'script' ||
-				tag === 'template'
-			);
-		// https://html.spec.whatwg.org/multipage/syntax.html#parsing-main-inhead
-		case 'head':
-			return (
-				tag === 'base' ||
-				tag === 'basefont' ||
-				tag === 'bgsound' ||
-				tag === 'link' ||
-				tag === 'meta' ||
-				tag === 'title' ||
-				tag === 'noscript' ||
-				tag === 'noframes' ||
-				tag === 'style' ||
-				tag === 'script' ||
-				tag === 'template'
-			);
-		// https://html.spec.whatwg.org/multipage/semantics.html#the-html-element
-		case 'html':
-			return tag === 'head' || tag === 'body' || tag === 'frameset';
-		case 'frameset':
-			return tag === 'frame';
-		case '#document':
-			return tag === 'html';
-	}
-
-	// Probably in the "in body" parsing mode, so we outlaw only tag combos
-	// where the parsing rules cause implicit opens or closes to be added.
-	// https://html.spec.whatwg.org/multipage/syntax.html#parsing-main-inbody
-	switch (tag) {
-		case 'h1':
-		case 'h2':
-		case 'h3':
-		case 'h4':
-		case 'h5':
-		case 'h6':
-			return (
-				parent_tag !== 'h1' &&
-				parent_tag !== 'h2' &&
-				parent_tag !== 'h3' &&
-				parent_tag !== 'h4' &&
-				parent_tag !== 'h5' &&
-				parent_tag !== 'h6'
-			);
-
-		case 'rp':
-		case 'rt':
-			return implied_end_tags.indexOf(parent_tag) === -1;
-
-		case 'body':
-		case 'caption':
-		case 'col':
-		case 'colgroup':
-		case 'frameset':
-		case 'frame':
-		case 'head':
-		case 'html':
-		case 'tbody':
-		case 'td':
-		case 'tfoot':
-		case 'th':
-		case 'thead':
-		case 'tr':
-			// These tags are only valid with a few parents that have special child
-			// parsing rules -- if we're down here, then none of those matched and
-			// so we allow it only if we don't know what the parent is, as all other
-			// cases are invalid.
-			return parent_tag == null;
-	}
-
-	return true;
-}
-
 /**
  * @type {import('zimmerframe').Visitors<import('#compiler').SvelteNode, import('./types.js').AnalysisState>}
  */
diff --git a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js
index 26cccdd99..648abf21f 100644
--- a/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js
+++ b/packages/svelte/src/compiler/phases/3-transform/server/transform-server.js
@@ -1207,6 +1207,12 @@ const template_visitors = {
 			inner_context.visit(node, state);
 		}
 
+		if (context.state.options.dev) {
+			context.state.template.push(
+				t_statement(b.stmt(b.call('$.push_element', b.literal(node.name), b.id('$$payload'))))
+			);
+		}
+
 		process_children(trimmed, node, inner_context);
 
 		if (body_expression !== null) {
@@ -1239,6 +1245,9 @@ const template_visitors = {
 		if (!VoidElements.includes(node.name) && metadata.namespace !== 'foreign') {
 			context.state.template.push(t_string(`</${node.name}>`));
 		}
+		if (context.state.options.dev) {
+			context.state.template.push(t_statement(b.stmt(b.call('$.pop_element'))));
+		}
 	},
 	SvelteElement(node, context) {
 		let tag = /** @type {import('estree').Expression} */ (context.visit(node.tag));
@@ -1281,6 +1290,12 @@ const template_visitors = {
 
 		serialize_element_attributes(node, inner_context);
 
+		if (context.state.options.dev) {
+			context.state.template.push(
+				t_statement(b.stmt(b.call('$.push_element', tag, b.id('$$payload'))))
+			);
+		}
+
 		context.state.template.push(
 			t_statement(
 				b.if(
@@ -1304,6 +1319,9 @@ const template_visitors = {
 			),
 			t_expression(anchor_id)
 		);
+		if (context.state.options.dev) {
+			context.state.template.push(t_statement(b.stmt(b.call('$.pop_element'))));
+		}
 	},
 	EachBlock(node, context) {
 		const state = context.state;
diff --git a/packages/svelte/src/constants.js b/packages/svelte/src/constants.js
index faacabcb9..a9b80ea84 100644
--- a/packages/svelte/src/constants.js
+++ b/packages/svelte/src/constants.js
@@ -88,3 +88,164 @@ export const DOMBooleanAttributes = [
 
 export const namespace_svg = 'http://www.w3.org/2000/svg';
 export const namespace_html = 'http://www.w3.org/1999/xhtml';
+
+// while `input` is also an interactive element, it is never moved by the browser, so we don't need to check for it
+export const interactive_elements = new Set([
+	'a',
+	'button',
+	'iframe',
+	'embed',
+	'select',
+	'textarea'
+]);
+
+export const disallowed_parapgraph_contents = [
+	'address',
+	'article',
+	'aside',
+	'blockquote',
+	'details',
+	'div',
+	'dl',
+	'fieldset',
+	'figcapture',
+	'figure',
+	'footer',
+	'form',
+	'h1',
+	'h2',
+	'h3',
+	'h4',
+	'h5',
+	'h6',
+	'header',
+	'hr',
+	'menu',
+	'nav',
+	'ol',
+	'pre',
+	'section',
+	'table',
+	'ul'
+];
+
+// https://html.spec.whatwg.org/multipage/syntax.html#generate-implied-end-tags
+const implied_end_tags = ['dd', 'dt', 'li', 'option', 'optgroup', 'p', 'rp', 'rt'];
+
+/**
+ * @param {string} tag
+ * @param {string} parent_tag
+ * @returns {boolean}
+ */
+export function is_tag_valid_with_parent(tag, parent_tag) {
+	// First, let's check if we're in an unusual parsing mode...
+	switch (parent_tag) {
+		// https://html.spec.whatwg.org/multipage/syntax.html#parsing-main-inselect
+		case 'select':
+			return tag === 'option' || tag === 'optgroup' || tag === '#text';
+		case 'optgroup':
+			return tag === 'option' || tag === '#text';
+		// Strictly speaking, seeing an <option> doesn't mean we're in a <select>
+		// but
+		case 'option':
+			return tag === '#text';
+		// https://html.spec.whatwg.org/multipage/syntax.html#parsing-main-intd
+		// https://html.spec.whatwg.org/multipage/syntax.html#parsing-main-incaption
+		// No special behavior since these rules fall back to "in body" mode for
+		// all except special table nodes which cause bad parsing behavior anyway.
+
+		// https://html.spec.whatwg.org/multipage/syntax.html#parsing-main-intr
+		case 'tr':
+			return (
+				tag === 'th' || tag === 'td' || tag === 'style' || tag === 'script' || tag === 'template'
+			);
+		// https://html.spec.whatwg.org/multipage/syntax.html#parsing-main-intbody
+		case 'tbody':
+		case 'thead':
+		case 'tfoot':
+			return tag === 'tr' || tag === 'style' || tag === 'script' || tag === 'template';
+		// https://html.spec.whatwg.org/multipage/syntax.html#parsing-main-incolgroup
+		case 'colgroup':
+			return tag === 'col' || tag === 'template';
+		// https://html.spec.whatwg.org/multipage/syntax.html#parsing-main-intable
+		case 'table':
+			return (
+				tag === 'caption' ||
+				tag === 'colgroup' ||
+				tag === 'tbody' ||
+				tag === 'tfoot' ||
+				tag === 'thead' ||
+				tag === 'style' ||
+				tag === 'script' ||
+				tag === 'template'
+			);
+		// https://html.spec.whatwg.org/multipage/syntax.html#parsing-main-inhead
+		case 'head':
+			return (
+				tag === 'base' ||
+				tag === 'basefont' ||
+				tag === 'bgsound' ||
+				tag === 'link' ||
+				tag === 'meta' ||
+				tag === 'title' ||
+				tag === 'noscript' ||
+				tag === 'noframes' ||
+				tag === 'style' ||
+				tag === 'script' ||
+				tag === 'template'
+			);
+		// https://html.spec.whatwg.org/multipage/semantics.html#the-html-element
+		case 'html':
+			return tag === 'head' || tag === 'body' || tag === 'frameset';
+		case 'frameset':
+			return tag === 'frame';
+		case '#document':
+			return tag === 'html';
+	}
+
+	// Probably in the "in body" parsing mode, so we outlaw only tag combos
+	// where the parsing rules cause implicit opens or closes to be added.
+	// https://html.spec.whatwg.org/multipage/syntax.html#parsing-main-inbody
+	switch (tag) {
+		case 'h1':
+		case 'h2':
+		case 'h3':
+		case 'h4':
+		case 'h5':
+		case 'h6':
+			return (
+				parent_tag !== 'h1' &&
+				parent_tag !== 'h2' &&
+				parent_tag !== 'h3' &&
+				parent_tag !== 'h4' &&
+				parent_tag !== 'h5' &&
+				parent_tag !== 'h6'
+			);
+
+		case 'rp':
+		case 'rt':
+			return implied_end_tags.indexOf(parent_tag) === -1;
+
+		case 'body':
+		case 'caption':
+		case 'col':
+		case 'colgroup':
+		case 'frameset':
+		case 'frame':
+		case 'head':
+		case 'html':
+		case 'tbody':
+		case 'td':
+		case 'tfoot':
+		case 'th':
+		case 'thead':
+		case 'tr':
+			// These tags are only valid with a few parents that have special child
+			// parsing rules -- if we're down here, then none of those matched and
+			// so we allow it only if we don't know what the parent is, as all other
+			// cases are invalid.
+			return parent_tag == null;
+	}
+
+	return true;
+}
diff --git a/packages/svelte/src/internal/server/index.js b/packages/svelte/src/internal/server/index.js
index 621d05248..7ae14d16a 100644
--- a/packages/svelte/src/internal/server/index.js
+++ b/packages/svelte/src/internal/server/index.js
@@ -1,11 +1,23 @@
 import * as $ from '../client/runtime.js';
 import { is_promise, noop } from '../common.js';
 import { subscribe_to_store } from '../../store/utils.js';
-import { DOMBooleanAttributes } from '../../constants.js';
+import {
+	DOMBooleanAttributes,
+	disallowed_parapgraph_contents,
+	interactive_elements,
+	is_tag_valid_with_parent
+} from '../../constants.js';
 import { DEV } from 'esm-env';
 
 export * from '../client/validate.js';
 
+/**
+ * @typedef {{
+ * 	tag: string;
+ * 	parent: null | Element;
+ * }} Element
+ */
+
 /**
  * @typedef {{
  * 	head: string;
@@ -51,6 +63,11 @@ export const VoidElements = new Set([
 	'wbr'
 ]);
 
+/**
+ * @type {Element | null}
+ */
+let current_element = null;
+
 /** @returns {Payload} */
 function create_payload() {
 	return { out: '', head: { title: '', out: '', anchor: 0 }, anchor: 0 };
@@ -79,6 +96,58 @@ export function assign_payload(p1, p2) {
 	p1.anchor = p2.anchor;
 }
 
+/**
+ * @param {Payload} payload
+ * @param {string} message
+ */
+function error_on_client(payload, message) {
+	message =
+		`Svelte SSR validation error:\n\n${message}\n\n` +
+		'Ensure your components render valid HTML as the browser will try to repair invalid HTML, ' +
+		'which may result in content being shifted around and will likely result in a hydration mismatch.';
+	// eslint-disable-next-line no-console
+	console.error(message);
+	payload.head.out += `<script>console.error(${message})</script>`;
+}
+
+/**
+ * @param {string} tag
+ * @param {Payload} payload
+ */
+export function push_element(tag, payload) {
+	if (current_element !== null && !is_tag_valid_with_parent(tag, current_element.tag)) {
+		error_on_client(payload, `<${tag}> is invalid inside <${current_element.tag}>`);
+	}
+	if (interactive_elements.has(tag)) {
+		let element = current_element;
+		while (element !== null) {
+			if (interactive_elements.has(element.tag)) {
+				error_on_client(payload, `<${tag}> is invalid inside <${element.tag}>`);
+			}
+			element = element.parent;
+		}
+	}
+	if (disallowed_parapgraph_contents.includes(tag)) {
+		let element = current_element;
+		while (element !== null) {
+			if (element.tag === 'p') {
+				error_on_client(payload, `<${tag}> is invalid inside <p>`);
+			}
+			element = element.parent;
+		}
+	}
+	current_element = {
+		tag,
+		parent: current_element
+	};
+}
+
+export function pop_element() {
+	if (current_element !== null) {
+		current_element = current_element.parent;
+	}
+}
+
 /**
  * @param {Payload} payload
  * @param {string} tag
diff --git a/packages/svelte/src/legacy/legacy-client.js b/packages/svelte/src/legacy/legacy-client.js
index 5b962d309..09f122e37 100644
--- a/packages/svelte/src/legacy/legacy-client.js
+++ b/packages/svelte/src/legacy/legacy-client.js
@@ -15,7 +15,7 @@ import * as $ from '../internal/index.js';
  * 	component: import('../main/public.js').SvelteComponent<Props, Events, Slots>;
  * 	immutable?: boolean;
  * 	hydrate?: boolean;
- * 	recover?: false;
+ * 	recover?: boolean;
  * }} options
  * @returns {import('../main/public.js').SvelteComponent<Props, Events, Slots> & Exports}
  */
diff --git a/packages/svelte/tests/runtime-legacy/shared.ts b/packages/svelte/tests/runtime-legacy/shared.ts
index 062168de5..02168c79f 100644
--- a/packages/svelte/tests/runtime-legacy/shared.ts
+++ b/packages/svelte/tests/runtime-legacy/shared.ts
@@ -67,6 +67,7 @@ export interface RuntimeTest<Props extends Record<string, any> = Record<string,
 	warnings?: string[];
 	expect_unhandled_rejections?: boolean;
 	withoutNormalizeHtml?: boolean;
+	recover?: boolean;
 }
 
 let unhandled_rejection: Error | null = null;
@@ -258,7 +259,7 @@ async function run_test_variant(
 				target,
 				immutable: config.immutable,
 				intro: config.intro,
-				recover: false,
+				recover: config.recover === undefined ? false : config.recover,
 				hydrate: variant === 'hydrate'
 			});
 
diff --git a/packages/svelte/tests/runtime-runes/samples/invalid-html-ssr/Component.svelte b/packages/svelte/tests/runtime-runes/samples/invalid-html-ssr/Component.svelte
new file mode 100644
index 000000000..847d402cf
--- /dev/null
+++ b/packages/svelte/tests/runtime-runes/samples/invalid-html-ssr/Component.svelte
@@ -0,0 +1 @@
+<h1>foo</h1>
diff --git a/packages/svelte/tests/runtime-runes/samples/invalid-html-ssr/_config.js b/packages/svelte/tests/runtime-runes/samples/invalid-html-ssr/_config.js
new file mode 100644
index 000000000..9ca851217
--- /dev/null
+++ b/packages/svelte/tests/runtime-runes/samples/invalid-html-ssr/_config.js
@@ -0,0 +1,41 @@
+import { test } from '../../test';
+
+let console_error = console.error;
+
+/**
+ * @type {any[]}
+ */
+const log = [];
+
+export default test({
+	compileOptions: {
+		dev: true
+	},
+
+	html: `<p></p><h1>foo</h1><p></p>`,
+
+	recover: true,
+
+	before_test() {
+		console.error = (x) => {
+			log.push(x);
+		};
+	},
+
+	after_test() {
+		console.error = console_error;
+		log.length = 0;
+	},
+
+	async test({ assert, target, variant }) {
+		await assert.htmlEqual(target.innerHTML, `<p></p><h1>foo</h1><p></p>`);
+		if (variant === 'hydrate') {
+			assert.equal(
+				log[0],
+				`Svelte SSR validation error:\n\n<h1> is invalid inside <p>\n\n` +
+					'Ensure your components render valid HTML as the browser will try to repair invalid HTML, ' +
+					'which may result in content being shifted around and will likely result in a hydration mismatch.'
+			);
+		}
+	}
+});
diff --git a/packages/svelte/tests/runtime-runes/samples/invalid-html-ssr/main.svelte b/packages/svelte/tests/runtime-runes/samples/invalid-html-ssr/main.svelte
new file mode 100644
index 000000000..e7fb324d9
--- /dev/null
+++ b/packages/svelte/tests/runtime-runes/samples/invalid-html-ssr/main.svelte
@@ -0,0 +1,7 @@
+<script>
+	import Component from "./Component.svelte";
+</script>
+
+<p>
+	<Component />
+</p>
diff --git a/packages/svelte/types/index.d.ts b/packages/svelte/types/index.d.ts
index d3fb1f340..0fd664f09 100644
--- a/packages/svelte/types/index.d.ts
+++ b/packages/svelte/types/index.d.ts
@@ -1724,7 +1724,7 @@ declare module 'svelte/legacy' {
 		component: SvelteComponent<Props, Events, Slots>;
 		immutable?: boolean | undefined;
 		hydrate?: boolean | undefined;
-		recover?: false | undefined;
+		recover?: boolean | undefined;
 	}): SvelteComponent<Props, Events, Slots> & Exports;
 	/**
 	 * Takes the component function and returns a Svelte 4 compatible component constructor.