From e6945bcf23b6ad29388bbadaf5bb3cc31dd4a114 Mon Sep 17 00:00:00 2001 From: cin Date: Wed, 24 Jan 2024 07:46:28 +0800 Subject: [PATCH] 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 Co-authored-by: Nate Moore Co-authored-by: Nate Moore Co-authored-by: Happydev <81974850+MoustaphaDev@users.noreply.github.com> --- .changeset/metal-garlics-exercise.md | 5 ++ .../src/vite-plugin-html/transform/escape.ts | 12 +++-- .../src/vite-plugin-html/transform/slots.ts | 4 +- .../src/vite-plugin-html/transform/utils.ts | 43 +++++++++++++--- .../fixtures/html-escape-complex/package.json | 8 +++ .../html-escape-complex/src/pages/index.html | 31 ++++++++++++ .../astro/test/html-escape-complex.test.js | 50 +++++++++++++++++++ pnpm-lock.yaml | 6 +++ 8 files changed, 146 insertions(+), 13 deletions(-) create mode 100644 .changeset/metal-garlics-exercise.md create mode 100644 packages/astro/test/fixtures/html-escape-complex/package.json create mode 100644 packages/astro/test/fixtures/html-escape-complex/src/pages/index.html create mode 100644 packages/astro/test/html-escape-complex.test.js diff --git a/.changeset/metal-garlics-exercise.md b/.changeset/metal-garlics-exercise.md new file mode 100644 index 0000000000..ad9ca5e3b1 --- /dev/null +++ b/.changeset/metal-garlics-exercise.md @@ -0,0 +1,5 @@ +--- +"astro": patch +--- + +Fixes escaping behavior for `.html` files and components diff --git a/packages/astro/src/vite-plugin-html/transform/escape.ts b/packages/astro/src/vite-plugin-html/transform/escape.ts index 1c250d43dc..f1ffa02e46 100644 --- a/packages/astro/src/vite-plugin-html/transform/escape.ts +++ b/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 ?? {})) { - const newKey = needsEscape(key) ? escape(key) : key; - const newValue = needsEscape(value) ? escape(value) : value; + if (!node.properties) return; + for (let [key, value] of Object.entries(node.properties)) { + 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}"`); } diff --git a/packages/astro/src/vite-plugin-html/transform/slots.ts b/packages/astro/src/vite-plugin-html/transform/slots.ts index a549a48c91..e324edda42 100644 --- a/packages/astro/src/vite-plugin-html/transform/slots.ts +++ b/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()}\`}`); } }); }; diff --git a/packages/astro/src/vite-plugin-html/transform/utils.ts b/packages/astro/src/vite-plugin-html/transform/utils.ts index 6d61e7532d..667910450a 100644 --- a/packages/astro/src/vite-plugin-html/transform/utils.ts +++ b/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; - s.overwrite(start, end, newValue); + const length = token.length; + 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; } diff --git a/packages/astro/test/fixtures/html-escape-complex/package.json b/packages/astro/test/fixtures/html-escape-complex/package.json new file mode 100644 index 0000000000..f37957dd1f --- /dev/null +++ b/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:*" + } +} diff --git a/packages/astro/test/fixtures/html-escape-complex/src/pages/index.html b/packages/astro/test/fixtures/html-escape-complex/src/pages/index.html new file mode 100644 index 0000000000..7399304836 --- /dev/null +++ b/packages/astro/test/fixtures/html-escape-complex/src/pages/index.html @@ -0,0 +1,31 @@ + + + + + + + + Astro + + + +
+
+
+
+
+
+ + + + + diff --git a/packages/astro/test/html-escape-complex.test.js b/packages/astro/test/html-escape-complex.test.js new file mode 100644 index 0000000000..beed15501b --- /dev/null +++ b/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