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 <[email protected]> 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
