GROOVY-12278: Escape markup attribute values and refuse names which a…

[paulk-asert]

…re 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.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 83.33333% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.1847%. Comparing base (55dcfb9) to head (ff20e0a).
⚠️ Report is 12 commits behind head on master.

Files with missing lines	Patch %	Lines
...c/main/groovy/groovy/text/markup/BaseTemplate.java	83.3333%	0 Missing and 3 partials ⚠️

Additional details and impacted files

@@                Coverage Diff                 @@
##               master      #2815        +/-   ##
==================================================
+ Coverage     70.1846%   70.1847%   +0.0002%     
- Complexity      35846      35875        +29     
==================================================
  Files            1562       1563         +1     
  Lines          132542     132563        +21     
  Branches        24379      24395        +16     
==================================================
+ Hits            93024      93039        +15     
+ Misses          31109      31107         -2     
- Partials         8409       8417         +8     

Files with missing lines	Coverage Δ
...c/main/groovy/groovy/text/markup/BaseTemplate.java	90.0524% <83.3333%> (-0.9589%)	⬇️

... and 16 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Copilot]

Pull request overview

Updates MarkupTemplateEngine’s attribute rendering to treat attribute values as data (escaping XML metacharacters) and to reject invalid attribute names (preventing attribute-shape injection), with accompanying regression tests for GROOVY-12278.

Changes:

• Escape attribute values for &, <, >, and the configured quote delimiter when emitting attributes.
• Reject attribute names that are not valid “names” instead of attempting to escape them; adjust xmlDeclaration() to emit a proper encoding attribute.
• Add tests covering attribute-value escaping and invalid attribute-name rejection.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
subprojects/groovy-templates/src/main/groovy/groovy/text/markup/BaseTemplate.java	Implements attribute value escaping + attribute name validation; fixes xmlDeclaration() encoding attribute emission; removes unused quote-escaping helper.
subprojects/groovy-templates/src/test/groovy/groovy/text/MarkupTemplateEngineTest.groovy	Adds GROOVY-12278 regression tests for attribute escaping and invalid attribute-name rejection.

Suppressed comments (1)

subprojects/groovy-templates/src/main/groovy/groovy/text/markup/BaseTemplate.java:263

• checkAttributeName implements only a simplified, UTF-16 char-based subset of XML name rules. Since MarkupTemplateEngine targets XML/XHTML, this can reject valid XML names (e.g. characters represented as surrogate pairs, and other XML NameChar categories) and may not match the XML 1.0/1.1 Name production the engine claims to generate.

    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);
        }
    }

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.