This is an automated email from the ASF dual-hosted git repository. garydgregory pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/commons-secure-xml.git
commit 18dcc5f74a1227bfe5ac96ef54fbe804ef3c6be8 Author: Gary Gregory <[email protected]> AuthorDate: Tue Sep 1 08:03:20 2026 -0400 Sort members. --- .../xml/secure/SecureTransformerFactory.java | 30 ++++---- .../commons/xml/secure/AttackTestSupport.java | 42 +++++------ .../xml/secure/OverrideDefaultParserTest.java | 10 +-- .../secure/SaxonTransformerExternalCallsTest.java | 88 +++++++++++----------- .../xml/secure/SchemaContentModelLimitTest.java | 34 ++++----- .../commons/xml/secure/SecureXMLFilterTest.java | 36 ++++----- .../xml/secure/SecureXMLInputFactoryTest.java | 56 +++++++------- .../commons/xml/secure/SecureXPathFactoryTest.java | 76 +++++++++---------- 8 files changed, 186 insertions(+), 186 deletions(-) diff --git a/src/main/java/org/apache/commons/xml/secure/SecureTransformerFactory.java b/src/main/java/org/apache/commons/xml/secure/SecureTransformerFactory.java index f7dfa13..e31f7ab 100644 --- a/src/main/java/org/apache/commons/xml/secure/SecureTransformerFactory.java +++ b/src/main/java/org/apache/commons/xml/secure/SecureTransformerFactory.java @@ -187,21 +187,6 @@ private Wrapper(final SAXTransformerFactory delegate, final Supplier<Source> emp delegate.setURIResolver(floor); } - /** - * {@inheritDoc} - * - * @throws FactoryConfigurationError Thrown from a factory in case of a {@link java.util.ServiceConfigurationError service - * configuration error} or if the implementation is not available or cannot be instantiated. - */ - @Override - public Source getAssociatedStylesheet(final Source source, final String media, final String title, final String charset) - throws TransformerConfigurationException { - // Xalan's getAssociatedStylesheet drops a SAXSource's reader and self-provisions its own to scan for xml-stylesheet PIs (XALANJ-2849), and the - // JDK's XSLTC did the same before 8u162; hand those a DOM so no parser but ours ever sees the document. - final Source secure = isXalan(delegate) || JAVA_8 ? secureSourceToDom(source) : SecureSAXParserFactory.secure(source, overrideDefaultParser()); - return floorAssociated(delegate.getAssociatedStylesheet(secure, media, title, charset), secure.getSystemId()); - } - /** * Routes the href an {@code xml-stylesheet} PI yielded through the floor, so a URI distilled from untrusted content is opted in by the caller's * resolver or resolved to empty like any other content-named reference. @@ -227,6 +212,21 @@ private Source floorAssociated(final Source associated, final String base) throw } } + /** + * {@inheritDoc} + * + * @throws FactoryConfigurationError Thrown from a factory in case of a {@link java.util.ServiceConfigurationError service + * configuration error} or if the implementation is not available or cannot be instantiated. + */ + @Override + public Source getAssociatedStylesheet(final Source source, final String media, final String title, final String charset) + throws TransformerConfigurationException { + // Xalan's getAssociatedStylesheet drops a SAXSource's reader and self-provisions its own to scan for xml-stylesheet PIs (XALANJ-2849), and the + // JDK's XSLTC did the same before 8u162; hand those a DOM so no parser but ours ever sees the document. + final Source secure = isXalan(delegate) || JAVA_8 ? secureSourceToDom(source) : SecureSAXParserFactory.secure(source, overrideDefaultParser()); + return floorAssociated(delegate.getAssociatedStylesheet(secure, media, title, charset), secure.getSystemId()); + } + @Override public Object getAttribute(final String name) { return delegate.getAttribute(name); diff --git a/src/test/java/org/apache/commons/xml/secure/AttackTestSupport.java b/src/test/java/org/apache/commons/xml/secure/AttackTestSupport.java index 9277628..819f7ec 100644 --- a/src/test/java/org/apache/commons/xml/secure/AttackTestSupport.java +++ b/src/test/java/org/apache/commons/xml/secure/AttackTestSupport.java @@ -240,6 +240,13 @@ static void assertDomBlocksOrDoesNotLeak(final String payload) { assertNoLeakOrThrows(() -> domParseAndCaptureText(SecureDocumentBuilderFactory.newInstance(), payload), "DOM", SAXException.class); } + /** + * Same contract as {@link #assertDomDoesNotLeak(String)}, on a caller-configured secure factory. + */ + static void assertDomDoesNotLeak(final DocumentBuilderFactory factory, final String payload) { + assertNoLeakStrict(() -> domParseAndCaptureText(factory, payload), "DOM"); + } + /** * Asserts a secure DOM parse completes without throwing and without leaked content. * @@ -250,13 +257,6 @@ static void assertDomDoesNotLeak(final String payload) { assertDomDoesNotLeak(SecureDocumentBuilderFactory.newInstance(), payload); } - /** - * Same contract as {@link #assertDomDoesNotLeak(String)}, on a caller-configured secure factory. - */ - static void assertDomDoesNotLeak(final DocumentBuilderFactory factory, final String payload) { - assertNoLeakStrict(() -> domParseAndCaptureText(factory, payload), "DOM"); - } - /** * Asserts a secure DOM parse succeeds. * @@ -507,6 +507,13 @@ static void assertSaxParses(final String payload) { assertParseSucceeds(() -> consumeXmlReader(strictXMLReader(SecureSAXParserFactory.newInstance()), payload), "SAX"); } + /** + * Same contract as {@link #assertSchemaBlocks(Source)}, on a caller-configured secure factory. + */ + static void assertSchemaBlocks(final SchemaFactory factory, final Source xsd) { + assertParseFails(() -> strictSchema(factory, xsd), "Schema compile", SAXException.class, SecurityException.class); + } + /** * Asserts a secure Schema compilation throws. * @@ -516,13 +523,6 @@ static void assertSchemaBlocks(final Source xsd) { assertSchemaBlocks(SecureSchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI), xsd); } - /** - * Same contract as {@link #assertSchemaBlocks(Source)}, on a caller-configured secure factory. - */ - static void assertSchemaBlocks(final SchemaFactory factory, final Source xsd) { - assertParseFails(() -> strictSchema(factory, xsd), "Schema compile", SAXException.class, SecurityException.class); - } - /** * Asserts a secure Schema compile either blocks or completes: an unresolved import resolves to an empty schema (which may itself fail to compile) or is * rejected outright. See {@link #assertDomBlocksOrDoesNotLeak(String)}. @@ -543,6 +543,13 @@ static void assertSchemaCompiles(final Source xsd) { assertParseSucceeds(() -> strictSchema(SecureSchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI), xsd), "Schema compile"); } + /** + * Same contract as {@link #assertSchemaDoesNotLeak(Source)}, on a caller-configured secure factory. + */ + static void assertSchemaDoesNotLeak(final SchemaFactory factory, final Source xsd) { + assertParseSucceeds(() -> strictSchema(factory, xsd), "Schema compile"); + } + /** * Asserts a secure Schema compilation completes without throwing. * @@ -554,13 +561,6 @@ static void assertSchemaDoesNotLeak(final Source xsd) { assertSchemaDoesNotLeak(SecureSchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI), xsd); } - /** - * Same contract as {@link #assertSchemaDoesNotLeak(Source)}, on a caller-configured secure factory. - */ - static void assertSchemaDoesNotLeak(final SchemaFactory factory, final Source xsd) { - assertParseSucceeds(() -> strictSchema(factory, xsd), "Schema compile"); - } - /** * Asserts a secure StAX parse of the payload throws. * diff --git a/src/test/java/org/apache/commons/xml/secure/OverrideDefaultParserTest.java b/src/test/java/org/apache/commons/xml/secure/OverrideDefaultParserTest.java index d5b640e..eec1217 100644 --- a/src/test/java/org/apache/commons/xml/secure/OverrideDefaultParserTest.java +++ b/src/test/java/org/apache/commons/xml/secure/OverrideDefaultParserTest.java @@ -58,6 +58,11 @@ class OverrideDefaultParserTest { /** {@code true} where the runtime's factories know {@value SecureSAXParserFactory#OVERRIDE_DEFAULT_PARSER}; JDK 8 gained it in 8u162. */ private static final boolean SUPPORTS_FEATURE = probeFeature(); + /** Skips a test on a runtime whose factories do not recognize the feature, where there is no selection to observe. */ + private static void assumeFeatureSupported() { + assumeTrue(SUPPORTS_FEATURE, "runtime does not recognize " + FEATURE); + } + private static boolean probeFeature() { try { TransformerFactory.newInstance().setFeature(FEATURE, true); @@ -67,11 +72,6 @@ private static boolean probeFeature() { } } - /** Skips a test on a runtime whose factories do not recognize the feature, where there is no selection to observe. */ - private static void assumeFeatureSupported() { - assumeTrue(SUPPORTS_FEATURE, "runtime does not recognize " + FEATURE); - } - private static String transform(final TransformerFactory factory, final String text) throws Exception { final Transformer transformer = factory.newTransformer(AttackTestSupport.streamSource(AttackTestSupport.xsltBody(text))); final StringWriter out = new StringWriter(); diff --git a/src/test/java/org/apache/commons/xml/secure/SaxonTransformerExternalCallsTest.java b/src/test/java/org/apache/commons/xml/secure/SaxonTransformerExternalCallsTest.java index d75f430..eadd3c0 100644 --- a/src/test/java/org/apache/commons/xml/secure/SaxonTransformerExternalCallsTest.java +++ b/src/test/java/org/apache/commons/xml/secure/SaxonTransformerExternalCallsTest.java @@ -52,6 +52,22 @@ class SaxonTransformerExternalCallsTest { private static final String SAXON_TRANSFORMER_FACTORY_CLASS = "net.sf.saxon.TransformerFactoryImpl"; + /** Runs the expression through the secure Saxon factory; a throw is an acceptable block, otherwise the marker must be absent. */ + private static void assertSecureDoesNotLeak(final String expression) { + try { + final String result = transform(SecureTransformerFactory.secure(saxonFactory()), expression); + assertFalse(result.contains(AttackTestSupport.LEAKED_MARKER), "secure Saxon transform leaked through " + expression + ":\n" + result); + } catch (final TransformerException blocked) { + // Throwing also prevents the leak. + } + } + + /** Runs the expression through the unconfigured Saxon factory and asserts the marker is resolved into the output (leak control). */ + private static void assertUnconfiguredLeaks(final String expression) throws TransformerException { + final String result = transform(saxonFactory(), expression); + assertTrue(result.contains(AttackTestSupport.LEAKED_MARKER), "unconfigured Saxon was expected to resolve " + expression + ", got: " + result); + } + private static void assumeSaxonPresent() { boolean present; try { @@ -63,19 +79,13 @@ private static void assumeSaxonPresent() { Assumptions.assumeTrue(present, "Saxon is not on the classpath"); } - /** Wraps a single XPath 3.1 expression in an XSLT 3.0 stylesheet that copies its string value into the output. */ - private static String stylesheet(final String expression) { - return "<?xml version=\"1.0\"?>\n" - + "<xsl:stylesheet version=\"3.0\" xmlns:xsl=\"http://www.w3.org/1999/XSL/Transform\";>\n" - + " <xsl:template match=\"/\">\n" - + " <leaked><xsl:value-of select=\"" + expression + "\"/></leaked>\n" - + " </xsl:template>\n" - + "</xsl:stylesheet>\n"; - } - - /** URL of a fixture that carries {@link AttackTestSupport#LEAKED_MARKER}; {@code name} is a file under {@code src/test/resources/leaked/}. */ - private static String url(final String name) { - return AttackTestSupport.resourceUrl(name).toString(); + /** The {@code unparsed-text-available} answer under the secure factory, or {@code "blocked"} when the transform throws. */ + private static String availabilityUnderSecure(final String uri) { + try { + return transform(SecureTransformerFactory.secure(saxonFactory()), "unparsed-text-available('" + uri + "')").contains("true") ? "true" : "false"; + } catch (final TransformerException blocked) { + return "blocked"; + } } /** URL of a sibling resource that does not exist, so a real fetch fails; used as the negative side of the existence-oracle check. */ @@ -91,6 +101,16 @@ private static TransformerFactory saxonFactory() { } } + /** Wraps a single XPath 3.1 expression in an XSLT 3.0 stylesheet that copies its string value into the output. */ + private static String stylesheet(final String expression) { + return "<?xml version=\"1.0\"?>\n" + + "<xsl:stylesheet version=\"3.0\" xmlns:xsl=\"http://www.w3.org/1999/XSL/Transform\";>\n" + + " <xsl:template match=\"/\">\n" + + " <leaked><xsl:value-of select=\"" + expression + "\"/></leaked>\n" + + " </xsl:template>\n" + + "</xsl:stylesheet>\n"; + } + private static String transform(final TransformerFactory factory, final String expression) throws TransformerException { final StringWriter sink = new StringWriter(); factory.newTemplates(AttackTestSupport.streamSource(stylesheet(expression))).newTransformer() @@ -98,20 +118,15 @@ private static String transform(final TransformerFactory factory, final String e return sink.toString(); } - /** Runs the expression through the secure Saxon factory; a throw is an acceptable block, otherwise the marker must be absent. */ - private static void assertSecureDoesNotLeak(final String expression) { - try { - final String result = transform(SecureTransformerFactory.secure(saxonFactory()), expression); - assertFalse(result.contains(AttackTestSupport.LEAKED_MARKER), "secure Saxon transform leaked through " + expression + ":\n" + result); - } catch (final TransformerException blocked) { - // Throwing also prevents the leak. - } + /** URL of a fixture that carries {@link AttackTestSupport#LEAKED_MARKER}; {@code name} is a file under {@code src/test/resources/leaked/}. */ + private static String url(final String name) { + return AttackTestSupport.resourceUrl(name).toString(); } - /** Runs the expression through the unconfigured Saxon factory and asserts the marker is resolved into the output (leak control). */ - private static void assertUnconfiguredLeaks(final String expression) throws TransformerException { - final String result = transform(saxonFactory(), expression); - assertTrue(result.contains(AttackTestSupport.LEAKED_MARKER), "unconfigured Saxon was expected to resolve " + expression + ", got: " + result); + @Test + void secureTransformerBlocksJsonDoc() { + assumeSaxonPresent(); + assertSecureDoesNotLeak("json-doc('" + url("referenced.json") + "')?leaked"); } @Test @@ -126,12 +141,6 @@ void secureTransformerBlocksUnparsedTextLines() { assertSecureDoesNotLeak("string-join(unparsed-text-lines('" + url("referenced.txt") + "'), ' ')"); } - @Test - void secureTransformerBlocksJsonDoc() { - assumeSaxonPresent(); - assertSecureDoesNotLeak("json-doc('" + url("referenced.json") + "')?leaked"); - } - @Test void secureTransformerHidesUnparsedTextAvailability() { assumeSaxonPresent(); @@ -152,13 +161,10 @@ void secureTransformerHidesUnparsedTextAvailability() { "secure Saxon unparsed-text-available still distinguishes an existing file from a missing one"); } - /** The {@code unparsed-text-available} answer under the secure factory, or {@code "blocked"} when the transform throws. */ - private static String availabilityUnderSecure(final String uri) { - try { - return transform(SecureTransformerFactory.secure(saxonFactory()), "unparsed-text-available('" + uri + "')").contains("true") ? "true" : "false"; - } catch (final TransformerException blocked) { - return "blocked"; - } + @Test + void unconfiguredTransformerLeaksJsonDoc() throws TransformerException { + assumeSaxonPresent(); + assertUnconfiguredLeaks("json-doc('" + url("referenced.json") + "')?leaked"); } @Test @@ -172,10 +178,4 @@ void unconfiguredTransformerLeaksUnparsedTextLines() throws TransformerException assumeSaxonPresent(); assertUnconfiguredLeaks("string-join(unparsed-text-lines('" + url("referenced.txt") + "'), ' ')"); } - - @Test - void unconfiguredTransformerLeaksJsonDoc() throws TransformerException { - assumeSaxonPresent(); - assertUnconfiguredLeaks("json-doc('" + url("referenced.json") + "')?leaked"); - } } diff --git a/src/test/java/org/apache/commons/xml/secure/SchemaContentModelLimitTest.java b/src/test/java/org/apache/commons/xml/secure/SchemaContentModelLimitTest.java index 370c6d0..0b4946f 100644 --- a/src/test/java/org/apache/commons/xml/secure/SchemaContentModelLimitTest.java +++ b/src/test/java/org/apache/commons/xml/secure/SchemaContentModelLimitTest.java @@ -44,6 +44,15 @@ class SchemaContentModelLimitTest { /** Above both recognized implementations' limits (3,000 nodes on Xerces, 5,000 on the stock JDK); an unbounded run still finishes in seconds. */ private static final int MAX_OCCURS = 10_000; + /** Compiles the payload through {@code factory} and validates a matching instance, the step that forces the expansion. */ + private static void compileAndValidate(final SchemaFactory factory) throws Exception { + factory.setErrorHandler(AttackTestSupport.STRICT_REPORTER); + final Schema schema = factory.newSchema(AttackTestSupport.streamSource(maxOccursPayload())); + final Validator validator = schema.newValidator(); + validator.setErrorHandler(AttackTestSupport.STRICT_REPORTER); + validator.validate(AttackTestSupport.streamSource("<root><a>x</a><b>y</b></root>")); + } + private static String maxOccursPayload() { return "<?xml version=\"1.0\"?>\n" + "<xs:schema xmlns:xs=\"http://www.w3.org/2001/XMLSchema\";>\n" @@ -59,21 +68,20 @@ private static String maxOccursPayload() { + "</xs:schema>\n"; } - /** Compiles the payload through {@code factory} and validates a matching instance, the step that forces the expansion. */ - private static void compileAndValidate(final SchemaFactory factory) throws Exception { - factory.setErrorHandler(AttackTestSupport.STRICT_REPORTER); - final Schema schema = factory.newSchema(AttackTestSupport.streamSource(maxOccursPayload())); - final Validator validator = schema.newValidator(); - validator.setErrorHandler(AttackTestSupport.STRICT_REPORTER); - validator.validate(AttackTestSupport.streamSource("<root><a>x</a><b>y</b></root>")); - } - @Test void secureSchemaBoundsContentModelExpansion() { AttackTestSupport.assertParseFails(() -> compileAndValidate(SecureSchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI)), "Schema content-model expansion", org.xml.sax.SAXException.class); } + @Test + void unconfiguredSchemaValidatesWhereTheLimitIsOptional() { + // Control: the payload is a valid schema and instance, so a rejection above is the limit firing and not a malformed fixture. It is skipped on an + // implementation that bounds the expansion unconditionally (the stock JDK), where there is no unbounded run to compare against. + final SchemaFactory factory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI); + AttackTestSupport.assumeDoesNotThrow(() -> compileAndValidate(factory)); + } + @Test void unconfiguredSchemaWithSecureProcessingBoundsContentModelExpansion() { // Control: the payload does trip the limit once secure processing is on, so a pass above is the limit firing rather than the payload being harmless. @@ -83,12 +91,4 @@ void unconfiguredSchemaWithSecureProcessingBoundsContentModelExpansion() { compileAndValidate(factory); }, "Schema content-model expansion", org.xml.sax.SAXException.class); } - - @Test - void unconfiguredSchemaValidatesWhereTheLimitIsOptional() { - // Control: the payload is a valid schema and instance, so a rejection above is the limit firing and not a malformed fixture. It is skipped on an - // implementation that bounds the expansion unconditionally (the stock JDK), where there is no unbounded run to compare against. - final SchemaFactory factory = SchemaFactory.newInstance(XMLConstants.W3C_XML_SCHEMA_NS_URI); - AttackTestSupport.assumeDoesNotThrow(() -> compileAndValidate(factory)); - } } diff --git a/src/test/java/org/apache/commons/xml/secure/SecureXMLFilterTest.java b/src/test/java/org/apache/commons/xml/secure/SecureXMLFilterTest.java index 8414a80..5f68d0b 100644 --- a/src/test/java/org/apache/commons/xml/secure/SecureXMLFilterTest.java +++ b/src/test/java/org/apache/commons/xml/secure/SecureXMLFilterTest.java @@ -201,24 +201,6 @@ void sendsLexicalEventsToALexicalContentHandler() throws Exception { filter.parse(new InputSource(new StringReader("<root><!--comment--><![CDATA[text]]></root>"))); } - @Test - void wrapsErrorHandlerFailuresAndUsesTheConfiguredParent() throws Exception { - final SecureXMLFilter filter = filter(); - final SAXException handlerFailure = new SAXException("error handler"); - filter.setErrorHandler(new DefaultHandler() { - - @Override - public void warning(final SAXParseException e) throws SAXException { - throw handlerFailure; - } - }); - final TransformerException warning = assertThrows(TransformerException.class, () -> filter.warning(new TransformerException("warning"))); - assertSame(handlerFailure, warning.getCause()); - filter.setContentHandler(new DefaultHandler()); - filter.setParent(SecureSAXParserFactory.newXMLReader(false)); - filter.parse(new InputSource(new StringReader("<root/>"))); - } - @Test void surfacesIoFailuresFromTheParentReader() throws Exception { final SecureXMLFilter filter = filter(); @@ -236,4 +218,22 @@ public void parse(final InputSource input) throws IOException { assertTrue(exception instanceof SAXException || exception instanceof IOException, "parse must fail with a declared exception type: " + exception); } + + @Test + void wrapsErrorHandlerFailuresAndUsesTheConfiguredParent() throws Exception { + final SecureXMLFilter filter = filter(); + final SAXException handlerFailure = new SAXException("error handler"); + filter.setErrorHandler(new DefaultHandler() { + + @Override + public void warning(final SAXParseException e) throws SAXException { + throw handlerFailure; + } + }); + final TransformerException warning = assertThrows(TransformerException.class, () -> filter.warning(new TransformerException("warning"))); + assertSame(handlerFailure, warning.getCause()); + filter.setContentHandler(new DefaultHandler()); + filter.setParent(SecureSAXParserFactory.newXMLReader(false)); + filter.parse(new InputSource(new StringReader("<root/>"))); + } } diff --git a/src/test/java/org/apache/commons/xml/secure/SecureXMLInputFactoryTest.java b/src/test/java/org/apache/commons/xml/secure/SecureXMLInputFactoryTest.java index 05b92e7..ea6703b 100644 --- a/src/test/java/org/apache/commons/xml/secure/SecureXMLInputFactoryTest.java +++ b/src/test/java/org/apache/commons/xml/secure/SecureXMLInputFactoryTest.java @@ -477,6 +477,18 @@ void setPropertyWrongTypeForResolverHookSurfacesDelegateException() { "the delegate must surface its own rejection of a wrong-typed resolver"); } + @Test + void settingAResolverInstallsAFreshFloorInsteadOfMutatingTheInstalledOne() { + // The implementations copy the floor reference into every reader they create, so mutating the installed floor would change the resolution policy of + // readers created before the call, including ones already parsing. Replacing it leaves what those readers captured alone. + final RecordingXMLInputFactory fake = new RecordingXMLInputFactory(); + final XMLInputFactory secure = SecureXMLInputFactory.secure(fake); + final Object captured = fake.resolverHook; + secure.setXMLResolver((publicID, systemID, baseURI, namespace) -> null); + assertNotSame(captured, fake.resolverHook, "setting a resolver must install a fresh floor, not re-delegate the one already on the hook"); + assertNull(((FallbackIgnoreXMLResolver) captured).getDelegate(), "the floor an existing reader captured must keep resolving to empty"); + } + @Test void setXMLResolverNullClearsCallerDelegate() { final RecordingXMLInputFactory fake = new RecordingXMLInputFactory(); @@ -526,6 +538,22 @@ void woodstoxDtdHookRoutesBehindInstalledFloor() { assertSame(second, factory.getProperty(SecureXMLInputFactory.WSTX_DTD_RESOLVER), "a second caller resolver must replace the first behind the floor"); } + @Test + void woodstoxResolverHooksStayIndependent() { + // Woodstox routes setXMLResolver to both its DTD-subset and entity hooks, so one floor object sits on several of them. Setting one hook must not + // answer the others, which it would if the shared floor were mutated in place. + final XMLInputFactory secure = SecureXMLInputFactory.newInstance(); + final XMLResolver dtd = (publicID, systemID, baseURI, namespace) -> null; + try { + secure.setProperty(SecureXMLInputFactory.WSTX_DTD_RESOLVER, dtd); + } catch (final IllegalArgumentException notWoodstox) { + Assumptions.abort("the implementation does not support " + SecureXMLInputFactory.WSTX_DTD_RESOLVER); + return; + } + assertSame(dtd, secure.getProperty(SecureXMLInputFactory.WSTX_DTD_RESOLVER), "the hook the caller named must report their resolver"); + assertNull(secure.getProperty(SecureXMLInputFactory.WSTX_ENTITY_RESOLVER), "a resolver set on the DTD hook must not answer the entity hook"); + } + @Test void woodstoxUndeclaredEntityHookWrapsCallerResolver() { final XMLInputFactory factory = SecureXMLInputFactory.newInstance(); @@ -610,34 +638,6 @@ void wrapperDelegatesReaderCreationToDelegate() throws Exception { "the exact stream filter must be forwarded"); } - @Test - void settingAResolverInstallsAFreshFloorInsteadOfMutatingTheInstalledOne() { - // The implementations copy the floor reference into every reader they create, so mutating the installed floor would change the resolution policy of - // readers created before the call, including ones already parsing. Replacing it leaves what those readers captured alone. - final RecordingXMLInputFactory fake = new RecordingXMLInputFactory(); - final XMLInputFactory secure = SecureXMLInputFactory.secure(fake); - final Object captured = fake.resolverHook; - secure.setXMLResolver((publicID, systemID, baseURI, namespace) -> null); - assertNotSame(captured, fake.resolverHook, "setting a resolver must install a fresh floor, not re-delegate the one already on the hook"); - assertNull(((FallbackIgnoreXMLResolver) captured).getDelegate(), "the floor an existing reader captured must keep resolving to empty"); - } - - @Test - void woodstoxResolverHooksStayIndependent() { - // Woodstox routes setXMLResolver to both its DTD-subset and entity hooks, so one floor object sits on several of them. Setting one hook must not - // answer the others, which it would if the shared floor were mutated in place. - final XMLInputFactory secure = SecureXMLInputFactory.newInstance(); - final XMLResolver dtd = (publicID, systemID, baseURI, namespace) -> null; - try { - secure.setProperty(SecureXMLInputFactory.WSTX_DTD_RESOLVER, dtd); - } catch (final IllegalArgumentException notWoodstox) { - Assumptions.abort("the implementation does not support " + SecureXMLInputFactory.WSTX_DTD_RESOLVER); - return; - } - assertSame(dtd, secure.getProperty(SecureXMLInputFactory.WSTX_DTD_RESOLVER), "the hook the caller named must report their resolver"); - assertNull(secure.getProperty(SecureXMLInputFactory.WSTX_ENTITY_RESOLVER), "a resolver set on the DTD hook must not answer the entity hook"); - } - @Test void wrapperInstallsFloorOnDelegateHook() { final RecordingXMLInputFactory fake = new RecordingXMLInputFactory(); diff --git a/src/test/java/org/apache/commons/xml/secure/SecureXPathFactoryTest.java b/src/test/java/org/apache/commons/xml/secure/SecureXPathFactoryTest.java index caaa5bd..bc270c4 100644 --- a/src/test/java/org/apache/commons/xml/secure/SecureXPathFactoryTest.java +++ b/src/test/java/org/apache/commons/xml/secure/SecureXPathFactoryTest.java @@ -40,6 +40,24 @@ @Tag("xpath") class SecureXPathFactoryTest { + /** A processing limit the JDK's XPath implementation recognizes through the Java 18 property API. */ + private static final String XPATH_GROUP_LIMIT = "jdk.xml.xpathExprGrpLimit"; + + /** + * The Java 18 {@code XPathFactory} property method of the given name, or an aborted test where the platform predates it. + * + * <p>Reached reflectively because this suite compiles against the Java 8 API, the same reason the wrapper delegates the pair through method handles: the + * call has to resolve at run time, which is also exactly how a Java 18 caller reaches it.</p> + */ + private static Method propertyMethod(final String name, final Class<?>... parameterTypes) { + try { + return XPathFactory.class.getMethod(name, parameterTypes); + } catch (final NoSuchMethodException e) { + Assumptions.abort("XPathFactory." + name + " requires Java 18 or later"); + throw new AssertionError("unreachable"); + } + } + @Test void createsAndConfiguresAFactoryForTheDefaultObjectModel() throws Exception { final XPathFactory factory = SecureXPathFactory.newInstance(XPathFactory.DEFAULT_OBJECT_MODEL_URI); @@ -59,6 +77,17 @@ void createsSecureXPathFromStaticEntryPoints() { assertInstanceOf(SecureXPath.class, SecureXPathFactory.newDefaultInstance().newXPath()); } + @Test + void delegatesTheJava18PropertyApi() throws Exception { + // The wrapper is compiled against the Java 8 API, so without an explicit delegation the inherited default answers for it and every property the + // implementation supports, including its own limits, becomes unreachable through a secured factory. + final Method setProperty = propertyMethod("setProperty", String.class, String.class); + final Method getProperty = propertyMethod("getProperty", String.class); + final XPathFactory factory = SecureXPathFactory.newDefaultInstance(); + setProperty.invoke(factory, XPATH_GROUP_LIMIT, "5"); + assertEquals("5", getProperty.invoke(factory, XPATH_GROUP_LIMIT), "a property set on the secured factory must be read back from the delegate"); + } + @Test void preservesANullXPathFromTheDelegate() { final XPathFactory delegate = new XPathFactory() { @@ -93,6 +122,15 @@ public void setXPathVariableResolver(final XPathVariableResolver resolver) { assertNull(SecureXPathFactory.secure(delegate).newXPath()); } + @Test + void reportsAnUnknownPropertyLikeTheDelegate() { + final Method getProperty = propertyMethod("getProperty", String.class); + final XPathFactory factory = SecureXPathFactory.newDefaultInstance(); + final InvocationTargetException thrown = assertThrows(InvocationTargetException.class, + () -> getProperty.invoke(factory, "jdk.xml.noSuchProperty")); + assertInstanceOf(IllegalArgumentException.class, thrown.getCause(), "an unrecognized property must surface the delegate's own rejection"); + } + @Test void wrapsARejectedRequiredFeatureInSecureException() { final XPathFactory rejectingFactory = new XPathFactory() { @@ -127,42 +165,4 @@ public void setXPathVariableResolver(final XPathVariableResolver resolver) { }; assertThrows(SecureException.class, () -> SecureXPathFactory.secure(rejectingFactory)); } - - /** A processing limit the JDK's XPath implementation recognizes through the Java 18 property API. */ - private static final String XPATH_GROUP_LIMIT = "jdk.xml.xpathExprGrpLimit"; - - /** - * The Java 18 {@code XPathFactory} property method of the given name, or an aborted test where the platform predates it. - * - * <p>Reached reflectively because this suite compiles against the Java 8 API, the same reason the wrapper delegates the pair through method handles: the - * call has to resolve at run time, which is also exactly how a Java 18 caller reaches it.</p> - */ - private static Method propertyMethod(final String name, final Class<?>... parameterTypes) { - try { - return XPathFactory.class.getMethod(name, parameterTypes); - } catch (final NoSuchMethodException e) { - Assumptions.abort("XPathFactory." + name + " requires Java 18 or later"); - throw new AssertionError("unreachable"); - } - } - - @Test - void delegatesTheJava18PropertyApi() throws Exception { - // The wrapper is compiled against the Java 8 API, so without an explicit delegation the inherited default answers for it and every property the - // implementation supports, including its own limits, becomes unreachable through a secured factory. - final Method setProperty = propertyMethod("setProperty", String.class, String.class); - final Method getProperty = propertyMethod("getProperty", String.class); - final XPathFactory factory = SecureXPathFactory.newDefaultInstance(); - setProperty.invoke(factory, XPATH_GROUP_LIMIT, "5"); - assertEquals("5", getProperty.invoke(factory, XPATH_GROUP_LIMIT), "a property set on the secured factory must be read back from the delegate"); - } - - @Test - void reportsAnUnknownPropertyLikeTheDelegate() { - final Method getProperty = propertyMethod("getProperty", String.class); - final XPathFactory factory = SecureXPathFactory.newDefaultInstance(); - final InvocationTargetException thrown = assertThrows(InvocationTargetException.class, - () -> getProperty.invoke(factory, "jdk.xml.noSuchProperty")); - assertInstanceOf(IllegalArgumentException.class, thrown.getCause(), "an unrecognized property must surface the delegate's own rejection"); - } }
