Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading