Fix .html file escaping (#9606)

* fix: escape bug

* chore: add changeset

* fix: add test case

* fix: add test case

* fix: ut bug

* fix: ut bug

* Update .changeset/metal-garlics-exercise.md

* fix(html): properly handle escape sequences

* Update .changeset/metal-garlics-exercise.md

Co-authored-by: Happydev <81974850+MoustaphaDev@users.noreply.github.com>

* fix(html): properly handle attributes with escaped characters

* chore: improve tests

* chore: update lockfile

* chore: update changeset

* Update packages/astro/src/vite-plugin-html/transform/index.ts

---------

Co-authored-by: Emanuele Stoppa <my.burning@gmail.com>
Co-authored-by: Nate Moore <nate@astro.build>
Co-authored-by: Nate Moore <natemoo-re@users.noreply.github.com>
Co-authored-by: Happydev <81974850+MoustaphaDev@users.noreply.github.com>

.changeset/metal-garlics-exercise.md

@@ -0,0 +1,5 @@
+---
+"astro": patch
+---
+
+Fixes escaping behavior for `.html` files and components

packages/astro/src/vite-plugin-html/transform/escape.ts

@@ -3,19 +3,21 @@ import type MagicString from 'magic-string';
 import type { Plugin } from 'unified';
 import { visit } from 'unist-util-visit';
 
-import { escape, needsEscape, replaceAttribute } from './utils.js';
+import { escapeTemplateLiteralCharacters, needsEscape, replaceAttribute } from './utils.js';
 
 const rehypeEscape: Plugin<[{ s: MagicString }], Root> = ({ s }) => {
 	return (tree) => {
 		visit(tree, (node: Root | RootContent) => {
 			if (node.type === 'text' || node.type === 'comment') {
 				if (needsEscape(node.value)) {
-					s.overwrite(node.position!.start.offset!, node.position!.end.offset!, escape(node.value));
+					s.overwrite(node.position!.start.offset!, node.position!.end.offset!, escapeTemplateLiteralCharacters(node.value));
 				}
 			} else if (node.type === 'element') {
-				for (const [key, value] of Object.entries(node.properties ?? {})) {
+				if (!node.properties) return;
-					const newKey = needsEscape(key) ? escape(key) : key;
+				for (let [key, value] of Object.entries(node.properties)) {
-					const newValue = needsEscape(value) ? escape(value) : value;
+					key = key.replace(/([A-Z])/g, '-$1').toLowerCase()
+					const newKey = needsEscape(key) ? escapeTemplateLiteralCharacters(key) : key;
+					const newValue = needsEscape(value) ? escapeTemplateLiteralCharacters(value) : value;
 					if (newKey === key && newValue === value) continue;
 					replaceAttribute(s, node, key, value === '' ? newKey : `${newKey}="${newValue}"`);
 				}

packages/astro/src/vite-plugin-html/transform/slots.ts

@@ -3,7 +3,7 @@ import type { Plugin } from 'unified';
 
 import type MagicString from 'magic-string';
 import { visit } from 'unist-util-visit';
-import { escape } from './utils.js';
+import { escapeTemplateLiteralCharacters } from './utils.js';
 
 const rehypeSlots: Plugin<[{ s: MagicString }], Root> = ({ s }) => {
 	return (tree, file) => {
@@ -18,7 +18,7 @@ const rehypeSlots: Plugin<[{ s: MagicString }], Root> = ({ s }) => {
 				const text = file.value
 					.slice(first.position?.start.offset ?? 0, last.position?.end.offset ?? 0)
 					.toString();
-				s.overwrite(start, end, `\${${SLOT_PREFIX}["${name}"] ?? \`${escape(text).trim()}\`}`);
+				s.overwrite(start, end, `\${${SLOT_PREFIX}["${name}"] ?? \`${escapeTemplateLiteralCharacters(text).trim()}\`}`);
 			}
 		});
 	};

packages/astro/src/vite-plugin-html/transform/utils.ts

@@ -15,15 +15,46 @@ export function replaceAttribute(s: MagicString, node: Element, key: string, new
 	const token = tokens[0].replace(/([^>])(\>[\s\S]*$)/gim, '$1');
 	if (token.trim() === key) {
 		const end = start + key.length;
-		s.overwrite(start, end, newValue);
+		return s.overwrite(start, end, newValue, { contentOnly: true });
 	} else {
-		const end = start + `${key}=${tokens[2]}${tokens[3]}${tokens[2]}`.length;
+		const length = token.length;
-		s.overwrite(start, end, newValue);
+		const end = start + length;
+		return s.overwrite(start, end, newValue, { contentOnly: true });
 	}
 }
+
+// Embedding in our own template literal expression requires escaping
+// any meaningful template literal characters in the user's code!
+const NEEDS_ESCAPE_RE = /[`\\]|\$\{/g
+
 export function needsEscape(value: any): value is string {
-	return typeof value === 'string' && (value.includes('`') || value.includes('${'));
+	// Reset the RegExp's global state
+	NEEDS_ESCAPE_RE.lastIndex = 0;
+	return typeof value === 'string' && NEEDS_ESCAPE_RE.test(value);
 }
-export function escape(value: string) {
+
-	return value.replace(/`/g, '\\`').replace(/\$\{/g, '\\${');
+export function escapeTemplateLiteralCharacters(value: string) {
+	// Reset the RegExp's global state
+	NEEDS_ESCAPE_RE.lastIndex = 0;
+
+	let char: string | undefined;
+	let startIndex = 0;
+	let segment = '';
+	let text = '';
+	
+	// Rather than a naive `String.replace()`, we have to iterate through
+	// the raw contents to properly handle existing backslashes
+	while ([char] = NEEDS_ESCAPE_RE.exec(value) ?? []) {
+		// Final loop when char === undefined, append trailing content
+		if (!char) {
+			text += value.slice(startIndex);
+			break;
+		}
+		const endIndex = NEEDS_ESCAPE_RE.lastIndex - char.length;
+		const prefix = segment === '\\' ? '' : '\\';
+		segment = prefix + char;
+		text += value.slice(startIndex, endIndex) + segment;
+		startIndex = NEEDS_ESCAPE_RE.lastIndex;
+	}
+	return text;
 }

packages/astro/test/fixtures/html-escape-complex/package.json

@@ -0,0 +1,8 @@
+{
+  "name": "@test/html-escape-bug",
+  "version": "0.0.0",
+  "private": true,
+  "dependencies": {
+    "astro": "workspace:*"
+  }
+}

packages/astro/test/fixtures/html-escape-complex/src/pages/index.html

@@ -0,0 +1,31 @@
+<html lang="en">
+
+	<head>
+		<meta charset="utf-8" />
+		<link rel="icon" type="image/svg+xml" href="/favicon.svg" />
+		<meta name="viewport" content="width=device-width" />
+		<meta name="generator" content="{Astro.generator}" />
+		<title>Astro</title>
+	</head>
+
+	<body>
+		<div id="a" data-attr="${a}"></div>
+		<div id="b" data-attr="\\`${b}\\`"></div>
+		<div id="c" data-attr="\\\`${c}\\\`"></div>
+		<div id="d" data-attr="\\\\`${d}\\\\`"></div>
+		<div id="e" data-attr="\\\\\`${e}\\\\\`"></div>
+		<div id="f" data-attr="\\\\\\`${f}\\\\\\`"></div>
+
+		<script>
+			const normal = `There are ${count} things!`;
+			const content = `There are \`${count}\` things!`;
+			const a = "\`${a}\`";
+			const b = "\\`${b}\\`";
+			const c = "\\\`${c}\\\`";
+			const d = "\\\\`${d}\\\\`";
+			const e = "\\\\\`${e}\\\\\`";
+			const f = "\\\\\\`${f}\\\\\\`";
+		</script>
+	</body>
+
+</html>

packages/astro/test/html-escape-complex.test.js

@@ -0,0 +1,50 @@
+import { expect } from 'chai';
+import * as cheerio from 'cheerio';
+import { loadFixture } from './test-utils.js';
+
+describe('HTML Escape (Complex)', () => {
+	let fixture;
+	/** @type {string} */
+	let input;
+	/** @type {string} */
+	let output;
+
+	before(async () => {
+		fixture = await loadFixture({
+			root: './fixtures/html-escape-complex/',
+		});
+	});
+
+	describe('build', () => {
+		before(async () => {
+			await fixture.build();
+			// readFile operates relative to `dist`
+			input = await fixture.readFile('../src/pages/index.html');
+			output = await fixture.readFile('./index.html');
+		});
+
+		it('respects complex escape sequences in attributes', async () => {
+			const $in = cheerio.load(input);
+			const $out = cheerio.load(output);
+			for (const char of 'abcdef'.split('')) {
+				const attrIn = $in('#' + char).attr('data-attr');
+				const attrOut = $out('#' + char).attr('data-attr');
+				expect(attrOut).to.equal(attrIn);
+			}
+		});
+
+		it('respects complex escape sequences in <script>', async () => {
+			const $a = cheerio.load(input);
+			const $b = cheerio.load(output);
+			const scriptIn = $a('script');
+			const scriptOut = $b('script');
+
+			expect(scriptOut.text()).to.equal(scriptIn.text());
+		});
+
+		it('matches the entire source file', async () => {
+			// Ignore doctype insertion
+			expect(output.replace('<!DOCTYPE html>', '')).to.equal(input);
+		});
+	});
+});

pnpm-lock.yaml

@@ -2825,6 +2825,12 @@ importers:
         specifier: workspace:*
         version: link:../../..
 
+  packages/astro/test/fixtures/html-escape-complex:
+    dependencies:
+      astro:
+        specifier: workspace:*
+        version: link:../../..
+
   packages/astro/test/fixtures/html-page:
     dependencies:
       astro: