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 fca53edaf97ddb98a55fcd25fce9ac86858d1b2b Author: Piotr P. Karwasz <[email protected]> AuthorDate: Sat Apr 25 16:20:13 2026 +0200 fix: make ExternalSchemaTest more reliable (#1) The previous version relied on Payloads.UNREACHABLE_HTTP and a 2-second timing budget combined with a keyword filter to tell "hardened-blocked" apart from "TCP refused late". On macOS and Windows runners that combination is fragile. This change adds four schema fixtures under src/test/resources/leaked/: a base included.xsd that declares a leaked element and a LeakedType, plus three wrappers that reference it through xs:include, xs:import (in a different namespace) and xs:redefine. Each wrapper cannot compile unless the schemaLocation actually resolves, so compile-success vs compile-throw is itself the hardening signal, with no timing budget and no keyword filter. This change also pairs each case with an unconfiguredCompiles* positive control that runs the same fixture through SchemaFactory.newInstance() and asserts compilation succeeds. That proves the fixtures are well-formed and the resolution would happen without hardening, so the hardenedBlocks* side is genuinely exercising hardening. Finally, this change removes the now-unused AttackTestSupport.assertXPathBlocks helper. Assisted-By: Claude Opus 4.7 (1M context) <[email protected]> --- .../xml/factory/attacks/AttackTestSupport.java | 9 --- .../xml/factory/attacks/ExternalSchemaTest.java | 92 +++++++++++++++------- src/test/resources/leaked/included.xsd | 11 +++ src/test/resources/leaked/with-import.xsd | 15 ++++ src/test/resources/leaked/with-include.xsd | 15 ++++ src/test/resources/leaked/with-redefine.xsd | 19 +++++ 6 files changed, 123 insertions(+), 38 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 74f442e..fc5d3d9 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 @@ -182,15 +182,6 @@ static void assertValidatorBlocks(final String xml) { assertAttackBlocked(() -> XmlFactories.newSchemaFactory().newSchema(source(Payloads.BENIGN_SCHEMA)).newValidator().validate(source(xml))); } - /** - * Asserts that evaluating the given XPath expression on a fresh {@link javax.xml.xpath.XPath} from {@code factory} is blocked by the hardening layer. The - * expression is evaluated against an empty DOM document as context; this is enough for URI-fetching functions such as {@code doc()}, {@code collection()}, - * {@code unparsed-text()} and {@code json-doc()}, which do not consult the context node beyond requiring it to be non-null. - */ - static void assertXPathBlocks(final javax.xml.xpath.XPathFactory factory, final String expression) { - assertAttackBlocked(() -> factory.newXPath().evaluate(expression, XmlFactories.newDocumentBuilderFactory().newDocumentBuilder().newDocument())); - } - private static InputSource inputSource(final String xml) { return new InputSource(new StringReader(xml)); } diff --git a/src/test/java/org/apache/commons/xml/factory/attacks/ExternalSchemaTest.java b/src/test/java/org/apache/commons/xml/factory/attacks/ExternalSchemaTest.java index b394a4c..632335e 100644 --- a/src/test/java/org/apache/commons/xml/factory/attacks/ExternalSchemaTest.java +++ b/src/test/java/org/apache/commons/xml/factory/attacks/ExternalSchemaTest.java @@ -16,54 +16,88 @@ */ package org.apache.commons.xml.factory.attacks; +import static org.junit.jupiter.api.Assertions.assertDoesNotThrow; +import static org.junit.jupiter.api.Assertions.assertThrows; + +import java.net.URL; + +import javax.xml.XMLConstants; +import javax.xml.transform.stream.StreamSource; +import javax.xml.validation.SchemaFactory; + +import org.apache.commons.xml.factory.XmlFactories; import org.junit.jupiter.api.Test; /** - * Exercises {@link javax.xml.XMLConstants#ACCESS_EXTERNAL_SCHEMA ACCESS_EXTERNAL_SCHEMA} via the three XSD compile-time vectors it guards: - * {@code xs:import}, {@code xs:include} and {@code xs:redefine}. + * Checks whether schemas can pull in external schemas at compile time. + * + * <p>Each wrapper under {@code src/test/resources/leaked/} references an element or type defined only in {@code included.xsd}, so the wrapper cannot compile + * unless the external schemaLocation is actually resolved.</p> + * + * <p>Each case is exercised twice as a pair:</p> + * + * <ul> + * <li>An {@code unconfigured*} test compiles the wrapper through a default {@link SchemaFactory#newInstance(String)} and asserts compilation succeeds. This + * is the positive control: it proves the wrapper itself is well-formed and that the resolution would succeed if hardening were not in the way, so the + * paired {@code hardened*} test is genuinely exercising hardening rather than passing for an unrelated reason.</li> + * <li>A {@code hardened*} test compiles the wrapper through {@link XmlFactories#newSchemaFactory()} and asserts compilation throws. If hardening blocks the + * schemaLocation, compilation throws (either with an access-restriction message or with an unresolved-reference message); if hardening fails to block, + * compilation succeeds and the test fails.</li> + * </ul> + * + * <p>Cases covered:</p> + * + * <ul> + * <li>{@code xs:include schemaLocation="included.xsd"} of a sibling schema sharing the wrapper's target namespace.</li> + * <li>{@code xs:import namespace="..." schemaLocation="included.xsd"} of a sibling schema in a different target namespace.</li> + * <li>{@code xs:redefine schemaLocation="included.xsd"} that extends a complex type from the sibling schema.</li> + * </ul> */ class ExternalSchemaTest { - private static final String IMPORT_NAMESPACE = "http://example.org/external";; + private static void assertSchemaCompileFails(final String resource) { + final URL url = AttackTestSupport.resourceUrl(resource); + assertThrows(Exception.class, () -> { + final StreamSource src = new StreamSource(url.openStream(), url.toString()); + XmlFactories.newSchemaFactory().newSchema(src); + }, "Hardening did not block the external schemaLocation; schema compiled successfully from " + resource); + } - private static String xsdWithImport(final String namespace, final String schemaLocation) { - return "<?xml version=\"1.0\"?>\n" - + "<xs:schema xmlns:xs=\"http://www.w3.org/2001/XMLSchema\";>\n" - + " <xs:import namespace=\"" + namespace + "\" schemaLocation=\"" + schemaLocation + "\"/>\n" - + " <xs:element name=\"root\" type=\"xs:string\"/>\n" - + "</xs:schema>\n"; + private static void assertSchemaCompiles(final String resource) { + final URL url = AttackTestSupport.resourceUrl(resource); + assertDoesNotThrow(() -> { + final StreamSource src = new StreamSource(url.openStream(), url.toString()); + SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI).newSchema(src); + }, "Unconfigured SchemaFactory should compile " + resource + "; the wrapper or its sibling include/import/redefine target is broken."); } - private static String xsdWithInclude(final String schemaLocation) { - return "<?xml version=\"1.0\"?>\n" - + "<xs:schema xmlns:xs=\"http://www.w3.org/2001/XMLSchema\";>\n" - + " <xs:include schemaLocation=\"" + schemaLocation + "\"/>\n" - + " <xs:element name=\"root\" type=\"xs:string\"/>\n" - + "</xs:schema>\n"; + @Test + void hardenedBlocksImport() { + assertSchemaCompileFails("with-import.xsd"); } - private static String xsdWithRedefine(final String schemaLocation) { - return "<?xml version=\"1.0\"?>\n" - + "<xs:schema xmlns:xs=\"http://www.w3.org/2001/XMLSchema\";>\n" - + " <xs:redefine schemaLocation=\"" + schemaLocation + "\">\n" - + " <xs:complexType name=\"Placeholder\"><xs:sequence/></xs:complexType>\n" - + " </xs:redefine>\n" - + " <xs:element name=\"root\" type=\"xs:string\"/>\n" - + "</xs:schema>\n"; + @Test + void hardenedBlocksInclude() { + assertSchemaCompileFails("with-include.xsd"); + } + + @Test + void hardenedBlocksRedefine() { + assertSchemaCompileFails("with-redefine.xsd"); } @Test - void schemaFactoryBlocksImport() { - AttackTestSupport.assertSchemaCompilationBlocks(xsdWithImport(IMPORT_NAMESPACE, Payloads.UNREACHABLE_HTTP)); + void unconfiguredCompilesImport() { + assertSchemaCompiles("with-import.xsd"); } @Test - void schemaFactoryBlocksInclude() { - AttackTestSupport.assertSchemaCompilationBlocks(xsdWithInclude(Payloads.UNREACHABLE_HTTP)); + void unconfiguredCompilesInclude() { + assertSchemaCompiles("with-include.xsd"); } @Test - void schemaFactoryBlocksRedefine() { - AttackTestSupport.assertSchemaCompilationBlocks(xsdWithRedefine(Payloads.UNREACHABLE_HTTP)); + void unconfiguredCompilesRedefine() { + assertSchemaCompiles("with-redefine.xsd"); } } diff --git a/src/test/resources/leaked/included.xsd b/src/test/resources/leaked/included.xsd new file mode 100644 index 0000000..8bba9e1 --- /dev/null +++ b/src/test/resources/leaked/included.xsd @@ -0,0 +1,11 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- SPDX-License-Identifier: Apache-2.0 --> +<xs:schema xmlns:xs="http://www.w3.org/2001/XMLSchema"; + xmlns="http://example.org/leaked"; + targetNamespace="http://example.org/leaked"; + elementFormDefault="qualified"> + <xs:element name="leaked" type="xs:string"/> + <xs:complexType name="LeakedType"> + <xs:sequence/> + </xs:complexType> +</xs:schema> diff --git a/src/test/resources/leaked/with-import.xsd b/src/test/resources/leaked/with-import.xsd new file mode 100644 index 0000000..8c0205f --- /dev/null +++ b/src/test/resources/leaked/with-import.xsd @@ -0,0 +1,15 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- SPDX-License-Identifier: Apache-2.0 --> +<xs:schema xmlns:xs="http://www.w3.org/2001/XMLSchema"; + xmlns:l="http://example.org/leaked"; + targetNamespace="http://example.org/wrapper"; + elementFormDefault="qualified"> + <xs:import namespace="http://example.org/leaked"; schemaLocation="included.xsd"/> + <xs:element name="root"> + <xs:complexType> + <xs:sequence> + <xs:element ref="l:leaked"/> + </xs:sequence> + </xs:complexType> + </xs:element> +</xs:schema> diff --git a/src/test/resources/leaked/with-include.xsd b/src/test/resources/leaked/with-include.xsd new file mode 100644 index 0000000..f0886a5 --- /dev/null +++ b/src/test/resources/leaked/with-include.xsd @@ -0,0 +1,15 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- SPDX-License-Identifier: Apache-2.0 --> +<xs:schema xmlns:xs="http://www.w3.org/2001/XMLSchema"; + xmlns="http://example.org/leaked"; + targetNamespace="http://example.org/leaked"; + elementFormDefault="qualified"> + <xs:include schemaLocation="included.xsd"/> + <xs:element name="root"> + <xs:complexType> + <xs:sequence> + <xs:element ref="leaked"/> + </xs:sequence> + </xs:complexType> + </xs:element> +</xs:schema> diff --git a/src/test/resources/leaked/with-redefine.xsd b/src/test/resources/leaked/with-redefine.xsd new file mode 100644 index 0000000..63710e9 --- /dev/null +++ b/src/test/resources/leaked/with-redefine.xsd @@ -0,0 +1,19 @@ +<?xml version="1.0" encoding="UTF-8"?> +<!-- SPDX-License-Identifier: Apache-2.0 --> +<xs:schema xmlns:xs="http://www.w3.org/2001/XMLSchema"; + xmlns="http://example.org/leaked"; + targetNamespace="http://example.org/leaked"; + elementFormDefault="qualified"> + <xs:redefine schemaLocation="included.xsd"> + <xs:complexType name="LeakedType"> + <xs:complexContent> + <xs:extension base="LeakedType"> + <xs:sequence> + <xs:element name="extra" type="xs:string"/> + </xs:sequence> + </xs:extension> + </xs:complexContent> + </xs:complexType> + </xs:redefine> + <xs:element name="root" type="LeakedType"/> +</xs:schema>
