From 55c5bad28e0bb980a34c83e2272b847f1e92edba Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Mon, 31 Aug 2026 15:51:17 +0200 Subject: [PATCH 1/8] Harden XML parsing via commons-secure-xml Create SAX parsers and readers through org.apache.commons:commons-secure-xml. The secure factory enables FEATURE_SECURE_PROCESSING and installs a non-removable entity-resolver floor on every parser it produces: external DTD and entity lookups that a caller-set resolver does not resolve are resolved to empty content instead of being fetched, and internal entity expansion is bounded, regardless of the JAXP implementation on the classpath. Changes: - Add the commons-secure-xml dependency (1.0.0-SNAPSHOT until its first release) to core, jelly-tags/xml and jelly-tags/xmlunit. - core XMLParser keeps the documented JellyContext.setAllowDtdToCallExternalEntities(true) opt-in working by using a plain factory on that path; the default path uses the secure factory, and a factory assigned to the protected static field still wins. The flag-dependent choice is no longer cached in that field. - core ParseTag (which had no hardening at all) now creates its reader through the secure factory. - jelly-tags/xml: TransformTag's readers and ParseTag's dom4j SAXReader are built from the secure factory; dom4j and XMLReaderFactory otherwise provision readers through JAXP at their own defaults, and the deprecated org.xml.sax.driver system property no longer selects the reader class. The TransformerFactory itself stays unsecured for now: Xalan, which this module puts on the class path, drops the attributes of xsl:namespace-alias literal result elements under secure processing (XSLTElementProcessor rejects "foreign" attributes as non-fatal errors), silently breaking stylesheets such as the Schematron skeleton. - jelly-tags/xmlunit: the assertion tags' dom4j SAXReaders are built from the secure factory. - jelly-tags/html is unchanged: NekoHTML is an HTML scanner, not an XML parser. - Run the CI and CodeQL builds with -Puse-apache-snapshots (inherited from the org.apache:apache parent POM) so the commons-secure-xml SNAPSHOT resolves; CodeQL's autobuild receives the profile through MAVEN_ARGS. Assisted-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01MHgnMnGWHQoH2zD2jFdoMT --- .github/workflows/codeql-analysis.yml | 2 ++ .github/workflows/maven.yml | 2 +- core/pom.xml | 5 ++++ .../commons/jelly/parser/XMLParser.java | 16 ++++++++---- .../commons/jelly/tags/core/ParseTag.java | 3 ++- jelly-tags/xml/pom.xml | 5 ++++ .../commons/jelly/tags/xml/ParseTag.java | 13 ++++++++-- .../commons/jelly/tags/xml/TransformTag.java | 25 +++++++++++++++---- jelly-tags/xmlunit/pom.xml | 5 ++++ .../commons/jelly/tags/xmlunit/ActualTag.java | 11 +++++++- .../tags/xmlunit/AssertDocumentsEqualTag.java | 11 +++++++- .../jelly/tags/xmlunit/ExpectedTag.java | 11 +++++++- src/changes/changes.xml | 1 + 13 files changed, 93 insertions(+), 17 deletions(-) diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index eaf7ff522..9a2a53644 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -70,6 +70,8 @@ jobs: # If this step fails, then you should remove it and run the build manually (see below) - name: Autobuild uses: github/codeql-action/autobuild@cdf488f595d80d6e07e03d4674febd5ab45fa938 # v4.37.9 + env: + MAVEN_ARGS: -Puse-apache-snapshots # ℹī¸ Command-line programs to run using the OS shell. # 📚 https://git.io/JvXDl diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index dc488cbfd..5792ecf56 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -50,4 +50,4 @@ jobs: distribution: 'temurin' java-version: ${{ matrix.java }} - name: Build with Maven - run: mvn --errors --show-version --batch-mode --no-transfer-progress -Ddoclint=none -Darguments=-Xdoclint:none -Dcommons.javadoc.failOnWarnings=false + run: mvn --errors --show-version --batch-mode --no-transfer-progress -Ddoclint=none -Darguments=-Xdoclint:none -Dcommons.javadoc.failOnWarnings=false -Puse-apache-snapshots diff --git a/core/pom.xml b/core/pom.xml index ed6b76214..3aa770e17 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -52,6 +52,11 @@ + + org.apache.commons + commons-secure-xml + 1.0.0-SNAPSHOT + javax.servlet servlet-api diff --git a/core/src/main/java/org/apache/commons/jelly/parser/XMLParser.java b/core/src/main/java/org/apache/commons/jelly/parser/XMLParser.java index 7d076cd0c..31d870806 100644 --- a/core/src/main/java/org/apache/commons/jelly/parser/XMLParser.java +++ b/core/src/main/java/org/apache/commons/jelly/parser/XMLParser.java @@ -51,6 +51,7 @@ import org.apache.commons.jelly.util.ClassLoaderUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.apache.commons.xml.secure.SecureSAXParserFactory; import org.xml.sax.Attributes; import org.xml.sax.ErrorHandler; import org.xml.sax.InputSource; @@ -810,12 +811,17 @@ public SAXParser getParser() { // Create and return a new parser synchronized (this) { try { - if (factory == null) { - factory = SAXParserFactory.newInstance(); + SAXParserFactory parserFactory = factory; + if (parserFactory == null) { + // The secure factory's resolver floor would ignore external entities, so the + // documented opt-in keeps using a plain factory; do not cache the per-instance choice. + parserFactory = allowDtdToCallExternalEntities + ? SAXParserFactory.newInstance() + : SecureSAXParserFactory.newInstance(); } - factory.setNamespaceAware(true); - factory.setValidating(validating); - parser = factory.newSAXParser(); + parserFactory.setNamespaceAware(true); + parserFactory.setValidating(validating); + parser = parserFactory.newSAXParser(); return parser; } catch (final Exception e) { diff --git a/core/src/main/java/org/apache/commons/jelly/tags/core/ParseTag.java b/core/src/main/java/org/apache/commons/jelly/tags/core/ParseTag.java index c7f263333..d2fecb593 100644 --- a/core/src/main/java/org/apache/commons/jelly/tags/core/ParseTag.java +++ b/core/src/main/java/org/apache/commons/jelly/tags/core/ParseTag.java @@ -30,6 +30,7 @@ import org.apache.commons.jelly.parser.XMLParser; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.apache.commons.xml.secure.SecureSAXParserFactory; import org.xml.sax.ContentHandler; import org.xml.sax.InputSource; import org.xml.sax.SAXException; @@ -76,7 +77,7 @@ protected XMLParser createJellyParser() { * Factory method to create a new XMLReader */ protected XMLReader createXMLReader() throws ParserConfigurationException, SAXException { - final SAXParserFactory factory = SAXParserFactory.newInstance(); + final SAXParserFactory factory = SecureSAXParserFactory.newInstance(); factory.setNamespaceAware(true); final SAXParser parser = factory.newSAXParser(); return parser.getXMLReader(); diff --git a/jelly-tags/xml/pom.xml b/jelly-tags/xml/pom.xml index 507d57835..17102fbc5 100644 --- a/jelly-tags/xml/pom.xml +++ b/jelly-tags/xml/pom.xml @@ -31,6 +31,11 @@ The Jelly XML Tag Library. + + org.apache.commons + commons-secure-xml + 1.0.0-SNAPSHOT + commons-jelly commons-jelly diff --git a/jelly-tags/xml/src/main/java/org/apache/commons/jelly/tags/xml/ParseTag.java b/jelly-tags/xml/src/main/java/org/apache/commons/jelly/tags/xml/ParseTag.java index b701d5457..a861da5b0 100644 --- a/jelly-tags/xml/src/main/java/org/apache/commons/jelly/tags/xml/ParseTag.java +++ b/jelly-tags/xml/src/main/java/org/apache/commons/jelly/tags/xml/ParseTag.java @@ -16,13 +16,17 @@ */ package org.apache.commons.jelly.tags.xml; +import javax.xml.parsers.ParserConfigurationException; + import org.apache.commons.jelly.JellyTagException; import org.apache.commons.jelly.MissingAttributeException; import org.apache.commons.jelly.XMLOutput; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.apache.commons.xml.secure.SecureSAXParserFactory; import org.dom4j.Document; import org.dom4j.io.SAXReader; +import org.xml.sax.SAXException; /** A tag which parses some XML and defines a variable with the parsed Document. * The XML can either be specified as its body or can be passed in via the @@ -47,8 +51,13 @@ public ParseTag() { * Factory method to create a new SAXReader */ @Override - protected SAXReader createSAXReader() { - return new SAXReader(validate); + protected SAXReader createSAXReader() throws SAXException { + // dom4j builds its reader through JAXP internally; hand it one from the secure factory instead. + try { + return new SAXReader(SecureSAXParserFactory.newNSInstance().newSAXParser().getXMLReader(), validate); + } catch (final ParserConfigurationException e) { + throw new SAXException(e); + } } @Override diff --git a/jelly-tags/xml/src/main/java/org/apache/commons/jelly/tags/xml/TransformTag.java b/jelly-tags/xml/src/main/java/org/apache/commons/jelly/tags/xml/TransformTag.java index 348fe06c0..cc96a0712 100644 --- a/jelly-tags/xml/src/main/java/org/apache/commons/jelly/tags/xml/TransformTag.java +++ b/jelly-tags/xml/src/main/java/org/apache/commons/jelly/tags/xml/TransformTag.java @@ -27,6 +27,7 @@ import java.util.Iterator; import java.util.List; +import javax.xml.parsers.ParserConfigurationException; import javax.xml.transform.Result; import javax.xml.transform.TransformerConfigurationException; import javax.xml.transform.TransformerFactory; @@ -48,6 +49,7 @@ import org.apache.commons.jelly.impl.TagScript; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; +import org.apache.commons.xml.secure.SecureSAXParserFactory; import org.dom4j.Document; import org.dom4j.io.DocumentResult; import org.dom4j.io.DocumentSource; @@ -61,7 +63,6 @@ import org.xml.sax.SAXNotSupportedException; import org.xml.sax.XMLReader; import org.xml.sax.ext.LexicalHandler; -import org.xml.sax.helpers.XMLReaderFactory; /** A tag which parses some XML, applies an xslt transform to it * and defines a variable with the transformed Document. @@ -129,7 +130,7 @@ public TagBodyXMLReader(final Tag tag) private void doInvokeBody() throws SAXException { try { if (this.shouldParseBody()) { - final XMLReader anXMLReader = XMLReaderFactory.createXMLReader(); + final XMLReader anXMLReader = newSecureXMLReader(); anXMLReader.setContentHandler(this.xmlOutput); anXMLReader.setProperty(LEXICAL_HANDLER_PROPERTY,this.xmlOutput); final StringWriter writer = new StringWriter(); @@ -408,6 +409,10 @@ private boolean shouldParseBody() throws JellyTagException { * Constructor for TransformTag. */ public TransformTag() { + // Not the secure factory: Xalan (on the class path here) drops the attributes of + // xsl:namespace-alias literal result elements under secure processing (XSLTElementProcessor + // rejects "foreign" attributes), silently breaking stylesheets like the Schematron skeleton. + // The stylesheet is part of the script; the readers parsing the transform INPUT are secured. this.tf = (SAXTransformerFactory) TransformerFactory.newInstance(); } @@ -462,9 +467,19 @@ protected URIResolver createURIResolver() { * * @return XMLReader for the transform input * @throws SAXException - * If the value of the "org.xml.sax.driver" system property - * is null, or if the class cannot be loaded and instantiated. + * If the reader cannot be created. */ + /** + * Creates a namespace-aware XMLReader through the secure SAX parser factory. + */ + private static XMLReader newSecureXMLReader() throws SAXException { + try { + return SecureSAXParserFactory.newNSInstance().newSAXParser().getXMLReader(); + } catch (final ParserConfigurationException e) { + throw new SAXException(e); + } + } + protected XMLReader createXMLReader() throws SAXException { XMLReader xmlReader = null; final Object xmlReaderSourceObj = this.getXml(); @@ -474,7 +489,7 @@ protected XMLReader createXMLReader() throws SAXException { xmlReader = new TagBodyXMLReader(this); } else { - xmlReader = XMLReaderFactory.createXMLReader(); + xmlReader = newSecureXMLReader(); } return xmlReader; diff --git a/jelly-tags/xmlunit/pom.xml b/jelly-tags/xmlunit/pom.xml index f578cd108..e86f2a5b0 100644 --- a/jelly-tags/xmlunit/pom.xml +++ b/jelly-tags/xmlunit/pom.xml @@ -31,6 +31,11 @@ + + org.apache.commons + commons-secure-xml + 1.0.0-SNAPSHOT + xmlunit xmlunit diff --git a/jelly-tags/xmlunit/src/main/java/org/apache/commons/jelly/tags/xmlunit/ActualTag.java b/jelly-tags/xmlunit/src/main/java/org/apache/commons/jelly/tags/xmlunit/ActualTag.java index 6bcab4348..383f8c732 100644 --- a/jelly-tags/xmlunit/src/main/java/org/apache/commons/jelly/tags/xmlunit/ActualTag.java +++ b/jelly-tags/xmlunit/src/main/java/org/apache/commons/jelly/tags/xmlunit/ActualTag.java @@ -17,16 +17,25 @@ package org.apache.commons.jelly.tags.xmlunit; +import javax.xml.parsers.ParserConfigurationException; + import org.apache.commons.jelly.JellyTagException; import org.apache.commons.jelly.XMLOutput; +import org.apache.commons.xml.secure.SecureSAXParserFactory; import org.dom4j.Document; import org.dom4j.io.SAXReader; +import org.xml.sax.SAXException; public class ActualTag extends XMLUnitTagSupport { @Override protected SAXReader createSAXReader() { - return new SAXReader(); + // dom4j builds its reader through JAXP internally; hand it one from the secure factory instead. + try { + return new SAXReader(SecureSAXParserFactory.newNSInstance().newSAXParser().getXMLReader()); + } catch (final ParserConfigurationException | SAXException e) { + throw new IllegalStateException("Unable to create a new XML reader", e); + } } @Override diff --git a/jelly-tags/xmlunit/src/main/java/org/apache/commons/jelly/tags/xmlunit/AssertDocumentsEqualTag.java b/jelly-tags/xmlunit/src/main/java/org/apache/commons/jelly/tags/xmlunit/AssertDocumentsEqualTag.java index 1097b80ab..a1a93c42d 100644 --- a/jelly-tags/xmlunit/src/main/java/org/apache/commons/jelly/tags/xmlunit/AssertDocumentsEqualTag.java +++ b/jelly-tags/xmlunit/src/main/java/org/apache/commons/jelly/tags/xmlunit/AssertDocumentsEqualTag.java @@ -17,12 +17,16 @@ package org.apache.commons.jelly.tags.xmlunit; +import javax.xml.parsers.ParserConfigurationException; + import org.apache.commons.jelly.JellyTagException; import org.apache.commons.jelly.XMLOutput; +import org.apache.commons.xml.secure.SecureSAXParserFactory; import org.custommonkey.xmlunit.Diff; import org.custommonkey.xmlunit.XMLUnit; import org.dom4j.Document; import org.dom4j.io.SAXReader; +import org.xml.sax.SAXException; /** * Compares two XML documents using XMLUnit (http://xmlunit.sourceforge.net/). @@ -46,7 +50,12 @@ public class AssertDocumentsEqualTag extends XMLUnitTagSupport { @Override protected SAXReader createSAXReader() { - return new SAXReader(); + // dom4j builds its reader through JAXP internally; hand it one from the secure factory instead. + try { + return new SAXReader(SecureSAXParserFactory.newNSInstance().newSAXParser().getXMLReader()); + } catch (final ParserConfigurationException | SAXException e) { + throw new IllegalStateException("Unable to create a new XML reader", e); + } } @Override diff --git a/jelly-tags/xmlunit/src/main/java/org/apache/commons/jelly/tags/xmlunit/ExpectedTag.java b/jelly-tags/xmlunit/src/main/java/org/apache/commons/jelly/tags/xmlunit/ExpectedTag.java index ffd2ae103..0913c1492 100644 --- a/jelly-tags/xmlunit/src/main/java/org/apache/commons/jelly/tags/xmlunit/ExpectedTag.java +++ b/jelly-tags/xmlunit/src/main/java/org/apache/commons/jelly/tags/xmlunit/ExpectedTag.java @@ -17,16 +17,25 @@ package org.apache.commons.jelly.tags.xmlunit; +import javax.xml.parsers.ParserConfigurationException; + import org.apache.commons.jelly.JellyTagException; import org.apache.commons.jelly.XMLOutput; +import org.apache.commons.xml.secure.SecureSAXParserFactory; import org.dom4j.Document; import org.dom4j.io.SAXReader; +import org.xml.sax.SAXException; public class ExpectedTag extends XMLUnitTagSupport { @Override protected SAXReader createSAXReader() { - return new SAXReader(); + // dom4j builds its reader through JAXP internally; hand it one from the secure factory instead. + try { + return new SAXReader(SecureSAXParserFactory.newNSInstance().newSAXParser().getXMLReader()); + } catch (final ParserConfigurationException | SAXException e) { + throw new IllegalStateException("Unable to create a new XML reader", e); + } } @Override diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 7b3382edf..022f71547 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -48,6 +48,7 @@ The type attribute can be add,update,fix,remove. Extract commons version into new POM property ${mx4j.version}. + Create XML parsers and readers through org.apache.commons:commons-secure-xml, so external entities and DTDs are no longer fetched unless JellyContext.setAllowDtdToCallExternalEntities(true) is used. Throw specialized RuntimeExceptions instead of plain RuntimeExceptions. Deprecate NestedRuntimeException for RuntimeException. Fix building on modern Java. From 6bcde77fc5f63bc5ecb2731d193b00437b05d276 Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Thu, 3 Sep 2026 07:04:05 +0200 Subject: [PATCH 2/8] Use the Commons Secure XML 1.0.0 release candidate Bump org.apache.commons:commons-secure-xml from 1.0.0-SNAPSHOT to 1.0.0 and add the temporary staging repository https://repository.apache.org/content/repositories/orgapachecommons-1962/ after Central, so the vote gets downstream CI results. Drop the -Puse-apache-snapshots profile from the CI workflows, which the release version no longer needs. Remove the staging repository once 1.0.0 is released. Assisted-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_0167e29ScPEdfzJnEFm95imK --- .github/workflows/codeql-analysis.yml | 2 -- .github/workflows/maven.yml | 2 +- core/pom.xml | 2 +- jelly-tags/xml/pom.xml | 2 +- jelly-tags/xmlunit/pom.xml | 2 +- pom.xml | 21 +++++++++++++++++++++ 6 files changed, 25 insertions(+), 6 deletions(-) diff --git a/.github/workflows/codeql-analysis.yml b/.github/workflows/codeql-analysis.yml index 9a2a53644..eaf7ff522 100644 --- a/.github/workflows/codeql-analysis.yml +++ b/.github/workflows/codeql-analysis.yml @@ -70,8 +70,6 @@ jobs: # If this step fails, then you should remove it and run the build manually (see below) - name: Autobuild uses: github/codeql-action/autobuild@cdf488f595d80d6e07e03d4674febd5ab45fa938 # v4.37.9 - env: - MAVEN_ARGS: -Puse-apache-snapshots # ℹī¸ Command-line programs to run using the OS shell. # 📚 https://git.io/JvXDl diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml index 5792ecf56..dc488cbfd 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -50,4 +50,4 @@ jobs: distribution: 'temurin' java-version: ${{ matrix.java }} - name: Build with Maven - run: mvn --errors --show-version --batch-mode --no-transfer-progress -Ddoclint=none -Darguments=-Xdoclint:none -Dcommons.javadoc.failOnWarnings=false -Puse-apache-snapshots + run: mvn --errors --show-version --batch-mode --no-transfer-progress -Ddoclint=none -Darguments=-Xdoclint:none -Dcommons.javadoc.failOnWarnings=false diff --git a/core/pom.xml b/core/pom.xml index 3aa770e17..1b96e528b 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -55,7 +55,7 @@ org.apache.commons commons-secure-xml - 1.0.0-SNAPSHOT + 1.0.0 javax.servlet diff --git a/jelly-tags/xml/pom.xml b/jelly-tags/xml/pom.xml index 17102fbc5..e92b8af98 100644 --- a/jelly-tags/xml/pom.xml +++ b/jelly-tags/xml/pom.xml @@ -34,7 +34,7 @@ org.apache.commons commons-secure-xml - 1.0.0-SNAPSHOT + 1.0.0 commons-jelly diff --git a/jelly-tags/xmlunit/pom.xml b/jelly-tags/xmlunit/pom.xml index e86f2a5b0..2159eb205 100644 --- a/jelly-tags/xmlunit/pom.xml +++ b/jelly-tags/xmlunit/pom.xml @@ -34,7 +34,7 @@ org.apache.commons commons-secure-xml - 1.0.0-SNAPSHOT + 1.0.0 xmlunit diff --git a/pom.xml b/pom.xml index b186102c2..1a8082a4e 100644 --- a/pom.xml +++ b/pom.xml @@ -152,6 +152,27 @@ scp://people.apache.org/www/commons.apache.org/${pom.artifactId.substring(8)}/ + + + + central + Central Repository + https://repo.maven.apache.org/maven2 + + false + + + + + apache.commons.staging + Apache Commons Secure XML 1.0.0 release candidate + https://repository.apache.org/content/repositories/orgapachecommons-1962/ + + false + + + + javax.servlet From 9c0e5110e3005d3605a6427efa478461483864e7 Mon Sep 17 00:00:00 2001 From: Gary Gregory Date: Sun, 6 Sep 2026 08:39:43 -0400 Subject: [PATCH 3/8] Bump Apache Commons Secure XML from 1.0.0-SNAPSHOT to 1.0.0 --- core/pom.xml | 1 - jelly-tags/xml/pom.xml | 1 - jelly-tags/xmlunit/pom.xml | 1 - pom.xml | 30 +++++++++--------------------- src/changes/changes.xml | 2 +- 5 files changed, 10 insertions(+), 25 deletions(-) diff --git a/core/pom.xml b/core/pom.xml index 1b96e528b..dc2f8768d 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -55,7 +55,6 @@ org.apache.commons commons-secure-xml - 1.0.0 javax.servlet diff --git a/jelly-tags/xml/pom.xml b/jelly-tags/xml/pom.xml index e92b8af98..ac1d1ddbc 100644 --- a/jelly-tags/xml/pom.xml +++ b/jelly-tags/xml/pom.xml @@ -34,7 +34,6 @@ org.apache.commons commons-secure-xml - 1.0.0 commons-jelly diff --git a/jelly-tags/xmlunit/pom.xml b/jelly-tags/xmlunit/pom.xml index 2159eb205..2dd847ba0 100644 --- a/jelly-tags/xmlunit/pom.xml +++ b/jelly-tags/xmlunit/pom.xml @@ -34,7 +34,6 @@ org.apache.commons commons-secure-xml - 1.0.0 xmlunit diff --git a/pom.xml b/pom.xml index 1a8082a4e..02c3be108 100644 --- a/pom.xml +++ b/pom.xml @@ -152,27 +152,15 @@ scp://people.apache.org/www/commons.apache.org/${pom.artifactId.substring(8)}/ - - - - central - Central Repository - https://repo.maven.apache.org/maven2 - - false - - - - - apache.commons.staging - Apache Commons Secure XML 1.0.0 release candidate - https://repository.apache.org/content/repositories/orgapachecommons-1962/ - - false - - - - + + + + org.apache.commons + commons-secure-xml + 1.0.0 + + + javax.servlet diff --git a/src/changes/changes.xml b/src/changes/changes.xml index 022f71547..5c2b3ffe2 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -48,7 +48,7 @@ The type attribute can be add,update,fix,remove. Extract commons version into new POM property ${mx4j.version}. - Create XML parsers and readers through org.apache.commons:commons-secure-xml, so external entities and DTDs are no longer fetched unless JellyContext.setAllowDtdToCallExternalEntities(true) is used. + Create XML parsers and readers through org.apache.commons:commons-secure-xml, so external entities and DTDs are no longer fetched unless JellyContext.setAllowDtdToCallExternalEntities(true) is used. Throw specialized RuntimeExceptions instead of plain RuntimeExceptions. Deprecate NestedRuntimeException for RuntimeException. Fix building on modern Java. From 4c8213b55e5265d58697122aabf467d049f73281 Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Sun, 6 Sep 2026 21:51:22 +0200 Subject: [PATCH 4/8] fix: use allow-all resolver for `allowDtdToCallExternalEntities` --- .../apache/commons/jelly/parser/XMLParser.java | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/core/src/main/java/org/apache/commons/jelly/parser/XMLParser.java b/core/src/main/java/org/apache/commons/jelly/parser/XMLParser.java index 31d870806..4e837cab3 100644 --- a/core/src/main/java/org/apache/commons/jelly/parser/XMLParser.java +++ b/core/src/main/java/org/apache/commons/jelly/parser/XMLParser.java @@ -53,6 +53,7 @@ import org.apache.commons.logging.LogFactory; import org.apache.commons.xml.secure.SecureSAXParserFactory; import org.xml.sax.Attributes; +import org.xml.sax.EntityResolver; import org.xml.sax.ErrorHandler; import org.xml.sax.InputSource; import org.xml.sax.Locator; @@ -77,6 +78,8 @@ public class XMLParser extends DefaultHandler { */ protected static SAXParserFactory factory = null; + private static final EntityResolver ALLOW_ALL_RESOLVER = (publicId, systemId) -> new InputSource(systemId); + /** JellyContext which is used to locate tag libraries*/ private JellyContext context = new JellyContext(); @@ -813,11 +816,7 @@ public SAXParser getParser() { try { SAXParserFactory parserFactory = factory; if (parserFactory == null) { - // The secure factory's resolver floor would ignore external entities, so the - // documented opt-in keeps using a plain factory; do not cache the per-instance choice. - parserFactory = allowDtdToCallExternalEntities - ? SAXParserFactory.newInstance() - : SecureSAXParserFactory.newInstance(); + parserFactory = SecureSAXParserFactory.newInstance(); } parserFactory.setNamespaceAware(true); parserFactory.setValidating(validating); @@ -882,10 +881,8 @@ public boolean getValidating() { public synchronized XMLReader getXMLReader() throws SAXException { if (reader == null) { reader = getParser().getXMLReader(); - if (!allowDtdToCallExternalEntities) { - reader.setFeature("http://xml.org/sax/features/external-general-entities", false); - reader.setFeature("http://xml.org/sax/features/external-parameter-entities", false); - reader.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false); + if (isAllowDtdToCallExternalEntities()) { + reader.setEntityResolver(ALLOW_ALL_RESOLVER); } if (this.defaultNamespaceURI != null) { reader = new DefaultNamespaceFilter(this.defaultNamespaceURI, reader); From e5592064c720dccb327a84c0090bfe1d215a727e Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Sun, 6 Sep 2026 22:01:50 +0200 Subject: [PATCH 5/8] fix: simplify getParser() --- .../org/apache/commons/jelly/parser/XMLParser.java | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/core/src/main/java/org/apache/commons/jelly/parser/XMLParser.java b/core/src/main/java/org/apache/commons/jelly/parser/XMLParser.java index 4e837cab3..5cf4a61d6 100644 --- a/core/src/main/java/org/apache/commons/jelly/parser/XMLParser.java +++ b/core/src/main/java/org/apache/commons/jelly/parser/XMLParser.java @@ -814,13 +814,12 @@ public SAXParser getParser() { // Create and return a new parser synchronized (this) { try { - SAXParserFactory parserFactory = factory; - if (parserFactory == null) { - parserFactory = SecureSAXParserFactory.newInstance(); + if (factory == null) { + factory = SecureSAXParserFactory.newInstance(); } - parserFactory.setNamespaceAware(true); - parserFactory.setValidating(validating); - parser = parserFactory.newSAXParser(); + factory.setNamespaceAware(true); + factory.setValidating(validating); + parser = factory.newSAXParser(); return parser; } catch (final Exception e) { From 3c49192efc45cc55c7ac521bc7515d21d723aa16 Mon Sep 17 00:00:00 2001 From: Gary Gregory Date: Mon, 7 Sep 2026 17:37:19 -0400 Subject: [PATCH 6/8] Update XMLReader factory method to use newNSInstance --- .../main/java/org/apache/commons/jelly/tags/core/ParseTag.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/src/main/java/org/apache/commons/jelly/tags/core/ParseTag.java b/core/src/main/java/org/apache/commons/jelly/tags/core/ParseTag.java index d2fecb593..80148a691 100644 --- a/core/src/main/java/org/apache/commons/jelly/tags/core/ParseTag.java +++ b/core/src/main/java/org/apache/commons/jelly/tags/core/ParseTag.java @@ -77,7 +77,7 @@ protected XMLParser createJellyParser() { * Factory method to create a new XMLReader */ protected XMLReader createXMLReader() throws ParserConfigurationException, SAXException { - final SAXParserFactory factory = SecureSAXParserFactory.newInstance(); + final SAXParserFactory factory = SecureSAXParserFactory.newNSInstance(); factory.setNamespaceAware(true); final SAXParser parser = factory.newSAXParser(); return parser.getXMLReader(); From be2e1f1732451435dcf4291247ecdc00bc108571 Mon Sep 17 00:00:00 2001 From: Gary Gregory Date: Mon, 7 Sep 2026 17:37:38 -0400 Subject: [PATCH 7/8] Remove namespace awareness from XMLReader creation Removed namespace awareness setting from XMLReader factory method. --- .../main/java/org/apache/commons/jelly/tags/core/ParseTag.java | 1 - 1 file changed, 1 deletion(-) diff --git a/core/src/main/java/org/apache/commons/jelly/tags/core/ParseTag.java b/core/src/main/java/org/apache/commons/jelly/tags/core/ParseTag.java index 80148a691..0bddc9f02 100644 --- a/core/src/main/java/org/apache/commons/jelly/tags/core/ParseTag.java +++ b/core/src/main/java/org/apache/commons/jelly/tags/core/ParseTag.java @@ -78,7 +78,6 @@ protected XMLParser createJellyParser() { */ protected XMLReader createXMLReader() throws ParserConfigurationException, SAXException { final SAXParserFactory factory = SecureSAXParserFactory.newNSInstance(); - factory.setNamespaceAware(true); final SAXParser parser = factory.newSAXParser(); return parser.getXMLReader(); } From aef9d98d2a40315c4ec63bb94a2b41b7f4db0f4c Mon Sep 17 00:00:00 2001 From: Gary Gregory Date: Mon, 7 Sep 2026 17:39:10 -0400 Subject: [PATCH 8/8] Update XMLParser to use newNSInstance for factory --- .../main/java/org/apache/commons/jelly/parser/XMLParser.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/core/src/main/java/org/apache/commons/jelly/parser/XMLParser.java b/core/src/main/java/org/apache/commons/jelly/parser/XMLParser.java index 5cf4a61d6..dfda9d33a 100644 --- a/core/src/main/java/org/apache/commons/jelly/parser/XMLParser.java +++ b/core/src/main/java/org/apache/commons/jelly/parser/XMLParser.java @@ -815,9 +815,8 @@ public SAXParser getParser() { synchronized (this) { try { if (factory == null) { - factory = SecureSAXParserFactory.newInstance(); + factory = SecureSAXParserFactory.newNSInstance(); } - factory.setNamespaceAware(true); factory.setValidating(validating); parser = factory.newSAXParser(); return parser;