You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@grails.apache.org by jd...@apache.org on 2026/09/11 13:57:52 UTC
(grails-core) branch fix/xml-parser-hardening updated: strictly parse plugin config always
This is an automated email from the ASF dual-hosted git repository.
jdaugherty pushed a commit to branch fix/xml-parser-hardening
in repository https://gitbox.apache.org/repos/asf/grails-core.git
The following commit(s) were added to refs/heads/fix/xml-parser-hardening by this push:
new 890833e9c4 strictly parse plugin config always
890833e9c4 is described below
commit 890833e9c42078c531a260664248ee9ccdee6a84
Author: James Daugherty <jd...@jdresources.net>
AuthorDate: Fri Sep 11 09:57:41 2026 -0400
strictly parse plugin config always
---
.../apache/grails/core/plugins/PluginUtils.java | 5 +--
.../grails/core/plugins/PluginDiscoverySpec.groovy | 40 ++++++++++++----------
...balGrailsClassInjectorTransformationSpec.groovy | 39 +++++++++++++++++++++
grails-doc/src/en/guide/upgrading.adoc | 4 ++-
.../src/en/guide/upgrading/upgrading80x.adoc | 9 +++--
.../grails/gradle/common/XmlParserFeature.java | 6 ++--
.../org/grails/io/support/SpringIOUtils.java | 5 ++-
7 files changed, 78 insertions(+), 30 deletions(-)
diff --git a/grails-core/src/main/groovy/org/apache/grails/core/plugins/PluginUtils.java b/grails-core/src/main/groovy/org/apache/grails/core/plugins/PluginUtils.java
index c9cda0a9b2..83d618feab 100644
--- a/grails-core/src/main/groovy/org/apache/grails/core/plugins/PluginUtils.java
+++ b/grails-core/src/main/groovy/org/apache/grails/core/plugins/PluginUtils.java
@@ -138,8 +138,9 @@ public final class PluginUtils {
try {
Enumeration<URL> resources = classLoader.getResources(PLUGIN_XML_PATTERN);
- // descriptors on the classpath are trusted input and may declare a DOCTYPE
- SAXParser saxParser = SpringIOUtils.newSAXParser(true);
+ // Grails generates this descriptor and never writes a DOCTYPE, so it is read with the
+ // strict parser, matching the compile-time transform that generates and rewrites it
+ SAXParser saxParser = SpringIOUtils.newSAXParser();
while (resources.hasMoreElements()) {
URL url = resources.nextElement();
diff --git a/grails-core/src/test/groovy/org/apache/grails/core/plugins/PluginDiscoverySpec.groovy b/grails-core/src/test/groovy/org/apache/grails/core/plugins/PluginDiscoverySpec.groovy
index e33b4b52d7..8948786c8b 100644
--- a/grails-core/src/test/groovy/org/apache/grails/core/plugins/PluginDiscoverySpec.groovy
+++ b/grails-core/src/test/groovy/org/apache/grails/core/plugins/PluginDiscoverySpec.groovy
@@ -82,32 +82,36 @@ class PluginDiscoverySpec extends Specification {
tempDir.deleteDir()
}
- def 'reads a plugin descriptor that declares a doctype'() {
- given: 'a grails-plugin.xml whose DOCTYPE names a DTD that does not exist, so retrieval would fail'
- def tempDir = File.createTempDir()
- def metaInfDir = new File(tempDir, 'META-INF').tap { mkdirs() }
- def missingDtd = new File(tempDir, 'missing.dtd').toURI().toASCIIString()
- new File(metaInfDir, 'grails-plugin.xml').text = """<?xml version="1.0" encoding="UTF-8"?>
-<!DOCTYPE plugin SYSTEM '${missingDtd}'>
-<plugin name='test'>
- <type>com.example.TestGrailsPlugin</type>
- <resource>com.example.MyDomainClass</resource>
+ def 'skips a plugin descriptor that declares a doctype'() {
+ given: 'a descriptor that declares a DOCTYPE, which Grails never generates'
+ def doctypeDir = File.createTempDir()
+ def doctypeMetaInf = new File(doctypeDir, 'META-INF').tap { mkdirs() }
+ new File(doctypeMetaInf, 'grails-plugin.xml').text = """<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE plugin>
+<plugin name='doctype'>
+ <type>com.example.DoctypeGrailsPlugin</type>
</plugin>
"""
- def classLoader = new URLClassLoader([tempDir.toURI().toURL()] as URL[], (ClassLoader) null)
+
+ and: 'a second descriptor on the same classpath that declares none'
+ def plainDir = File.createTempDir()
+ def plainMetaInf = new File(plainDir, 'META-INF').tap { mkdirs() }
+ new File(plainMetaInf, 'grails-plugin.xml').text = """<plugin name='plain'>
+ <type>com.example.PlainGrailsPlugin</type>
+</plugin>
+"""
+ def classLoader = new URLClassLoader(
+ [doctypeDir.toURI().toURL(), plainDir.toURI().toURL()] as URL[], (ClassLoader) null)
when: 'plugin descriptor resources are scanned'
def descriptors = PluginUtils.scanPluginDescriptorResources(classLoader)
- then: 'the descriptor is read and the DTD is skipped rather than retrieved'
- descriptors.size() == 1
- with(descriptors[0]) {
- providedPlugins == ['com.example.TestGrailsPlugin']
- providedClasses == ['com.example.MyDomainClass']
- }
+ then: 'the declaration is refused and only the descriptor without one is discovered'
+ descriptors*.providedPlugins == [['com.example.PlainGrailsPlugin']]
cleanup:
- tempDir.deleteDir()
+ doctypeDir.deleteDir()
+ plainDir.deleteDir()
}
def 'ignores malformed plugin descriptor XML without failing discovery'() {
diff --git a/grails-core/src/test/groovy/org/grails/compiler/injection/GlobalGrailsClassInjectorTransformationSpec.groovy b/grails-core/src/test/groovy/org/grails/compiler/injection/GlobalGrailsClassInjectorTransformationSpec.groovy
index d3742d8b53..9e72d5f506 100644
--- a/grails-core/src/test/groovy/org/grails/compiler/injection/GlobalGrailsClassInjectorTransformationSpec.groovy
+++ b/grails-core/src/test/groovy/org/grails/compiler/injection/GlobalGrailsClassInjectorTransformationSpec.groovy
@@ -720,6 +720,45 @@ class GlobalGrailsClassInjectorTransformationSpec extends Specification {
xml.resources.resource*.text() == ['KeptThing']
}
+ void "plugin xml update recreates a descriptor that declares a doctype"() {
+ given:
+ def logCapture = new LogCapture(GlobalGrailsClassInjectorTransformation, Level.WARN)
+
+ and: 'an existing descriptor declaring a doctype, which Grails never generates'
+ def pluginXml = new File(tempDir, 'doctype-plugin.xml')
+ pluginXml.text = '''
+ <!DOCTYPE plugin>
+ <plugin>
+ <resources>
+ <resource>ExistingThing</resource>
+ </resources>
+ </plugin>
+ '''
+
+ when: 'the transformation attempts to update the descriptor'
+ transformation.updatePluginXml(null, null, pluginXml, ['NewThing'])
+
+ then: 'the declaration is refused, so the descriptor is discarded and a warning is logged'
+ !pluginXml.exists()
+ logCapture.events.size() == 1
+ with(logCapture.events[0]) {
+ level == Level.WARN
+ formattedMessage == "Failed to update existing file ${pluginXml.absolutePath}. Recreating it instead..."
+ }
+
+ and: 'the deferred names are written out when the descriptor is next generated'
+ transformation.generatePluginXml(
+ compilePlugin('class DoctypeRecoveredGrailsPlugin {}'),
+ '1.0',
+ [] as Set,
+ pluginXml
+ )
+ new XmlSlurper().parse(pluginXml).resources.resource*.text() == ['NewThing']
+
+ cleanup:
+ logCapture.close()
+ }
+
void "plugin xml update recreates safely when the existing descriptor is malformed"() {
given:
def logCapture = new LogCapture(GlobalGrailsClassInjectorTransformation, Level.WARN)
diff --git a/grails-doc/src/en/guide/upgrading.adoc b/grails-doc/src/en/guide/upgrading.adoc
index 3a0765d804..5e7edb0cd4 100644
--- a/grails-doc/src/en/guide/upgrading.adoc
+++ b/grails-doc/src/en/guide/upgrading.adoc
@@ -39,4 +39,6 @@ How a `DOCTYPE` declaration is treated depends on where the document comes from.
Request bodies are refused if they declare one. An `application/xml`, `text/xml` or `application/hal+xml` request body, or a string passed to `XML.parse`, that carries a `DOCTYPE` is rejected rather than bound: binding fails with the `invalidRequestBody` error code and the target is left unpopulated. This is stricter than blocking external entities, because a body whose `DOCTYPE` is entirely internal and harmless is refused too, and there is no setting that relaxes it. A client that s [...]
-Descriptors that Grails reads from the classpath may declare one. JSP tag library descriptors, `web.xml` and `grails-plugin.xml` are trusted input and routinely carry a `DOCTYPE`; `jakarta.servlet.jsp.jstl` ships several such descriptors, among them `c-1_0-rt.tld`, which the default `grails.gsp.tldScanPattern` scans. These are parsed with the declaration permitted and the entity and DTD hardening above still in force, so the DTD a descriptor names is never retrieved. The same applies [...]
+Descriptors that Grails reads from the classpath may declare one. JSP tag library descriptors and `web.xml` are trusted input and routinely carry a `DOCTYPE`; `jakarta.servlet.jsp.jstl` ships several such descriptors, among them `c-1_0-rt.tld`, which the default `grails.gsp.tldScanPattern` scans. These are parsed with the declaration permitted and the entity and DTD hardening above still in force, so the DTD a descriptor names is never retrieved. The same applies to `response.xml` in [...]
+
+The plugin descriptor `grails-plugin.xml` is the exception among the descriptors. Grails generates it during compilation and never writes a `DOCTYPE`, so both the compiler that rewrites it and the plugin discovery that reads it back use the strict parser. A descriptor that has been hand-edited to add a declaration is refused: at compile time it is discarded and regenerated, and at runtime the plugin it describes is skipped.
diff --git a/grails-doc/src/en/guide/upgrading/upgrading80x.adoc b/grails-doc/src/en/guide/upgrading/upgrading80x.adoc
index 4882dc908a..4ff9c9c208 100644
--- a/grails-doc/src/en/guide/upgrading/upgrading80x.adoc
+++ b/grails-doc/src/en/guide/upgrading/upgrading80x.adoc
@@ -3588,6 +3588,9 @@ a `DOCTYPE` is additionally refused rather than bound, whether or not the declar
Binding such a body fails with the `invalidRequestBody` error code. There is no setting that relaxes this; a client that
sends a declaration needs to omit it.
-Descriptors that Grails reads from the classpath, such as JSP tag library descriptors, `web.xml` and `grails-plugin.xml`,
-and the `response.xml` accessor in controller unit tests are not affected. They may declare a `DOCTYPE`, though the DTD
-it names is never retrieved. See <<upgrading>> for the full description of the XML parsing defaults.
+Descriptors that Grails reads from the classpath, such as JSP tag library descriptors and `web.xml`, and the
+`response.xml` accessor in controller unit tests are not affected. They may declare a `DOCTYPE`, though the DTD it names
+is never retrieved. The plugin descriptor `grails-plugin.xml` is not among them: Grails generates it and never writes a
+declaration, so it is read strictly at compile time and at runtime alike.
+
+See <<upgrading>> for the full description of the XML parsing defaults.
diff --git a/grails-gradle/common/src/main/groovy/org/apache/grails/gradle/common/XmlParserFeature.java b/grails-gradle/common/src/main/groovy/org/apache/grails/gradle/common/XmlParserFeature.java
index 477440aa5d..fb10d909fb 100644
--- a/grails-gradle/common/src/main/groovy/org/apache/grails/gradle/common/XmlParserFeature.java
+++ b/grails-gradle/common/src/main/groovy/org/apache/grails/gradle/common/XmlParserFeature.java
@@ -43,9 +43,9 @@ public enum XmlParserFeature {
*
* <p>Enabling this is stricter than blocking external entities: it refuses documents whose
* DOCTYPE is entirely internal and harmless. It suits untrusted input such as HTTP request
- * bodies. Descriptors read from the classpath — JSP tag library definitions, {@code web.xml},
- * {@code grails-plugin.xml} — routinely carry a DOCTYPE, so their readers need a parser that
- * leaves this disabled while keeping the entity and DTD features below switched off.
+ * bodies. Descriptors read from the classpath — JSP tag library definitions and
+ * {@code web.xml} — routinely carry a DOCTYPE, so their readers need a parser that leaves this
+ * disabled while keeping the entity and DTD features below switched off.
*/
DISALLOW_DOCTYPE_DECL("http://apache.org/xml/features/disallow-doctype-decl"),
diff --git a/grails-gradle/model/src/main/groovy/org/grails/io/support/SpringIOUtils.java b/grails-gradle/model/src/main/groovy/org/grails/io/support/SpringIOUtils.java
index 20ef4ce304..b107c03d06 100644
--- a/grails-gradle/model/src/main/groovy/org/grails/io/support/SpringIOUtils.java
+++ b/grails-gradle/model/src/main/groovy/org/grails/io/support/SpringIOUtils.java
@@ -429,9 +429,8 @@ public class SpringIOUtils {
* <p>Documents parsed through this class fall into two trust levels, and each level gets its
* own parser. Request bodies are untrusted and are refused if they declare a {@code DOCTYPE};
* that is the {@code false} form and the default. Descriptors read from the application
- * classpath, such as JSP tag library descriptors, {@code web.xml} and
- * {@code grails-plugin.xml}, are trusted and routinely declare one, so their readers pass
- * {@code true}.
+ * classpath, such as JSP tag library descriptors and {@code web.xml}, are trusted and routinely
+ * declare one, so their readers pass {@code true}.
*
* <p>Tolerating the declaration does not reopen the XXE vector. External general entities,
* external parameter entities, DTD grammar loading and external DTD retrieval stay off on both