GROOVY-12275: Encode snippet attribute values and bound snippet markup regexes

Two defects in the same {@snippet} handling, both reached from a doc comment
in the source being documented.

The class and id attributes were appended to the generated element without
encoding. The attribute parser accepts a double quote inside a value which
was single quoted or unquoted, so a value could close its attribute and the
tag around it. Encode both through a new SimpleGroovyClassDoc.encodeAttribute,
which escapes the ampersand first and then the characters that can end an
attribute or start a tag. The snippet body was already escaped; this brings
the attributes up to the same standard.

A markup directive's regex attribute was compiled and run against snippet
lines with no bound. Give each directive a deadline using RegexGuard, and
leave the line unannotated rather than half annotated if it expires.

The payload in the test is worth a note. The finding cites (a+)+$ against a
long run of characters, and on a current JDK that is not slow: the textbook
nested-quantifier patterns, (a+)+b, (a|aa)+$, (x+x+)+y and (a*)*b among
them, all complete in about a millisecond, because the engine recognises
them. A backreference still backtracks exponentially. Measured with the
guard removed, a directive carrying (a+)+\1b against a 32 character line
took 152 seconds to render one page, and grows exponentially with the line;
with the guard the same page renders in well under a second. So the finding
is right that the risk exists and wrong about how it is reached, and a test
built on its own example would have passed with or without a fix.

Author: paulk-asert

subprojects/groovy-groovydoc/src/main/java/org/codehaus/groovy/tools/groovydoc/SimpleGroovyClassDoc.java

@@ -1245,6 +1245,24 @@ public static String encodeAngleBrackets(String text) {
         return text == null ? null : text.replace("<", "&lt;").replace(">", "&gt;");
     }
 
