jamesfredley commented on code in PR #16331:
URL: https://github.com/apache/grails-core/pull/16331#discussion_r3970744555
##########
grails-gradle/model/src/main/groovy/org/grails/io/support/SpringIOUtils.java:
##########
@@ -416,45 +419,86 @@ public static SAXParser newSAXParser() throws
ParserConfigurationException, SAXE
return factory.newSAXParser();
}
- private static SAXParserFactory saxParserFactory = null;
+ /**
+ * Configuration key permitting {@code DOCTYPE} declarations in documents
parsed by this class.
+ *
+ * <p>Parsers handed out here reject a {@code DOCTYPE} by default. Set
+ * {@code grails.xml.allowDocTypeDeclaration} to {@code true} in {@code
application.yml}, or as
+ * a system property, to accept one.
+ *
+ * <p>Opting in does not reopen the XXE vector. External general entities,
external parameter
+ * entities and external DTDs stay refused whichever way this is set, so
an entity pointing at
+ * a file on disk still contributes nothing. What opting in changes is
only whether a document
+ * carrying a declaration is refused outright.
+ *
+ * <p>It exists because these parsers also read trusted descriptors from
the classpath, and
+ * some of those carry a {@code DOCTYPE}. JSP tag library descriptors are
the common case:
+ * {@code jakarta.servlet.jsp.jstl} ships several, among them {@code
c-1_0-rt.tld}, which the
+ * default {@code grails.gsp.tldScanPattern} scans.
+ */
+ public static final String ALLOW_DOCTYPE_DECLARATION =
"grails.xml.allowDocTypeDeclaration";
- private static SAXParserFactory createParserFactory() throws
ParserConfigurationException {
- if (saxParserFactory == null) {
- saxParserFactory = FactorySupport.createSaxParserFactory();
- saxParserFactory.setNamespaceAware(true);
- saxParserFactory.setValidating(false);
+ /**
+ * Parser features switched off for every parser this class hands out.
+ *
+ * <p>{@link XmlParserFeature#DISALLOW_DOCTYPE_DECL} is handled separately
because it is the
+ * one feature an application may turn off; see {@link
#ALLOW_DOCTYPE_DECLARATION}.
+ */
+ private static final XmlParserFeature[] DISABLED_PARSER_FEATURES = {
+ XmlParserFeature.EXTERNAL_GENERAL_ENTITIES,
+ XmlParserFeature.EXTERNAL_PARAMETER_ENTITIES,
+ XmlParserFeature.LOAD_DTD_GRAMMAR,
+ XmlParserFeature.LOAD_EXTERNAL_DTD
+ };
- try {
-
saxParserFactory.setFeature("https://apache.org/xml/features/disallow-doctype-decl";,
false);
- } catch (Exception pce) {
- // ignore, parser doesn't support
- }
- try {
-
saxParserFactory.setFeature("https://xml.org/sax/features/external-general-entities";,
false);
- } catch (Exception pce) {
- // ignore, parser doesn't support
- }
- try {
-
saxParserFactory.setFeature("https://xml.org/sax/features/external-parameter-entities";,
false);
- } catch (Exception pce) {
- // ignore, parser doesn't support
- }
- try {
-
saxParserFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
- } catch (Exception e) {
- // ignore, parser doesn't support
- }
- try {
-
saxParserFactory.setFeature("https://apache.org/xml/features/nonvalidating/load-dtd-grammar";,
false);
- } catch (Exception e) {
- // ignore, parser doesn't support
+ private static SAXParserFactory strictParserFactory = null;
+
+ private static SAXParserFactory docTypeParserFactory = null;
+
+ private static SAXParserFactory createParserFactory() throws
ParserConfigurationException {
+ if (isDocTypeDeclarationAllowed()) {
+ if (docTypeParserFactory == null) {
+ docTypeParserFactory = buildParserFactory(true);
}
+ return docTypeParserFactory;
+ }
+ if (strictParserFactory == null) {
+ strictParserFactory = buildParserFactory(false);
Review Comment:
Thanks. This lazy-initialization pattern is pre-existing; the PR only splits
the factory into two fields. It does not introduce a publication regression, so
we are treating this خدمت as outside this change.
--
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]
