diff options
| -rw-r--r-- | .changeset/metal-garlics-exercise.md | 5 | ||||
| -rw-r--r-- | packages/astro/src/vite-plugin-html/transform/escape.ts | 12 | ||||
| -rw-r--r-- | packages/astro/src/vite-plugin-html/transform/slots.ts | 4 | ||||
| -rw-r--r-- | packages/astro/src/vite-plugin-html/transform/utils.ts | 43 | ||||
| -rw-r--r-- | packages/astro/test/fixtures/html-escape-complex/package.json | 8 | ||||
| -rw-r--r-- | packages/astro/test/fixtures/html-escape-complex/src/pages/index.html | 31 | ||||
| -rw-r--r-- | packages/astro/test/html-escape-complex.test.js | 50 | ||||
| -rw-r--r-- | pnpm-lock.yaml | 6 |
8 files changed, 146 insertions, 13 deletions
diff --git a/.changeset/metal-garlics-exercise.md b/.changeset/metal-garlics-exercise.md new file mode 100644 index 000000000..ad9ca5e3b --- /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 1c250d43d..f1ffa02e4 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 a549a48c9..e324edda4 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 6d61e7532..667910450 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 000000000..f37957dd1 --- /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 000000000..739930483 --- /dev/null +++ b/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> 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 000000000..beed15501 --- /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 <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); + }); + }); +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index fafa7a936..32d3dea01 100644 --- a/pnpm-lock.yaml +++ b/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: |