You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by pk...@apache.org on 2026/06/20 20:09:16 UTC
(commons-xml) 21/50: fix: reliability of TraX stylesheet and XPath expression tests
This is an automated email from the ASF dual-hosted git repository.
ppkarwasz pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/commons-xml.git
commit a82ec99c5341e09f52fb575218051977b976eae5
Author: Piotr P. Karwasz <pk...@apache.org>
AuthorDate: Sat Apr 25 15:02:31 2026 +0200
fix: reliability of TraX stylesheet and XPath expression tests
---
.../xml/factory/attacks/AttackTestSupport.java | 14 +++
.../factory/attacks/ExternalStylesheetTest.java | 36 +++---
.../attacks/SaxonXPathExternalCallsTest.java | 125 ++++++++++++++++-----
.../included.xsl | 0
src/test/resources/leaked/referenced.json | 3 +
src/test/resources/leaked/referenced.txt | 2 +
.../referenced.xml | 0
.../with-document.xsl | 0
.../with-import.xsl | 0
.../with-include.xsl | 0
10 files changed, 134 insertions(+), 46 deletions(-)
diff --git a/src/test/java/org/apache/commons/xml/factory/attacks/AttackTestSupport.java b/src/test/java/org/apache/commons/xml/factory/attacks/AttackTestSupport.java
index fa965bc..74f442e 100644
--- a/src/test/java/org/apache/commons/xml/factory/attacks/AttackTestSupport.java
+++ b/src/test/java/org/apache/commons/xml/factory/attacks/AttackTestSupport.java
@@ -17,11 +17,13 @@
package org.apache.commons.xml.factory.attacks;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.fail;
import java.io.ByteArrayInputStream;
import java.io.StringReader;
import java.io.StringWriter;
+import java.net.URL;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
@@ -193,6 +195,18 @@ private static InputSource inputSource(final String xml) {
return new InputSource(new StringReader(xml));
}
+ /**
+ * Returns the URL of a fixture under {@code src/test/resources/leaked/} on the test classpath. Fails the test if the resource is not present.
+ *
+ * @param name the file name within the {@code leaked} directory, for example {@code "referenced.xml"}.
+ * @return a URL that can be used as a system id, opened for reading, or string-concatenated into an XPath URI.
+ */
+ static URL resourceUrl(final String name) {
+ final URL url = AttackTestSupport.class.getResource("/leaked/" + name);
+ assertNotNull(url, "test resource not found: " + name);
+ return url;
+ }
+
private static boolean messageOrCauseContainsHardeningKeyword(final Throwable t) {
Throwable current = t;
while (current != null) {
diff --git a/src/test/java/org/apache/commons/xml/factory/attacks/ExternalStylesheetTest.java b/src/test/java/org/apache/commons/xml/factory/attacks/ExternalStylesheetTest.java
index 7815a04..4a835ee 100644
--- a/src/test/java/org/apache/commons/xml/factory/attacks/ExternalStylesheetTest.java
+++ b/src/test/java/org/apache/commons/xml/factory/attacks/ExternalStylesheetTest.java
@@ -17,7 +17,6 @@
package org.apache.commons.xml.factory.attacks;
import static org.junit.jupiter.api.Assertions.assertFalse;
-import static org.junit.jupiter.api.Assertions.assertNotNull;
import java.io.StringReader;
import java.io.StringWriter;
@@ -33,7 +32,7 @@
/**
* Checks whether stylesheets can access external resources.
*
- * <p>Each fixture under {@code src/test/resources/ExternalStylesheetTest/} references a sibling file that, if hardening fails to block, would emit the
+ * <p>Each fixture under {@code src/test/resources/leaked/} references a sibling file that, if hardening fails to block, would emit the
* {@link #MARKER} string into the transform output. The assertion is deterministic: hardening either throws, or the call returns and the marker does not
* appear in the output.</p>
*
@@ -51,24 +50,8 @@ class ExternalStylesheetTest {
private static final String MARKER = "All your base are belong to us";
- @Test
- void transformerStylesheetBlocksInclude() {
- assertStylesheetExcludesMarker("with-include.xsl");
- }
-
- @Test
- void transformerStylesheetBlocksImport() {
- assertStylesheetExcludesMarker("with-import.xsl");
- }
-
- @Test
- void transformerBlocksDocument() {
- assertStylesheetExcludesMarker("with-document.xsl");
- }
-
private static void assertStylesheetExcludesMarker(final String resource) {
- final URL url = ExternalStylesheetTest.class.getResource("/ExternalStylesheetTest/" + resource);
- assertNotNull(url, "test resource not found: " + resource);
+ final URL url = AttackTestSupport.resourceUrl(resource);
final String output;
try {
final StreamSource src = new StreamSource(url.openStream(), url.toString());
@@ -82,4 +65,19 @@ private static void assertStylesheetExcludesMarker(final String resource) {
assertFalse(output.contains(MARKER),
"Hardening did not block the external reference; output contained marker '" + MARKER + "'.\nFull output:\n" + output);
}
+
+ @Test
+ void transformerBlocksDocument() {
+ assertStylesheetExcludesMarker("with-document.xsl");
+ }
+
+ @Test
+ void transformerStylesheetBlocksImport() {
+ assertStylesheetExcludesMarker("with-import.xsl");
+ }
+
+ @Test
+ void transformerStylesheetBlocksInclude() {
+ assertStylesheetExcludesMarker("with-include.xsl");
+ }
}
diff --git a/src/test/java/org/apache/commons/xml/factory/attacks/SaxonXPathExternalCallsTest.java b/src/test/java/org/apache/commons/xml/factory/attacks/SaxonXPathExternalCallsTest.java
index 6df71dd..e4c1bc0 100644
--- a/src/test/java/org/apache/commons/xml/factory/attacks/SaxonXPathExternalCallsTest.java
+++ b/src/test/java/org/apache/commons/xml/factory/attacks/SaxonXPathExternalCallsTest.java
@@ -16,8 +16,14 @@
*/
package org.apache.commons.xml.factory.attacks;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
import static org.junit.jupiter.api.Assumptions.assumeTrue;
+import javax.xml.parsers.DocumentBuilderFactory;
+import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.xpath.XPathExpressionException;
import javax.xml.xpath.XPathFactory;
import org.apache.commons.xml.factory.internal.CompositeProvider;
@@ -25,21 +31,72 @@
import org.junit.jupiter.api.Test;
/**
- * Exercises Saxon's XPath 3.1 URI-fetching functions and verifies that a hardened Saxon {@link XPathFactory} refuses them before any I/O.
+ * Checks whether Saxon's XPath 3.1 URI-fetching functions can pull external resources into the result.
*
- * <p>Saxon's XPath 3.1 engine ships four functions that open arbitrary URIs during expression evaluation:
- * {@code doc()}, {@code collection()}, {@code unparsed-text()} and {@code json-doc()}. None of them require a context node; each is triggered purely by the
- * string URI they receive. They are <em>not</em> classified as extension functions in Saxon's vocabulary, so disabling {@code ALLOW_EXTERNAL_FUNCTIONS} is
- * not sufficient to block them; a complete hardening has to close the URI-resolution path too.</p>
+ * <p>Saxon ships several XPath 3.1 functions that open arbitrary URIs during evaluation. None require a context node; each is triggered purely by the string
+ * URI it receives. They are <em>not</em> classified as extension functions in Saxon's vocabulary, so disabling {@code ALLOW_EXTERNAL_FUNCTIONS} is not enough
+ * to block them; a complete hardening has to close the URI-resolution path.</p>
*
- * <p>The JDK's built-in XPathFactory and Xalan have no equivalent vectors.</p>
+ * <p>Each fixture under {@code src/test/resources/leaked/} contains the {@link #MARKER} string. The tests dispatch the URI-fetching function at the file's URL
+ * and check whether the marker reaches the result.</p>
*
- * <p>Skipped when Saxon is not on the classpath; only runs under the {@code test-saxon} surefire profile.</p>
+ * <p>Cases covered, each as a pair (unconfigured Saxon factory expected to leak, hardened Saxon factory expected to block):</p>
+ *
+ * <ul>
+ * <li>{@code doc(uri)} reading {@code referenced.xml}.</li>
+ * <li>{@code json-doc(uri)} reading {@code referenced.json}.</li>
+ * <li>{@code unparsed-text(uri)} reading {@code referenced.txt}.</li>
+ * </ul>
+ *
+ * <p>The JDK's built-in XPathFactory and Xalan have no equivalent vectors. Skipped when Saxon is not on the classpath; only runs under the {@code test-saxon}
+ * surefire execution.</p>
*/
class SaxonXPathExternalCallsTest {
+ private static final String MARKER = "All your base are belong to us";
private static final String SAXON_XPATH_FACTORY_CLASS = "net.sf.saxon.xpath.XPathFactoryImpl";
+ private static void assertCallExcludesMarker(final XPathFactory factory, final String expression) {
+ final String result;
+ try {
+ result = evaluateAsString(factory, expression);
+ } catch (final Exception e) {
+ return; // hardening blocked at evaluation; acceptable outcome.
+ }
+ assertFalse(result.contains(MARKER),
+ "Hardening did not block the external reference; result contained marker '" + MARKER + "'.\nFull result:\n" + result);
+ }
+
+ private static void assertCallLeaksMarker(final XPathFactory factory, final String expression) {
+ final String result;
+ try {
+ result = evaluateAsString(factory, expression);
+ } catch (final Exception e) {
+ fail("Unconfigured Saxon XPath should resolve the external resource, but threw: " + e);
+ return;
+ }
+ assertTrue(result.contains(MARKER),
+ "Expected marker '" + MARKER + "' in result, got: " + result);
+ }
+
+ private static String docExpression() {
+ return "doc('" + AttackTestSupport.resourceUrl("referenced.xml") + "')/leaked";
+ }
+
+ private static String evaluateAsString(final XPathFactory factory, final String expression)
+ throws ParserConfigurationException, XPathExpressionException {
+ return factory.newXPath().evaluate(expression,
+ DocumentBuilderFactory.newInstance().newDocumentBuilder().newDocument());
+ }
+
+ private static XPathFactory hardenedSaxonXPathFactory() {
+ return CompositeProvider.getInstance().configure(saxonXPathFactory());
+ }
+
+ private static String jsonDocExpression() {
+ return "json-doc('" + AttackTestSupport.resourceUrl("referenced.json") + "')?leaked";
+ }
+
@BeforeAll
static void requireSaxon() {
try {
@@ -49,37 +106,51 @@ static void requireSaxon() {
}
}
+ /**
+ * Instantiates Saxon's {@code XPathFactoryImpl} reflectively.
+ *
+ * <p>Saxon 12.9 ships no {@code META-INF/services} entry for
+ * {@link javax.xml.xpath.XPathFactory}, so {@link XPathFactory#newInstance(String)} cannot find it; direct instantiation bypasses that lookup.</p>
+ */
+ private static XPathFactory saxonXPathFactory() {
+ try {
+ return (XPathFactory) Class.forName(SAXON_XPATH_FACTORY_CLASS).getDeclaredConstructor().newInstance();
+ } catch (final ReflectiveOperationException e) {
+ throw new AssertionError("Cannot instantiate " + SAXON_XPATH_FACTORY_CLASS, e);
+ }
+ }
+
+ private static String unparsedTextExpression() {
+ return "unparsed-text('" + AttackTestSupport.resourceUrl("referenced.txt") + "')";
+ }
+
@Test
- void xpathBlocksCollectionCall() {
- AttackTestSupport.assertXPathBlocks(hardenedSaxonXPathFactory(), "collection('" + Payloads.UNREACHABLE_HTTP + "')");
+ void hardenedXPathBlocksDoc() {
+ assertCallExcludesMarker(hardenedSaxonXPathFactory(), docExpression());
}
@Test
- void xpathBlocksDocCall() {
- AttackTestSupport.assertXPathBlocks(hardenedSaxonXPathFactory(), "doc('" + Payloads.UNREACHABLE_HTTP + "')");
+ void hardenedXPathBlocksJsonDoc() {
+ assertCallExcludesMarker(hardenedSaxonXPathFactory(), jsonDocExpression());
}
@Test
- void xpathBlocksJsonDocCall() {
- AttackTestSupport.assertXPathBlocks(hardenedSaxonXPathFactory(), "json-doc('" + Payloads.UNREACHABLE_HTTP + "')");
+ void hardenedXPathBlocksUnparsedText() {
+ assertCallExcludesMarker(hardenedSaxonXPathFactory(), unparsedTextExpression());
}
@Test
- void xpathBlocksUnparsedTextCall() {
- AttackTestSupport.assertXPathBlocks(hardenedSaxonXPathFactory(), "unparsed-text('" + Payloads.UNREACHABLE_HTTP + "')");
+ void unconfiguredXPathLeaksDoc() {
+ assertCallLeaksMarker(saxonXPathFactory(), docExpression());
}
- /**
- * Instantiates Saxon's {@code net.sf.saxon.xpath.XPathFactoryImpl} reflectively. Saxon 12.9 does not ship a {@code META-INF/services} entry for
- * {@code javax.xml.xpath.XPathFactory}, so {@code XPathFactory.newInstance(Saxon-URI)} cannot find it; direct instantiation bypasses that lookup.
- */
- private static XPathFactory hardenedSaxonXPathFactory() {
- final XPathFactory xpf;
- try {
- xpf = (XPathFactory) Class.forName(SAXON_XPATH_FACTORY_CLASS).getDeclaredConstructor().newInstance();
- } catch (final ReflectiveOperationException e) {
- throw new AssertionError("Cannot instantiate " + SAXON_XPATH_FACTORY_CLASS, e);
- }
- return CompositeProvider.getInstance().configure(xpf);
+ @Test
+ void unconfiguredXPathLeaksJsonDoc() {
+ assertCallLeaksMarker(saxonXPathFactory(), jsonDocExpression());
+ }
+
+ @Test
+ void unconfiguredXPathLeaksUnparsedText() {
+ assertCallLeaksMarker(saxonXPathFactory(), unparsedTextExpression());
}
}
diff --git a/src/test/resources/ExternalStylesheetTest/included.xsl b/src/test/resources/leaked/included.xsl
similarity index 100%
rename from src/test/resources/ExternalStylesheetTest/included.xsl
rename to src/test/resources/leaked/included.xsl
diff --git a/src/test/resources/leaked/referenced.json b/src/test/resources/leaked/referenced.json
new file mode 100644
index 0000000..08187f7
--- /dev/null
+++ b/src/test/resources/leaked/referenced.json
@@ -0,0 +1,3 @@
+{
+ "leaked": "All your base are belong to us"
+}
diff --git a/src/test/resources/leaked/referenced.txt b/src/test/resources/leaked/referenced.txt
new file mode 100644
index 0000000..b3c52d2
--- /dev/null
+++ b/src/test/resources/leaked/referenced.txt
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: Apache-2.0
+All your base are belong to us
diff --git a/src/test/resources/ExternalStylesheetTest/referenced.xml b/src/test/resources/leaked/referenced.xml
similarity index 100%
rename from src/test/resources/ExternalStylesheetTest/referenced.xml
rename to src/test/resources/leaked/referenced.xml
diff --git a/src/test/resources/ExternalStylesheetTest/with-document.xsl b/src/test/resources/leaked/with-document.xsl
similarity index 100%
rename from src/test/resources/ExternalStylesheetTest/with-document.xsl
rename to src/test/resources/leaked/with-document.xsl
diff --git a/src/test/resources/ExternalStylesheetTest/with-import.xsl b/src/test/resources/leaked/with-import.xsl
similarity index 100%
rename from src/test/resources/ExternalStylesheetTest/with-import.xsl
rename to src/test/resources/leaked/with-import.xsl
diff --git a/src/test/resources/ExternalStylesheetTest/with-include.xsl b/src/test/resources/leaked/with-include.xsl
similarity index 100%
rename from src/test/resources/ExternalStylesheetTest/with-include.xsl
rename to src/test/resources/leaked/with-include.xsl