From 51f3fe81b6a517a92ff95242c55ac13786e010f3 Mon Sep 17 00:00:00 2001
From: Josh Knapp <jknapp85@gmail.com>
Date: Sat, 8 Aug 2026 18:59:57 -0700
Subject: [PATCH] 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>
---
 craft/src/utils/format-html.test.ts | 42 +++++++++++++++
 craft/src/utils/format-html.ts      | 81 ++++++++++++++++++++++-------
 2 files changed, 105 insertions(+), 18 deletions(-)

diff --git a/craft/src/utils/format-html.test.ts b/craft/src/utils/format-html.test.ts
index a7bb1a1..fc9cb3a 100644
--- a/craft/src/utils/format-html.test.ts
+++ b/craft/src/utils/format-html.test.ts
@@ -39,3 +39,45 @@ describe('formatHtml', () => {
     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',
+    );
+  });
+});
diff --git a/craft/src/utils/format-html.ts b/craft/src/utils/format-html.ts
index 07214dd..f16e311 100644
--- a/craft/src/utils/format-html.ts
+++ b/craft/src/utils/format-html.ts
@@ -7,7 +7,13 @@
  * user markup is a formatter people stop trusting.
  *
  * 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
  * 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',
 ]);
 
+/** Elements whose content is never markup -- read verbatim to the literal closing tag. */
+const RAW_TEXT_TAGS = new Set(['pre', 'script', 'style']);
+
 const INDENT = '  ';
 
 interface Token {
@@ -47,7 +56,31 @@ function pushTextToken(tokens: Token[], raw: string): void {
   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[] {
   const tokens: Token[] = [];
   let i = 0;
@@ -64,7 +97,7 @@ function tokenize(src: string): Token[] {
       pushTextToken(tokens, src.slice(i, lt));
     }
 
-    const gt = src.indexOf('>', lt);
+    const gt = findTagEnd(src, lt);
     if (gt === -1) {
       // Unterminated '<' -- emit the remainder as text rather than looping.
       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 tag = nameMatch ? nameMatch[1].toLowerCase() : '';
 
-    // <pre> swallows everything up to its closing tag, untouched.
-    if (tag === 'pre' && !raw.startsWith('</')) {
-      const closeIdx = src.toLowerCase().indexOf('</pre>', gt);
-      if (closeIdx !== -1) {
-        const end = closeIdx + '</pre>'.length;
-        tokens.push({ text: src.slice(lt, end), tag: 'pre', kind: 'verbatim' });
-        i = end;
-        continue;
-      }
+    // <pre>/<script>/<style> swallow everything up to their closing tag,
+    // untouched -- their content is never scanned as markup. If no closing
+    // tag exists, swallow to the end of the document rather than risk
+    // misparsing raw JS/CSS as tags.
+    if (RAW_TEXT_TAGS.has(tag) && !raw.startsWith('</')) {
+      const closeRe = new RegExp(`</${tag}\\s*>`, 'i');
+      const rest = src.slice(gt + 1);
+      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('</');
@@ -105,6 +141,8 @@ function tokenize(src: string): Token[] {
 interface Frame {
   /** Raw text of the open tag. */
   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. */
   depth: number;
   /** 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 (stack.length === 0) {
-        // Unbalanced close with nothing open -- emit at depth 0.
-        flushRootInline();
-        lines.push(token.text);
+      const top = stack[stack.length - 1];
+
+      if (!top || top.tag !== token.tag) {
+        // 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;
       }
 
-      const top = stack.pop()!;
+      stack.pop();
       if (!top.committed) {
         // Nothing block-level ever interrupted this element: merge the
         // 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;
 
     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;
     }