You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by pa...@apache.org on 2026/08/19 08:59:01 UTC
(groovy) branch master updated: GROOVY-12278: Escape markup attribute values and refuse names which are not names
This is an automated email from the ASF dual-hosted git repository.
paulk-asert pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/master by this push:
new 307cd841e9 GROOVY-12278: Escape markup attribute values and refuse names which are not names
307cd841e9 is described below
commit 307cd841e90b918be94c40b147441f2af744d78e
Author: Paul King <pa...@asert.com.au>
AuthorDate: Tue Aug 18 20:55:39 2026 +1000
GROOVY-12278: Escape markup attribute values and refuse names which are not names
MarkupTemplateEngine writes element text through escapeXml, so a template
author can reasonably read the engine as treating the values it is given as
data. The attribute path did not hold up that reading. A value was escaped
only for the quote character configured as the delimiter, and an attribute
name was written exactly as it arrived.
Escape a value for the delimiter, as before, and additionally for the
ampersand and the angle brackets, which are not well formed inside an
attribute value whichever quote surrounds it. The other quote character is
neither unsafe nor ill formed there, so it is left as written and output is
unchanged for every value that was already well formed.
Refuse an attribute name which is not a name. A name has no escaped form:
escaping one produces a different name rather than a safe version of the
same one, so a name arriving from data is checked and rejected instead. This
is the half with teeth, since a map key such as
x='1' onmouseover='alert(1)'
was previously written out and introduced attributes of its own.
Doing so surfaced that xmlDeclaration passed " encoding" as an attribute
name, using a leading space as a separator; the space is now written
separately and the name is a name.
escapeQuotes had no remaining caller and is removed.
Behaviour change worth a release note: a template which places an ampersand
or an angle bracket in an attribute value now emits it escaped, and one
which builds attribute names from data will fail rather than emit markup
whose shape the data chose.
---
.../groovy/groovy/text/markup/BaseTemplate.java | 53 ++++++++++++++---
.../groovy/text/MarkupTemplateEngineTest.groovy | 66 ++++++++++++++++++++++
2 files changed, 112 insertions(+), 7 deletions(-)
diff --git a/subprojects/groovy-templates/src/main/groovy/groovy/text/markup/BaseTemplate.java b/subprojects/groovy-templates/src/main/groovy/groovy/text/markup/BaseTemplate.java
index 3415397b5e..4b304a680b 100644
--- a/subprojects/groovy-templates/src/main/groovy/groovy/text/markup/BaseTemplate.java
+++ b/subprojects/groovy-templates/src/main/groovy/groovy/text/markup/BaseTemplate.java
@@ -168,7 +168,8 @@ public abstract class BaseTemplate implements Writable {
out.write("<?xml ");
writeAttribute("version", "1.0");
if (configuration.getDeclarationEncoding() != null) {
- writeAttribute(" encoding", configuration.getDeclarationEncoding());
+ out.write(' ');
+ writeAttribute("encoding", configuration.getDeclarationEncoding());
}
out.write("?>");
out.write(configuration.getNewLineString());
@@ -213,13 +214,56 @@ public abstract class BaseTemplate implements Writable {
}
private void writeAttribute(String attName, String value) throws IOException {
+ checkAttributeName(attName);
out.write(attName);
out.write("=");
writeQt();
- out.write(escapeQuotes(value));
+ out.write(escapeAttributeValue(value));
writeQt();
}
+ /**
+ * Escapes an attribute value. The delimiter in use would otherwise end the value; the
+ * ampersand and less-than are not well formed inside an attribute value; and the
+ * greater-than, though well formed there, is escaped too so a value is treated exactly as
+ * element text is. The other quote character, being neither the delimiter nor ill formed,
+ * is left as written.
+ *
+ * @param str the attribute value
+ * @return the escaped value
+ */
+ private String escapeAttributeValue(final String str) {
+ String quote = configuration.isUseDoubleQuotes() ? "\"" : "'";
+ String escape = configuration.isUseDoubleQuotes() ? """ : "'";
+ // The ampersand goes first so the entities introduced after it are not re-escaped.
+ return str.replace("&", "&")
+ .replace("<", "<")
+ .replace(">", ">")
+ .replace(quote, escape);
+ }
+
+ /**
+ * Rejects an attribute name which is not a name.
+ * <p>
+ * A name has no escaped form: escaping one yields a different name rather than a safe
+ * version of the same one, and writing it unaltered lets a name taken from data introduce
+ * further attributes of its own. So an unusable name is refused instead.
+ *
+ * @param attName the attribute name to check
+ * @throws IllegalArgumentException if the name cannot be written as an attribute name
+ */
+ private static void checkAttributeName(final String attName) {
+ boolean usable = attName != null && !attName.isEmpty()
+ && (Character.isLetter(attName.charAt(0)) || attName.charAt(0) == '_' || attName.charAt(0) == ':');
+ for (int i = 1; usable && i < attName.length(); i += 1) {
+ char c = attName.charAt(i);
+ usable = Character.isLetterOrDigit(c) || c == '-' || c == '_' || c == '.' || c == ':';
+ }
+ if (!usable) {
+ throw new IllegalArgumentException("Invalid markup attribute name: " + attName);
+ }
+ }
+
private void writeQt() throws IOException {
if (configuration.isUseDoubleQuotes()) {
out.write('"');
@@ -235,11 +279,6 @@ public abstract class BaseTemplate implements Writable {
}
}
- private String escapeQuotes(String str) {
- String quote = configuration.isUseDoubleQuotes() ? "\"" : "'";
- String escape = configuration.isUseDoubleQuotes() ? """ : "'";
- return str.replace(quote, escape);
- }
/**
* This is the main method responsible for writing a tag and its attributes.
diff --git a/subprojects/groovy-templates/src/test/groovy/groovy/text/MarkupTemplateEngineTest.groovy b/subprojects/groovy-templates/src/test/groovy/groovy/text/MarkupTemplateEngineTest.groovy
index 29e72d2e9a..fecfbc65f8 100644
--- a/subprojects/groovy-templates/src/test/groovy/groovy/text/MarkupTemplateEngineTest.groovy
+++ b/subprojects/groovy-templates/src/test/groovy/groovy/text/MarkupTemplateEngineTest.groovy
@@ -273,6 +273,72 @@ final class MarkupTemplateEngineTest {
assert rendered.toString() == '<html><a href=\'foo.html\'>Link text</a><tagWithQuote attr=\'fo'o\'/></html>'
}
+ // GROOVY-12278: an attribute value is data, as element text is. yield() escapes all five XML
+ // metacharacters; the attribute path escaped only the delimiter in use, so an ampersand or a
+ // less-than in a value produced markup that is not well formed, and a greater-than was left
+ // unescaped where element text escapes it.
+ @Test
+ void testAttributeValuesAreEscapedLikeElementText() {
+ def engine = new MarkupTemplateEngine(new TemplateConfiguration())
+ def template = engine.createTemplate '''
+ html {
+ div(title: value)
+ }
+ '''
+ StringWriter rendered = new StringWriter()
+ template.make([value: 'a & b < c > d']).writeTo(rendered)
+ assert rendered.toString() == "<html><div title='a & b < c > d'/></html>"
+ }
+
+ // The delimiter still cannot end the value, and the other quote is neither unsafe nor
+ // ill-formed inside one, so it is left as written.
+ @Test
+ void testAttributeValueCannotEndItsOwnAttribute() {
+ def engine = new MarkupTemplateEngine(new TemplateConfiguration())
+ def template = engine.createTemplate '''
+ html {
+ div(title: value)
+ }
+ '''
+ StringWriter rendered = new StringWriter()
+ template.make([value: "x' onmouseover='alert(1)"]).writeTo(rendered)
+ assert !rendered.toString().contains("onmouseover='alert")
+ assert rendered.toString().contains(''')
+ }
+
+ // GROOVY-12278: a name has no escaped form, so a name taken from data which is not a name is
+ // refused rather than written out to introduce attributes of its own.
+ @Test
+ void testAttributeNameTakenFromDataMustBeAName() {
+ def engine = new MarkupTemplateEngine(new TemplateConfiguration())
+ def template = engine.createTemplate '''
+ html {
+ div(attrs)
+ }
+ '''
+ StringWriter rendered = new StringWriter()
+ def err = shouldFail(IllegalArgumentException) {
+ template.make([attrs: ["x='1' onmouseover='alert(1)'": 'y']]).writeTo(rendered)
+ }
+ assert err.message.contains('Invalid markup attribute name')
+ }
+
+ @Test
+ void testOrdinaryAttributeNamesStillRender() {
+ def engine = new MarkupTemplateEngine(new TemplateConfiguration())
+ def template = engine.createTemplate '''
+ html {
+ div('data-id': 1, 'xlink:href': 'a', _private: 2, 'a.b': 3)
+ }
+ '''
+ StringWriter rendered = new StringWriter()
+ template.make().writeTo(rendered)
+ assert rendered.toString().contains("data-id='1'")
+ assert rendered.toString().contains("xlink:href='a'")
+ assert rendered.toString().contains("_private='2'")
+ assert rendered.toString().contains("a.b='3'")
+ }
+
@Test
void testTagsWithAttributesAndDoubleQuotes() {
def engine = new MarkupTemplateEngine(new TemplateConfiguration())