Copilot commented on code in PR #16310:
URL: https://github.com/apache/grails-core/pull/16310#discussion_r3931685429
##########
grails-gradle/model/src/main/groovy/org/grails/io/support/SpringIOUtils.java:
##########
@@ -423,19 +423,24 @@ private static SAXParserFactory createParserFactory()
throws ParserConfiguration
saxParserFactory = FactorySupport.createSaxParserFactory();
saxParserFactory.setNamespaceAware(true);
saxParserFactory.setValidating(false);
+ try {
+ saxParserFactory.setXIncludeAware(false);
+ } catch (UnsupportedOperationException e) {
+ // ignore, parser doesn't support
+ }
try {
-
saxParserFactory.setFeature("https://apache.org/xml/features/disallow-doctype-decl";,
false);
+
saxParserFactory.setFeature("http://apache.org/xml/features/disallow-doctype-decl";,
true);
Review Comment:
The DOCTYPE setting still fails open: `setFeature` is allowed to throw when
a configured SAX provider does not recognize or support this Xerces feature,
and this catch then returns a factory that can accept DOCTYPEs. That conflicts
with the new rejection guarantee and recreates the hidden-hardening-failure
pattern this change is meant to fix. Treat this feature as mandatory by
clearing the cached factory and propagating a `ParserConfigurationException`;
the other defense-in-depth settings can remain best-effort.
##########
grails-testing-support-http-client/src/main/groovy/org/apache/grails/testing/http/client/utils/XmlUtils.groovy:
##########
@@ -59,7 +59,7 @@ class XmlUtils {
private static final Pattern XML_DECLARATION = ~/^\s*(<\?xml\b.*?\?>)/
private static final Map<String, Boolean> SECURE_XML_SLURPER_FEATURES = [
- (DISALLOW_DOCTYPE_DECL): false,
+ (DISALLOW_DOCTYPE_DECL): true,
Review Comment:
Putting the mandatory DOCTYPE feature in the same best-effort map as
optional hardening still permits DOCTYPEs whenever the selected SAX provider
rejects this feature, because the exception is swallowed below. Since this
method now promises a secure slurper that rejects DOCTYPEs, configure this
feature separately and let failure propagate to the existing
`IllegalStateException` wrapper.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
