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 e7520cc20f1c6d9c4788a82acd76f8bc8577ac34 Author: Gary Gregory <[email protected]> AuthorDate: Tue Oct 6 11:29:57 2026 -0400 Sort members --- .../apache/commons/xml/secure/SecureException.java | 34 +++++++-------- .../secure/SecureDocumentBuilderFactoryTest.java | 50 ++++++++++++---------- .../xml/secure/SecureSAXParserFactoryTest.java | 16 +++---- 3 files changed, 52 insertions(+), 48 deletions(-) diff --git a/src/main/java/org/apache/commons/xml/secure/SecureException.java b/src/main/java/org/apache/commons/xml/secure/SecureException.java index eecfe9a..5447bc4 100644 --- a/src/main/java/org/apache/commons/xml/secure/SecureException.java +++ b/src/main/java/org/apache/commons/xml/secure/SecureException.java @@ -51,6 +51,23 @@ final class SecureException extends IllegalStateException { */ static final String THROW_ON_UNRESOLVED = "org.apache.commons.xml.secure.throwOnUnresolved"; + /** + * Builds the standard exception for a failure to create a parser or reader from an already secured factory. + * + * <p> + * Every supported implementation provides parsers and readers as a routine capability, and rejects an unsupported setting when it is set on the factory, + * not when a parser is built. The wrapped {@code ParserConfigurationException} or {@code SAXException} therefore signals a broken environment rather than + * a per-parse condition, so the exception is unchecked. + * </p> + * + * @param type The type of the object that could not be created, such as {@code DocumentBuilder}, {@code SAXParser} or {@code XMLReader}. + * @param cause The original checked exception from the JAXP implementation. + * @return the exception to throw. + */ + static SecureException creationFailed(final Class<?> type, final Throwable cause) { + return new SecureException("Failed to create a secure " + type.getSimpleName(), cause); + } + /** * Builds the standard exception for a rejected secure setting. * @param name The name of the feature, attribute or property that could not be set. @@ -78,23 +95,6 @@ static String forbidden(final String type, final String namespace, final String SecureException.THROW_ON_UNRESOLVED, type, namespace, publicId, systemId, baseURI); } - /** - * Builds the standard exception for a failure to create a parser or reader from an already secured factory. - * - * <p> - * Every supported implementation provides parsers and readers as a routine capability, and rejects an unsupported setting when it is set on the factory, - * not when a parser is built. The wrapped {@code ParserConfigurationException} or {@code SAXException} therefore signals a broken environment rather than - * a per-parse condition, so the exception is unchecked. - * </p> - * - * @param type The type of the object that could not be created, such as {@code DocumentBuilder}, {@code SAXParser} or {@code XMLReader}. - * @param cause The original checked exception from the JAXP implementation. - * @return the exception to throw. - */ - static SecureException creationFailed(final Class<?> type, final Throwable cause) { - return new SecureException("Failed to create a secure " + type.getSimpleName(), cause); - } - /** * Whether unresolved external references must be rejected instead of resolved to empty content. * diff --git a/src/test/java/org/apache/commons/xml/secure/SecureDocumentBuilderFactoryTest.java b/src/test/java/org/apache/commons/xml/secure/SecureDocumentBuilderFactoryTest.java index 463217b..75948e4 100644 --- a/src/test/java/org/apache/commons/xml/secure/SecureDocumentBuilderFactoryTest.java +++ b/src/test/java/org/apache/commons/xml/secure/SecureDocumentBuilderFactoryTest.java @@ -35,6 +35,9 @@ import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; +import org.w3c.dom.Document; +import org.xml.sax.InputSource; + import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.Tag; import org.junit.jupiter.api.Test; @@ -109,15 +112,6 @@ private static String setFactoryIdProperty(final String factoryClassName) { return previous; } - @Test - void createsSecureBuildersFromEveryStaticEntryPoint() throws Exception { - Assumptions.assumeTrue(AttackTestSupport.DOM_RESOLVES_INTERNAL_ENTITIES, "the platform DOM is left unwrapped: it does not resolve user-defined entities"); - assertInstanceOf(SecureDocumentBuilder.class, SecureDocumentBuilderFactory.newInstance().newDocumentBuilder()); - assertInstanceOf(SecureDocumentBuilder.class, SecureDocumentBuilderFactory.newDefaultInstance().newDocumentBuilder()); - assertInstanceOf(SecureDocumentBuilder.class, SecureDocumentBuilderFactory.newNSInstance().newDocumentBuilder()); - assertInstanceOf(SecureDocumentBuilder.class, SecureDocumentBuilderFactory.newDefaultNSInstance().newDocumentBuilder()); - } - @Test void createsBuildersDirectly() { final DocumentBuilder builder = SecureDocumentBuilderFactory.newNSDocumentBuilder(); @@ -128,20 +122,12 @@ void createsBuildersDirectly() { } @Test - // Mockito generates the mock classes and its plugin proxies at run time, which a closed-world native image cannot do. - @DisabledInNativeImage - void newDocumentBuilderWrapsDeclaredExceptions() throws Exception { - Assumptions.assumeFalse(AttackTestSupport.IS_ANDROID, "Skipped on Android: parser selection is pinned to the platform implementation"); - final String previous = setFactoryIdProperty(MockDocumentBuilderFactory.class.getName()); - try { - final ParserConfigurationException cause = new ParserConfigurationException("test"); - MockDocumentBuilderFactory.delegate = mock(DocumentBuilderFactory.class); - when(MockDocumentBuilderFactory.delegate.newDocumentBuilder()).thenThrow(cause); - assertSame(cause, assertThrows(IllegalStateException.class, SecureDocumentBuilderFactory::newNSDocumentBuilder).getCause()); - } finally { - setFactoryIdProperty(previous); - MockDocumentBuilderFactory.delegate = null; - } + void createsSecureBuildersFromEveryStaticEntryPoint() throws Exception { + Assumptions.assumeTrue(AttackTestSupport.DOM_RESOLVES_INTERNAL_ENTITIES, "the platform DOM is left unwrapped: it does not resolve user-defined entities"); + assertInstanceOf(SecureDocumentBuilder.class, SecureDocumentBuilderFactory.newInstance().newDocumentBuilder()); + assertInstanceOf(SecureDocumentBuilder.class, SecureDocumentBuilderFactory.newDefaultInstance().newDocumentBuilder()); + assertInstanceOf(SecureDocumentBuilder.class, SecureDocumentBuilderFactory.newNSInstance().newDocumentBuilder()); + assertInstanceOf(SecureDocumentBuilder.class, SecureDocumentBuilderFactory.newDefaultNSInstance().newDocumentBuilder()); } @Test @@ -180,6 +166,23 @@ void forwardsEverySupportedFactoryConfiguration() throws Exception { assertNotNull(factory.newDocumentBuilder()); } + @Test + // Mockito generates the mock classes and its plugin proxies at run time, which a closed-world native image cannot do. + @DisabledInNativeImage + void newDocumentBuilderWrapsDeclaredExceptions() throws Exception { + Assumptions.assumeFalse(AttackTestSupport.IS_ANDROID, "Skipped on Android: parser selection is pinned to the platform implementation"); + final String previous = setFactoryIdProperty(MockDocumentBuilderFactory.class.getName()); + try { + final ParserConfigurationException cause = new ParserConfigurationException("test"); + MockDocumentBuilderFactory.delegate = mock(DocumentBuilderFactory.class); + when(MockDocumentBuilderFactory.delegate.newDocumentBuilder()).thenThrow(cause); + assertSame(cause, assertThrows(IllegalStateException.class, SecureDocumentBuilderFactory::newNSDocumentBuilder).getCause()); + } finally { + setFactoryIdProperty(previous); + MockDocumentBuilderFactory.delegate = null; + } + } + @Test void newNSInstanceFollowsParserSelection() throws Exception { Assumptions.assumeFalse(AttackTestSupport.IS_ANDROID, "Skipped on Android: the platform factory is used unwrapped"); @@ -197,4 +200,5 @@ void newNSInstanceFollowsParserSelection() throws Exception { setFactoryIdProperty(previous); } } + } diff --git a/src/test/java/org/apache/commons/xml/secure/SecureSAXParserFactoryTest.java b/src/test/java/org/apache/commons/xml/secure/SecureSAXParserFactoryTest.java index cfa69b0..469cff4 100644 --- a/src/test/java/org/apache/commons/xml/secure/SecureSAXParserFactoryTest.java +++ b/src/test/java/org/apache/commons/xml/secure/SecureSAXParserFactoryTest.java @@ -123,14 +123,6 @@ private static String setFactoryIdProperty(final String factoryClassName) { return previous; } - @Test - void createsSecureParsersFromEveryStaticEntryPoint() throws Exception { - assertNotNull(SecureSAXParserFactory.newInstance().newSAXParser()); - assertNotNull(SecureSAXParserFactory.newDefaultInstance().newSAXParser()); - assertNotNull(SecureSAXParserFactory.newNSInstance().newSAXParser()); - assertNotNull(SecureSAXParserFactory.newDefaultNSInstance().newSAXParser()); - } - @Test void createsParsersAndReadersDirectly() throws Exception { final SAXParser parser = SecureSAXParserFactory.newNSSAXParser(); @@ -148,6 +140,14 @@ void createsParsersAndReadersDirectly() throws Exception { assertNull(SecureSAXParserFactory.newNSXMLReader(null).getContentHandler()); } + @Test + void createsSecureParsersFromEveryStaticEntryPoint() throws Exception { + assertNotNull(SecureSAXParserFactory.newInstance().newSAXParser()); + assertNotNull(SecureSAXParserFactory.newDefaultInstance().newSAXParser()); + assertNotNull(SecureSAXParserFactory.newNSInstance().newSAXParser()); + assertNotNull(SecureSAXParserFactory.newDefaultNSInstance().newSAXParser()); + } + @Test void forwardsFactoryConfigurationAndCreatesNamespaceAwareParsers() throws Exception { final SAXParserFactory factory = SecureSAXParserFactory.newInstance();