+    /**
+     * Escapes text for use as the value of a quoted HTML attribute, so that text taken from a
+     * doc comment cannot close the attribute or the tag around it.
+     *
+     * @param text the text to escape
+     * @return the escaped text
+     */
+    public static String encodeAttribute(String text) {
+        if (text == null) return null;
+        // The ampersand is replaced first so that the entities introduced below are not
+        // themselves re-escaped.
+        return text.replace("&", "&amp;")
+                .replace("<", "&lt;")
+                .replace(">", "&gt;")
+                .replace("\"", "&quot;")
+                .replace("'", "&#39;");
+    }
+
     /**
      * Stores the rendered class name including any type arguments.
      *

subprojects/groovy-groovydoc/src/main/java/org/codehaus/groovy/tools/groovydoc/TagRenderer.java

@@ -30,6 +30,9 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
+
+import groovy.util.regex.RegexGuard;
+import groovy.util.regex.RegexTimeoutException;
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
@@ -100,6 +103,13 @@
  */
 final class TagRenderer {
 
+    /**
+     * How long a single snippet markup directive may spend matching one line. A directive
+     * carries a regex written by the author of the documented source, so the budget bounds what
+     * a doc comment can cost the build rather than trying to decide which patterns are safe.
+     */
+    private static final long MATCH_TIMEOUT_MILLIS = 250L;
+
     /** Block-tag names that get merged under a single display heading. */
     static final Map<String, String> COLLATED_TAGS = new LinkedHashMap<>();
     static {
@@ -450,9 +460,15 @@ private static int renderSnippetAt(String text, int start, int nameEnd, StringBu
         // or hyperlinks.
         String processed = processSnippetMarkup(dedented, links, relPath, rootDoc, classDoc);
         out.append("<pre><code");
-        if (!combined.isEmpty()) out.append(" class=\"").append(combined).append('"');
+        // The class and id values come from snippet attributes, which the parser above lets
+        // carry a double quote when the attribute itself was single quoted or unquoted.
+        if (!combined.isEmpty()) {
+            out.append(" class=\"").append(SimpleGroovyClassDoc.encodeAttribute(combined)).append('"');
+        }
         String id = attrs.get("id");
-        if (id != null && !id.isEmpty()) out.append(" id=\"").append(id).append('"');
+        if (id != null && !id.isEmpty()) {
+            out.append(" id=\"").append(SimpleGroovyClassDoc.encodeAttribute(id)).append('"');
+        }
         out.append('>').append(processed).append("</code></pre>");
         return endPos - start;
     }
@@ -759,15 +775,28 @@ private static String applyDirective(String escaped, Directive d,
                                          GroovyRootDoc rootDoc, SimpleGroovyClassDoc classDoc) {
         Pattern pat = buildMatchPattern(d);
         if (pat == null) return escaped; // no match clause — nothing to do
-        Matcher m = pat.matcher(escaped);
+        // Both the pattern and the line it runs against come from the documented source, so a
+        // directive can otherwise pin the doc build on a few bytes. Give each directive a
+        // deadline rather than trying to judge which patterns are safe.
+        Matcher m;
+        try {
+            m = RegexGuard.matcher(pat, escaped, MATCH_TIMEOUT_MILLIS);
+        } catch (RegexTimeoutException e) {
+            return escaped;
+        }
         StringBuilder sb = new StringBuilder();
         int last = 0;
-        while (m.find()) {
-            sb.append(escaped, last, m.start());
-            String match = m.group();
-            String wrapped = wrapForDirective(match, d, links, relPath, rootDoc, classDoc);
-            sb.append(wrapped);
-            last = m.end();
+        try {
+            while (m.find()) {
+                sb.append(escaped, last, m.start());
+                String match = m.group();
+                String wrapped = wrapForDirective(match, d, links, relPath, rootDoc, classDoc);
+                sb.append(wrapped);
+                last = m.end();
+            }
+        } catch (RegexTimeoutException e) {
+            // Leave the line as it was rather than half annotated.
+            return escaped;
         }
         sb.append(escaped, last, escaped.length());
         return sb.toString();

subprojects/groovy-groovydoc/src/test/groovy/org/codehaus/groovy/tools/groovydoc/GroovyDocToolTest.java

@@ -252,6 +252,80 @@ public void testSnippetTagExternalFormLoadsFromSnippetFiles() throws Exception {
                 snip.contains("Licensed to the Apache"));
     }
 
+    // GROOVY-12275: snippet attribute values reach quoted HTML attributes. The attribute parser
+    // accepts a double quote inside a single-quoted or unquoted value, so an id could close the
+    // attribute and the tag around it.
+    public void testSnippetAttributeValuesCannotEscapeTheirAttribute() throws Exception {
+        String pkg = "org/codehaus/groovy/tools/groovydoc/testfiles/docfiles";
+        Path tmp = Files.createTempDirectory("snippet-attr-");
+        Path pkgDir = tmp.resolve(pkg);
+        Files.createDirectories(pkgDir);
+        Files.writeString(pkgDir.resolve("SnippetAttr.groovy"),
+                "package " + pkg.replace('/', '.') + "\n" +
+                "/**\n" +
+                " * {@snippet id='x\"><img src=q onerror=\"alert(1)' class='y\"><b' :\n" +
+                " * def a = 1\n" +
+                " * }\n" +
+                " */\n" +
+                "class SnippetAttr {}\n");
+
+        String doc = renderSingle(tmp, pkg, "SnippetAttr");
+        assertNotNull(doc);
+        assertTrue("the snippet should still render in:\n" + doc, doc.contains("<pre><code"));
+        assertFalse("an attribute value closed its attribute in:\n" + doc,
+                doc.contains("<img src=q"));
+        assertFalse("an attribute value opened a tag in:\n" + doc, doc.contains("\"><b"));
+    }
+
+    // GROOVY-12275: a markup directive's regex is written by the author of the documented
+    // source and runs against lines they also wrote, so it must not be able to pin the build.
+    // Note the payload: on a current JDK the textbook nested-quantifier patterns are optimised
+    // away and finish instantly, while a backreference still backtracks exponentially.
+    public void testSnippetMarkupRegexCannotHangTheBuild() throws Exception {
+        String pkg = "org/codehaus/groovy/tools/groovydoc/testfiles/docfiles";
+        Path tmp = Files.createTempDirectory("snippet-redos-");
+        Path pkgDir = tmp.resolve(pkg);
+        Files.createDirectories(pkgDir);
+        String payload = "a".repeat(32);
+        Files.writeString(pkgDir.resolve("SnippetRedos.groovy"),
+                "package " + pkg.replace('/', '.') + "\n" +
+                "/**\n" +
+                " * {@snippet lang=\"groovy\" :\n" +
+                " * " + payload + " // @highlight regex=\"(a+)+\\1b\" type=\"bold\"\n" +
+                " * }\n" +
+                " */\n" +
+                "class SnippetRedos {}\n");
+
+        long start = System.nanoTime();
+        String doc = renderSingle(tmp, pkg, "SnippetRedos");
+        long elapsedMs = (System.nanoTime() - start) / 1_000_000L;
+
+        assertNotNull(doc);
+        assertTrue("the snippet should still render in:\n" + doc, doc.contains("<pre><code"));
+        assertTrue("the snippet body should survive in:\n" + doc, doc.contains(payload));
+        // Unguarded this payload runs for minutes and grows exponentially with the line length;
+        // guarded it is bounded per directive. The threshold is loose so the test is about the
+        // bound existing, not about the speed of the machine.
+        assertTrue("rendering took " + elapsedMs + "ms, so the directive regex was not bounded",
+                elapsedMs < 30_000L);
+    }
+
+    /** Renders one class from a temporary source tree and returns its page. */
+    private String renderSingle(Path sourcePath, String pkg, String simpleName) throws Exception {
+        GroovyDocTool tool = new GroovyDocTool(
+                new FileSystemResourceManager("src/main/resources"),
+                new String[]{sourcePath.toString()},
+                GroovyDocTemplateInfo.DEFAULT_DOC_TEMPLATES,
+                GroovyDocTemplateInfo.DEFAULT_PACKAGE_TEMPLATES,
+                GroovyDocTemplateInfo.DEFAULT_CLASS_TEMPLATES,
+                new ArrayList<>(), null, new Properties()
+        );
+        tool.add(List.of(pkg + "/" + simpleName + ".groovy"));
+        MockOutputTool output = new MockOutputTool();
+        tool.renderToOutput(output, MOCK_DIR);
+        return output.getText(MOCK_DIR + "/" + pkg + "/" + simpleName + ".html");
+    }
+
     // Auto-strip opt-out: {@snippet file="X" keepHeader=true} preserves the
     // file content verbatim, and lang is inferred from the file's extension
     // when no explicit lang= is given.