From 915bc09142e6789272c875e5b5bf975bfc6ba251 Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Mon, 31 Aug 2026 15:25:33 +0200 Subject: [PATCH 1/3] Harden XML parsing via commons-secure-xml Create XML parsers and transformers through org.apache.commons:commons-secure-xml. The secure factories enable FEATURE_SECURE_PROCESSING and install a non-removable entity-resolver floor on every parser they produce: external DTD, entity, schema and XInclude 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). - Route factory creation through SecureDocumentBuilderFactory in DOMParser and SecureTransformerFactory in XMLDocumentContainer; the caller-configurable factory settings (validation, namespace awareness, entity expansion, whitespace, comments, coalescing) keep working. - JDOMParser builds its SAX reader through the secure factory as well, by overriding SAXBuilder.createParser(); documents with internal DTD subsets parse as before. - Parsers registered through DocumentContainer.registerXMLParser remain under the control of their authors. - 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 +- pom.xml | 5 +++++ src/changes/changes.xml | 1 + .../commons/jxpath/XMLDocumentContainer.java | 4 ++-- .../apache/commons/jxpath/xml/DOMParser.java | 3 ++- .../apache/commons/jxpath/xml/JDOMParser.java | 19 ++++++++++++++++++- 7 files changed, 31 insertions(+), 5 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 3f8896d46..b2d1de81a 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -59,4 +59,4 @@ jobs: distribution: ${{ runner.os == 'macOS' && matrix.java == '8' && 'zulu' || 'temurin' }} java-version: ${{ matrix.java }} - name: Build with Maven - run: mvn --errors --show-version --batch-mode --no-transfer-progress -DtrimStackTrace=false + run: mvn --errors --show-version --batch-mode --no-transfer-progress -DtrimStackTrace=false -Puse-apache-snapshots diff --git a/pom.xml b/pom.xml index 7d5d90798..25a425668 100644 --- a/pom.xml +++ b/pom.xml @@ -149,6 +149,11 @@ + + org.apache.commons + commons-secure-xml + 1.0.0-SNAPSHOT + javax.servlet servlet-api diff --git a/src/changes/changes.xml b/src/changes/changes.xml index cc310276b..a546dec09 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -49,6 +49,7 @@ The type attribute can be add,update,fix,remove. + Create the DOM and JDOM parsers and the XML transformer through org.apache.commons:commons-secure-xml, so external entities and DTDs are no longer fetched by default. POM assembly:single does not generate binary convenience files (tar/zip). Make dynamicPropertyHandlerMap in ValueUtils thread-safe #251. Refactor JXPathIntrospector internal static maps to use concurrent classes instead of synchronization. diff --git a/src/main/java/org/apache/commons/jxpath/XMLDocumentContainer.java b/src/main/java/org/apache/commons/jxpath/XMLDocumentContainer.java index cb09f657b..c8001586a 100644 --- a/src/main/java/org/apache/commons/jxpath/XMLDocumentContainer.java +++ b/src/main/java/org/apache/commons/jxpath/XMLDocumentContainer.java @@ -22,10 +22,10 @@ import javax.xml.transform.Source; import javax.xml.transform.Transformer; -import javax.xml.transform.TransformerFactory; import javax.xml.transform.dom.DOMResult; import org.apache.commons.jxpath.xml.DocumentContainer; +import org.apache.commons.xml.secure.SecureTransformerFactory; /** * An XML document container reads and parses XML only when it is accessed. JXPath traverses Containers transparently - you use the same paths to access objects @@ -85,7 +85,7 @@ public Object getValue() { try { if (source != null) { final DOMResult result = new DOMResult(); - final Transformer trans = TransformerFactory.newInstance().newTransformer(); + final Transformer trans = SecureTransformerFactory.newInstance().newTransformer(); trans.transform(source, result); document = result.getNode(); } else { diff --git a/src/main/java/org/apache/commons/jxpath/xml/DOMParser.java b/src/main/java/org/apache/commons/jxpath/xml/DOMParser.java index 26e7c69d7..35796d5be 100644 --- a/src/main/java/org/apache/commons/jxpath/xml/DOMParser.java +++ b/src/main/java/org/apache/commons/jxpath/xml/DOMParser.java @@ -22,6 +22,7 @@ import javax.xml.parsers.DocumentBuilderFactory; import org.apache.commons.jxpath.JXPathException; +import org.apache.commons.xml.secure.SecureDocumentBuilderFactory; /** * An implementation of the XMLParser interface that produces a DOM Document. @@ -38,7 +39,7 @@ public DOMParser() { @Override public Object parseXML(final InputStream stream) { try { - final DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); + final DocumentBuilderFactory factory = SecureDocumentBuilderFactory.newInstance(); factory.setValidating(isValidating()); factory.setNamespaceAware(isNamespaceAware()); factory.setIgnoringElementContentWhitespace(isIgnoringElementContentWhitespace()); diff --git a/src/main/java/org/apache/commons/jxpath/xml/JDOMParser.java b/src/main/java/org/apache/commons/jxpath/xml/JDOMParser.java index a10122cdd..bddea63a7 100644 --- a/src/main/java/org/apache/commons/jxpath/xml/JDOMParser.java +++ b/src/main/java/org/apache/commons/jxpath/xml/JDOMParser.java @@ -19,8 +19,13 @@ import java.io.InputStream; +import javax.xml.parsers.SAXParserFactory; + import org.apache.commons.jxpath.JXPathException; +import org.apache.commons.xml.secure.SecureSAXParserFactory; +import org.jdom.JDOMException; import org.jdom.input.SAXBuilder; +import org.xml.sax.XMLReader; /** * An implementation of the XMLParser interface that produces a JDOM Document. @@ -40,7 +45,19 @@ public Object parseXML(final InputStream stream) { throw new JXPathException("JDOM parser configuration error. JDOM does not support the namespaceAware=false setting."); } try { - final SAXBuilder builder = new SAXBuilder(); + // JDOM builds its reader through JAXP internally; hand it one from the secure factory instead. + final SAXBuilder builder = new SAXBuilder() { + @Override + protected XMLReader createParser() throws JDOMException { + try { + final SAXParserFactory factory = SecureSAXParserFactory.newNSInstance(); + factory.setValidating(isValidating()); + return factory.newSAXParser().getXMLReader(); + } catch (final Exception ex) { + throw new JDOMException("Unable to create a new XML reader", ex); + } + } + }; builder.setExpandEntities(isExpandEntityReferences()); builder.setIgnoringElementContentWhitespace(isIgnoringElementContentWhitespace()); builder.setValidation(isValidating()); From 49e6c1fdb665dbd4d1043706a37aec1af4022fb3 Mon Sep 17 00:00:00 2001 From: "Piotr P. Karwasz" Date: Thu, 3 Sep 2026 07:04:07 +0200 Subject: [PATCH 2/3] 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 +- pom.xml | 23 ++++++++++++++++++++++- 3 files changed, 23 insertions(+), 4 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 b2d1de81a..3f8896d46 100644 --- a/.github/workflows/maven.yml +++ b/.github/workflows/maven.yml @@ -59,4 +59,4 @@ jobs: distribution: ${{ runner.os == 'macOS' && matrix.java == '8' && 'zulu' || 'temurin' }} java-version: ${{ matrix.java }} - name: Build with Maven - run: mvn --errors --show-version --batch-mode --no-transfer-progress -DtrimStackTrace=false -Puse-apache-snapshots + run: mvn --errors --show-version --batch-mode --no-transfer-progress -DtrimStackTrace=false diff --git a/pom.xml b/pom.xml index 25a425668..8b9540e60 100644 --- a/pom.xml +++ b/pom.xml @@ -148,11 +148,32 @@ + + + + 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-SNAPSHOT + 1.0.0 javax.servlet From d2466f881b7461a2fc97191e258287a6140791a4 Mon Sep 17 00:00:00 2001 From: Gary Gregory Date: Sun, 6 Sep 2026 08:39:36 -0400 Subject: [PATCH 3/3] Bump Apache Commons Secure XML from 1.0.0-SNAPSHOT to 1.0.0 --- pom.xml | 21 --------------------- src/changes/changes.xml | 2 +- 2 files changed, 1 insertion(+), 22 deletions(-) diff --git a/pom.xml b/pom.xml index 8b9540e60..63bac1261 100644 --- a/pom.xml +++ b/pom.xml @@ -148,27 +148,6 @@ - - - - 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 diff --git a/src/changes/changes.xml b/src/changes/changes.xml index a546dec09..32630e06e 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -49,7 +49,7 @@ The type attribute can be add,update,fix,remove. - Create the DOM and JDOM parsers and the XML transformer through org.apache.commons:commons-secure-xml, so external entities and DTDs are no longer fetched by default. + Create the DOM and JDOM parsers and the XML transformer through org.apache.commons:commons-secure-xml, so external entities and DTDs are no longer fetched by default. POM assembly:single does not generate binary convenience files (tar/zip). Make dynamicPropertyHandlerMap in ValueUtils thread-safe #251. Refactor JXPathIntrospector internal static maps to use concurrent classes instead of synchronization.