fix(site-builder): make formatHtml quote- and raw-text-aware

Code review found two Important bugs from only <pre> being exempted from
the naive </> tag-boundary scan: a > inside a quoted attribute value split
tags and broke idempotency, and <script>/<style> (declared BLOCK_TAGS but
never given raw-text treatment) let JS/CSS < and > desync sibling nesting.
Adds a quote-aware tag-end scanner, generalizes verbatim handling to
<script>/<style>, and makes close-tag stack popping verify the tag name
before popping instead of blindly popping by position.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-08-08 18:59:57 -07:00
co-authored by Claude Opus 5
parent 321a193b83
commit 51f3fe81b6
2 changed files with 105 additions and 18 deletions
+42
View File
@@ -39,3 +39,45 @@ describe('formatHtml', () => {
expect(formatHtml('</div><p>x</p>')).toBe('</div>\n<p>x</p>'); expect(formatHtml('</div><p>x</p>')).toBe('</div>\n<p>x</p>');
}); });
}); });
// Regression coverage from code review: only <pre> was originally exempted
// from the naive '<'/'>' tag-boundary scan, which let a '>' inside a quoted
// attribute value corrupt output, and let '<'/'>' inside <script>/<style>
// content be misparsed as tag boundaries.
describe('formatHtml - raw content and quoted attributes', () => {
test('a ">" inside a quoted attribute value does not split the tag', () => {
const src = '<div title="a>b"><p>hi</p></div><footer>bye</footer>';
expect(formatHtml(src)).toBe(
'<div title="a>b">\n <p>hi</p>\n</div>\n<footer>bye</footer>',
);
});
test('"<" and ">" inside <script> content do not desync sibling nesting', () => {
const src = '<div><script>a < b;</script></div><div><script>c < d;</script></div>';
expect(formatHtml(src)).toBe(
'<div>\n <script>a < b;</script>\n</div>\n<div>\n <script>c < d;</script>\n</div>',
);
});
test('a CSS child combinator inside <style> content is not treated as markup', () => {
const src = '<div><style>div > p { color: red; }</style></div>';
expect(formatHtml(src)).toBe(
'<div>\n <style>div > p { color: red; }</style>\n</div>',
);
});
test('is idempotent across a quoted ">" attribute and <script> content', () => {
const quotedAttr = formatHtml('<div title="a>b"><p>hi</p></div><footer>bye</footer>');
expect(formatHtml(quotedAttr)).toBe(quotedAttr);
const scriptSrc = '<div><script>a < b;</script></div><div><script>c < d;</script></div>';
const scripted = formatHtml(scriptSrc);
expect(formatHtml(scripted)).toBe(scripted);
});
test('an unclosed <pre> is swallowed verbatim to the end of the document', () => {
expect(formatHtml('<div><pre>no closing tag here')).toBe(
'<div>\n <pre>no closing tag here',
);
});
});
+63 -18
View File
@@ -7,7 +7,13 @@
* user markup is a formatter people stop trusting. * user markup is a formatter people stop trusting.
* *
* Inline tags (<strong>, <a>, <span>, ...) are left exactly where they sit, * Inline tags (<strong>, <a>, <span>, ...) are left exactly where they sit,
* and <pre> contents are copied through verbatim. * and <pre>/<script>/<style> contents are copied through verbatim -- their
* content is scanned only for the literal closing tag, never treated as
* markup, so `<` / `>` inside JS comparisons or CSS combinators can't be
* mistaken for tag boundaries.
*
* The tag-boundary scan itself is quote-aware: a `>` inside a single- or
* double-quoted attribute value (e.g. `title="a>b"`) does not end the tag.
* *
* Design note: a block-open tag is not committed to its own output line the * Design note: a block-open tag is not committed to its own output line the
* moment it is seen. It stays "pending" on a stack frame; if only inline * moment it is seen. It stays "pending" on a stack frame; if only inline
@@ -31,6 +37,9 @@ const VOID_TAGS = new Set([
'link', 'meta', 'param', 'source', 'track', 'wbr', 'link', 'meta', 'param', 'source', 'track', 'wbr',
]); ]);
/** Elements whose content is never markup -- read verbatim to the literal closing tag. */
const RAW_TEXT_TAGS = new Set(['pre', 'script', 'style']);
const INDENT = ' '; const INDENT = ' ';
interface Token { interface Token {
@@ -47,7 +56,31 @@ function pushTextToken(tokens: Token[], raw: string): void {
if (collapsed.trim()) tokens.push({ text: collapsed, tag: '', kind: 'text' }); if (collapsed.trim()) tokens.push({ text: collapsed, tag: '', kind: 'text' });
} }
/** Split source into tags and text runs, treating <pre>...</pre> as one atom. */ /**
* Find the '>' that closes the tag opened at `lt` (the index of its '<'),
* without being fooled by a '>' inside a single- or double-quoted
* attribute value. Returns -1 if the tag is never closed.
*/
function findTagEnd(src: string, lt: number): number {
let i = lt + 1;
let quote: string | null = null;
while (i < src.length) {
const ch = src[i];
if (quote) {
if (ch === quote) quote = null;
} else if (ch === '"' || ch === "'") {
quote = ch;
} else if (ch === '>') {
return i;
}
i += 1;
}
return -1;
}
/** Split source into tags and text runs, treating raw-text element bodies as one atom. */
function tokenize(src: string): Token[] { function tokenize(src: string): Token[] {
const tokens: Token[] = []; const tokens: Token[] = [];
let i = 0; let i = 0;
@@ -64,7 +97,7 @@ function tokenize(src: string): Token[] {
pushTextToken(tokens, src.slice(i, lt)); pushTextToken(tokens, src.slice(i, lt));
} }
const gt = src.indexOf('>', lt); const gt = findTagEnd(src, lt);
if (gt === -1) { if (gt === -1) {
// Unterminated '<' -- emit the remainder as text rather than looping. // Unterminated '<' -- emit the remainder as text rather than looping.
pushTextToken(tokens, src.slice(lt)); pushTextToken(tokens, src.slice(lt));
@@ -75,15 +108,18 @@ function tokenize(src: string): Token[] {
const nameMatch = /^<\/?\s*([a-zA-Z][a-zA-Z0-9-]*)/.exec(raw); const nameMatch = /^<\/?\s*([a-zA-Z][a-zA-Z0-9-]*)/.exec(raw);
const tag = nameMatch ? nameMatch[1].toLowerCase() : ''; const tag = nameMatch ? nameMatch[1].toLowerCase() : '';
// <pre> swallows everything up to its closing tag, untouched. // <pre>/<script>/<style> swallow everything up to their closing tag,
if (tag === 'pre' && !raw.startsWith('</')) { // untouched -- their content is never scanned as markup. If no closing
const closeIdx = src.toLowerCase().indexOf('</pre>', gt); // tag exists, swallow to the end of the document rather than risk
if (closeIdx !== -1) { // misparsing raw JS/CSS as tags.
const end = closeIdx + '</pre>'.length; if (RAW_TEXT_TAGS.has(tag) && !raw.startsWith('</')) {
tokens.push({ text: src.slice(lt, end), tag: 'pre', kind: 'verbatim' }); const closeRe = new RegExp(`</${tag}\\s*>`, 'i');
i = end; const rest = src.slice(gt + 1);
continue; const match = closeRe.exec(rest);
} const end = match ? gt + 1 + match.index + match[0].length : src.length;
tokens.push({ text: src.slice(lt, end), tag, kind: 'verbatim' });
i = end;
continue;
} }
const isClose = raw.startsWith('</'); const isClose = raw.startsWith('</');
@@ -105,6 +141,8 @@ function tokenize(src: string): Token[] {
interface Frame { interface Frame {
/** Raw text of the open tag. */ /** Raw text of the open tag. */
text: string; text: string;
/** Lowercased tag name, used to pair this frame with its real close tag. */
tag: string;
/** Indent depth at which this element's tags render. */ /** Indent depth at which this element's tags render. */
depth: number; depth: number;
/** Inline content accumulated directly under this element since it opened /** Inline content accumulated directly under this element since it opened
@@ -156,14 +194,21 @@ export function formatHtml(src: string): string {
} }
if (token.kind === 'close') { if (token.kind === 'close') {
if (stack.length === 0) { const top = stack[stack.length - 1];
// Unbalanced close with nothing open -- emit at depth 0.
flushRootInline(); if (!top || top.tag !== token.tag) {
lines.push(token.text); // Stray close: either nothing is open, or the innermost open frame
// is a *different* element. Don't fabricate a pairing by popping
// it anyway -- that would misattribute a real element's close tag
// to the wrong element and leave the stack silently desynced for
// everything after it. Emit this close in place instead, and leave
// whatever is genuinely open on the stack for later resolution.
if (stack.length === 0) flushRootInline();
lines.push(INDENT.repeat(stack.length) + token.text);
continue; continue;
} }
const top = stack.pop()!; stack.pop();
if (!top.committed) { if (!top.committed) {
// Nothing block-level ever interrupted this element: merge the // Nothing block-level ever interrupted this element: merge the
// open tag, its inline content, and the close tag onto one line. // open tag, its inline content, and the close tag onto one line.
@@ -185,7 +230,7 @@ export function formatHtml(src: string): string {
const depth = stack.length; const depth = stack.length;
if (token.kind === 'open') { if (token.kind === 'open') {
stack.push({ text: token.text, depth, inline: '', committed: false }); stack.push({ text: token.text, tag: token.tag, depth, inline: '', committed: false });
continue; continue;
} }