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
The following commit(s) were added to refs/heads/main by this push:
new 5f09671 Add helpers that return a parser directly (COMMONSXML-18)
(#104)
5f09671 is described below
commit 5f096717316b1ab87b0787621e495ee2025ab56c
Author: Piotr P. Karwasz <[email protected]>
AuthorDate: Sat Sep 26 16:02:52 2026 +0200
Add helpers that return a parser directly (COMMONSXML-18) (#104)
* Add helpers that return a parser directly (COMMONSXML-18)
Most callers want a parser, not a factory. Add static helpers that build one
from a freshly secured factory:
- SecureDocumentBuilderFactory.newDocumentBuilder() and
newNSDocumentBuilder()
- SecureSAXParserFactory.newSAXParser() and newNSSAXParser()
- SecureSAXParserFactory.newXMLReader(ContentHandler) and
newNSXMLReader(ContentHandler), which register the given handler, if any
The checked exceptions they would declare cannot occur once the factory is
configured, so they are reported as IllegalStateException. The Javadoc
recommends reusing the returned parser, with reset(), over caching a
factory.
Bump the version to 1.1.0, since this adds public API.
Assisted-By: Claude Opus 5.5 <[email protected]>
* Add @since tag to newDocumentBuilder methods
* Add @since 1.1.0 annotation to parser methods
* Keep only the namespace-aware parser helpers
Remove newDocumentBuilder(), newSAXParser() and
newXMLReader(ContentHandler):
explicitly non-namespace-aware parsing is rare, and on Android a factory
that
never set namespace awareness produces namespace-aware SAX parsers, so the
plain variants could not keep their promise without extra configuration.
They can be added later if needed.
Add a newNSXMLReader() overload for callers that register no content
handler,
the majority of getXMLReader() call sites.
Assisted-By: Claude Opus 5.5 <[email protected]>
---------
Co-authored-by: Gary Gregory <[email protected]>
---
pom.xml | 6 +-
src/changes/changes.xml | 3 +-
.../xml/secure/SecureDocumentBuilderFactory.java | 24 ++++++
.../apache/commons/xml/secure/SecureException.java | 14 ++--
.../commons/xml/secure/SecureSAXParserFactory.java | 88 +++++++++++++++++++++-
src/main/javadoc/overview.html | 36 +++++++--
.../secure/SecureDocumentBuilderFactoryTest.java | 66 ++++++++++++++++
.../commons/xml/secure/SecureExceptionTest.java | 2 +-
.../xml/secure/SecureSAXParserFactoryTest.java | 29 ++++++-
9 files changed, 245 insertions(+), 23 deletions(-)
diff --git a/pom.xml b/pom.xml
index 7666d35..196b353 100644
--- a/pom.xml
+++ b/pom.xml
@@ -24,7 +24,7 @@ limitations under the License.
<version>105</version>
</parent>
<artifactId>commons-secure-xml</artifactId>
- <version>1.0.1-SNAPSHOT</version>
+ <version>1.1.0-SNAPSHOT</version>
<name>Apache Commons Secure XML</name>
<url>https://commons.apache.org/proper/commons-secure-xml/</url>
<inceptionYear>2026</inceptionYear>
@@ -40,11 +40,11 @@ limitations under the License.
<properties>
<!-- Release-related properties -->
<commons.main.branch>main</commons.main.branch>
- <commons.release.version>1.0.1</commons.release.version>
+ <commons.release.version>1.1.0</commons.release.version>
<commons.release.desc>(Java 8+)</commons.release.desc>
<commons.rc.version>RC1</commons.rc.version>
<commons.bc.version>1.0.0</commons.bc.version>
- <commons.release.next>1.0.2</commons.release.next>
+ <commons.release.next>1.1.1</commons.release.next>
<commons.componentid>secure-xml</commons.componentid>
<commons.packageId>secure-xml</commons.packageId>
<commons.module.name>org.apache.commons.xml.secure</commons.module.name>
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 54e3d81..fcfb716 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -31,7 +31,7 @@ The <action> type attribute can be add, update, fix, or
remove.
<title>Apache Commons Secure XML Changes</title>
</properties>
<body>
- <release version="1.0.1" date="YYYY-MM-DD" description="Second release.
Requires Java 8 or later.">
+ <release version="1.1.0" date="YYYY-MM-DD" description="Second release.
Requires Java 8 or later.">
<!-- FIX -->
<action type="fix" dev="ggregory" due-to="Gary Gregory">Fix the
OpenRewrite migration recipe to target static method calls instead of class
references.</action>
<action type="fix" dev="ggregory" due-to="Gary Gregory">Fix the
OpenRewrite migration recipe to add a dependency on
org.apache.commons:commons-secure-xml:1.0.0.</action>
@@ -42,6 +42,7 @@ The <action> type attribute can be add, update, fix, or
remove.
<action type="fix" dev="pkarwasz" due-to="Piotr P. Karwasz, Gary
Gregory">Report a stylesheet that produces no XMLFilter as a
TransformerConfigurationException instead of returning null.</action>
<action type="fix" dev="ggregory" due-to="Gary Gregory">General Javadoc
and site documentation improvements.</action>
<!-- ADD -->
+ <action type="add" dev="pkarwasz" due-to="Piotr P. Karwasz, Gary
Gregory" issue="COMMONSXML-18">Add helpers that return a secure,
namespace-aware DocumentBuilder, SAXParser or XMLReader directly, without the
factory.</action>
<!-- UPDATE -->
<action dev="ggregory" type="update" due-to="Gary Gregory">Bump
org.apache.commons:commons-parent from 104 to 105.</action>
<action dev="pkarwasz" type="update" due-to="Piotr P. Karwasz, Gary
Gregory">Bump com.android.library from 8.6.1 to 9.4.0 in
/android-tests.</action>
diff --git
a/src/main/java/org/apache/commons/xml/secure/SecureDocumentBuilderFactory.java
b/src/main/java/org/apache/commons/xml/secure/SecureDocumentBuilderFactory.java
index 8b50c83..9b709f6 100644
---
a/src/main/java/org/apache/commons/xml/secure/SecureDocumentBuilderFactory.java
+++
b/src/main/java/org/apache/commons/xml/secure/SecureDocumentBuilderFactory.java
@@ -269,6 +269,30 @@ public static DocumentBuilderFactory newInstance(final
String factoryClassName,
return secure(DocumentBuilderFactory.newInstance(factoryClassName,
classLoader));
}
+ /**
+ * Creates a new, secure, namespace-aware {@link DocumentBuilder} from
{@link #newNSInstance()}.
+ * <p>
+ * No factory is cached: each call configures a fresh one. To parse many
documents, keep the returned builder and call {@link DocumentBuilder#reset()}
+ * between documents. Reusing the builder saves more than caching the
factory would, and {@code reset()} costs next to nothing while keeping handler
state
+ * from leaking between parses. A builder is not thread-safe, so reuse it
within one thread.
+ * </p>
+ *
+ * @return A secure, namespace-aware builder.
+ * @throws IllegalStateException Thrown if a required secure setting
cannot be applied to the underlying implementation, or if the implementation
cannot
+ * create a builder.
+ * @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.
+ * @since 1.1.0
+ */
+ public static DocumentBuilder newNSDocumentBuilder() {
+ try {
+ return newNSInstance().newDocumentBuilder();
+ } catch (final ParserConfigurationException e) {
+ // Implementations reject settings when they are set on the
factory, not here: a failure means a broken environment.
+ throw SecureException.creationFailed(DocumentBuilder.class, e);
+ }
+ }
+
/**
* Returns a new, secure, namespace-aware {@link DocumentBuilderFactory},
enabling namespace awareness on {@link #newInstance()}, the behavior
* {@code DocumentBuilderFactory.newNSInstance()} (Java 13 or later) is
specified to have.
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 b75ad41..eecfe9a 100644
--- a/src/main/java/org/apache/commons/xml/secure/SecureException.java
+++ b/src/main/java/org/apache/commons/xml/secure/SecureException.java
@@ -26,7 +26,7 @@
* <ul>
* <li>No bundled secure recipe matches the concrete factory class.</li>
* <li>A recipe tried to apply a secure setting and the implementation
rejected it.</li>
- * <li>The implementation could not provide the internal secure reader the
Source-rewriting wrappers parse with.</li>
+ * <li>The implementation could not provide a secure parser or reader from
an already secured factory.</li>
* </ul>
*
* <p>
@@ -79,18 +79,20 @@ static String forbidden(final String type, final String
namespace, final String
}
/**
- * Builds the standard exception for a failure to provision an internal
reader.
+ * Builds the standard exception for a failure to create a parser or
reader from an already secured factory.
*
* <p>
- * Every supported implementation provides a reader as a routine
capability, so the wrapped {@code ParserConfigurationException} or
- * {@code SAXException} signals a broken environment rather than a
per-parse condition. The exception is therefore unchecked.
+ * 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 readerFailed(final Throwable cause) {
- return new SecureException("Failed to create a secure XMLReader",
cause);
+ static SecureException creationFailed(final Class<?> type, final Throwable
cause) {
+ return new SecureException("Failed to create a secure " +
type.getSimpleName(), cause);
}
/**
diff --git
a/src/main/java/org/apache/commons/xml/secure/SecureSAXParserFactory.java
b/src/main/java/org/apache/commons/xml/secure/SecureSAXParserFactory.java
index 62ca0e4..31a98f0 100644
--- a/src/main/java/org/apache/commons/xml/secure/SecureSAXParserFactory.java
+++ b/src/main/java/org/apache/commons/xml/secure/SecureSAXParserFactory.java
@@ -30,6 +30,7 @@
import javax.xml.transform.stream.StreamSource;
import javax.xml.validation.Schema;
+import org.xml.sax.ContentHandler;
import org.xml.sax.EntityResolver;
import org.xml.sax.InputSource;
import org.xml.sax.SAXException;
@@ -308,6 +309,69 @@ public static SAXParserFactory newNSInstance(final String
factoryClassName, fina
return makeNSAware(newInstance(factoryClassName, classLoader));
}
+ /**
+ * Creates a new, secure, namespace-aware {@link SAXParser} from {@link
#newNSInstance()}.
+ * <p>
+ * No factory is cached: each call configures a fresh one. To parse many
documents, keep the returned parser and call {@link SAXParser#reset()} between
+ * documents. Reusing the parser saves more than caching the factory
would, and {@code reset()} costs next to nothing while keeping handler state
from
+ * leaking between parses. A parser is not thread-safe, so reuse it within
one thread.
+ * </p>
+ *
+ * @return A secure, namespace-aware parser.
+ * @throws IllegalStateException Thrown if a required secure setting
cannot be applied to the underlying implementation, or if the implementation
cannot
+ * create a parser.
+ * @throws FactoryConfigurationError Thrown from {@link SAXParserFactory}
in case of a {@link java.util.ServiceConfigurationError service configuration
+ * error} or if the implementation is
not available or cannot be instantiated.
+ * @since 1.1.0
+ */
+ public static SAXParser newNSSAXParser() {
+ try {
+ return newNSInstance().newSAXParser();
+ } catch (final ParserConfigurationException | SAXException e) {
+ // Implementations reject settings when they are set on the
factory, not here: a failure means a broken environment.
+ throw SecureException.creationFailed(SAXParser.class, e);
+ }
+ }
+
+ /**
+ * Creates a new, secure, namespace-aware {@link XMLReader} from {@link
#newNSInstance()}, with no content handler registered.
+ * <p>
+ * No factory is cached: each call configures a fresh one. To parse many
documents, keep the returned reader and parse each document with it. Reusing the
+ * reader saves more than caching the factory would. Handlers set on the
reader stay set between parses, and a reader is not thread-safe, so reuse it
within
+ * one thread.
+ * </p>
+ *
+ * @return A secure, namespace-aware reader.
+ * @throws IllegalStateException Thrown if a required secure setting
cannot be applied to the underlying implementation, or if the implementation
cannot
+ * create a reader.
+ * @throws FactoryConfigurationError Thrown from {@link SAXParserFactory}
in case of a {@link java.util.ServiceConfigurationError service configuration
+ * error} or if the implementation is
not available or cannot be instantiated.
+ * @since 1.1.0
+ */
+ public static XMLReader newNSXMLReader() {
+ return newNSXMLReader(null);
+ }
+
+ /**
+ * Creates a new, secure, namespace-aware {@link XMLReader} from {@link
#newNSInstance()}.
+ * <p>
+ * No factory is cached: each call configures a fresh one. To parse many
documents, keep the returned reader and parse each document with it. Reusing the
+ * reader saves more than caching the factory would. Handlers set on the
reader stay set between parses, and a reader is not thread-safe, so reuse it
within
+ * one thread.
+ * </p>
+ *
+ * @param handler The content handler to register on the reader, or {@code
null} to register none.
+ * @return A secure, namespace-aware reader.
+ * @throws IllegalStateException Thrown if a required secure setting
cannot be applied to the underlying implementation, or if the implementation
cannot
+ * create a reader.
+ * @throws FactoryConfigurationError Thrown from {@link SAXParserFactory}
in case of a {@link java.util.ServiceConfigurationError service configuration
+ * error} or if the implementation is
not available or cannot be instantiated.
+ * @since 1.1.0
+ */
+ public static XMLReader newNSXMLReader(final ContentHandler handler) {
+ return newXMLReader(newNSInstance(), handler);
+ }
+
/**
* Creates a new secure, namespace-aware {@link XMLReader} for the TrAX,
XPath and schema wrappers to parse sources with, from the factory
* {@link #newNSInstance(boolean)} selects.
@@ -320,11 +384,29 @@ public static SAXParserFactory newNSInstance(final String
factoryClassName, fina
* configuration error} or if the
implementation is not available or cannot be instantiated.
*/
static XMLReader newXMLReader(final boolean overrideDefaultParser) {
+ return newXMLReader(newNSInstance(overrideDefaultParser), null);
+ }
+
+ /**
+ * Creates a new {@link XMLReader} from the given secure factory.
+ *
+ * @param factory The secure factory; never {@code null}.
+ * @param handler The content handler to register on the reader, or {@code
null} to register none.
+ * @return A secure reader.
+ * @throws IllegalStateException Thrown if the factory cannot create a
reader.
+ */
+ private static XMLReader newXMLReader(final SAXParserFactory factory,
final ContentHandler handler) {
+ final XMLReader reader;
try {
- return
newNSInstance(overrideDefaultParser).newSAXParser().getXMLReader();
- } catch (ParserConfigurationException | SAXException e) {
- throw SecureException.readerFailed(e);
+ reader = factory.newSAXParser().getXMLReader();
+ } catch (final ParserConfigurationException | SAXException e) {
+ // Implementations reject settings when they are set on the
factory, not here: a failure means a broken environment.
+ throw SecureException.creationFailed(XMLReader.class, e);
+ }
+ if (handler != null) {
+ reader.setContentHandler(handler);
}
+ return reader;
}
/**
diff --git a/src/main/javadoc/overview.html b/src/main/javadoc/overview.html
index aca0263..fe74a9c 100644
--- a/src/main/javadoc/overview.html
+++ b/src/main/javadoc/overview.html
@@ -174,7 +174,7 @@ <h2>Supported Implementations</h2>
import org.w3c.dom.Document;
import org.apache.commons.xml.secure.SecureDocumentBuilderFactory;
-Document doc =
SecureDocumentBuilderFactory.newInstance().newDocumentBuilder().parse(inputStream);
+Document doc =
SecureDocumentBuilderFactory.newNSDocumentBuilder().parse(inputStream);
</code>
</pre>
</div>
@@ -187,8 +187,13 @@ <h2>Supported Implementations</h2>
<pre class="sourceCode java">
<code class="sourceCode java">
import org.apache.commons.xml.secure.SecureSAXParserFactory;
+import org.xml.sax.InputSource;
+
+// With a SAXParser and a DefaultHandler
+SecureSAXParserFactory.newNSSAXParser().parse(inputStream, myDefaultHandler);
-SecureSAXParserFactory.newInstance().newSAXParser().parse(inputStream,
myDefaultHandler);
+// With an XMLReader and a ContentHandler
+SecureSAXParserFactory.newNSXMLReader(myContentHandler).parse(new
InputSource(inputStream));
</code>
</pre>
</div>
@@ -308,6 +313,17 @@ <h2>Factory Methods</h2>
suits a library with minimal XML requirements, which can parse with
the well-known platform parser rather than delegate the choice of
implementation to
the application developer.
</p>
+ <p>
+ Most callers never configure the factory: they want a parser.
+ For them, the DOM and SAX factory classes add helpers that return a
namespace-aware one directly,
+ built on <code>newNSInstance()</code>:
+ <code>newNSDocumentBuilder()</code>,
+ <code>newNSSAXParser()</code>,
+ and <code>newNSXMLReader()</code>, with an overload taking the
<code>ContentHandler</code> to register on the reader.
+ A caller that needs a parser without namespace awareness, or any other
setting, configures the factory instead.
+ They report a parser the configured factory fails to create as an
unchecked <code>IllegalStateException</code>,
+ since a supported implementation rejects a setting when it is set, not
when a parser is built.
+ </p>
</section>
<section id="stylesheets-and-schemas">
<h2>Stylesheets and Schemas</h2>
@@ -350,10 +366,18 @@ <h2>Transformer Handlers and Filters</h2>
<section id="caching-and-thread-safety">
<h2>Caching and Thread Safety</h2>
<p>
- There is no caching or pooling inside
- <code>org.apache.commons.xml.secure</code>
- ; callers on a hot path are responsible for their own caching. The
returned factories inherit the thread-safety properties of the underlying JAXP
- implementation, which in practice means they are not thread-safe.
Create a new factory per thread or synchronize externally.
+ There is no caching or pooling inside
<code>org.apache.commons.xml.secure</code>;
+ callers on a hot path are responsible for their own caching.
+ The returned factories inherit the thread-safety properties of the
underlying JAXP implementation,
+ which in practice means they are not thread-safe.
+ Create a new factory per thread or synchronize externally.
+ </p>
+ <p>
+ To parse many documents, reuse the parser rather than the factory.
+ Creating and securing a factory costs a fixed few microseconds per
call, and creating a parser from it costs about as much again;
+ both are noticeable only for tiny documents.
+ Keeping a <code>DocumentBuilder</code> or <code>SAXParser</code> and
calling its <code>reset()</code> method between documents saves both,
+ and <code>reset()</code> itself costs next to nothing while keeping
handler state from leaking between parses.
</p>
</section>
</section>
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 bddf0a0..463217b 100644
---
a/src/test/java/org/apache/commons/xml/secure/SecureDocumentBuilderFactoryTest.java
+++
b/src/test/java/org/apache/commons/xml/secure/SecureDocumentBuilderFactoryTest.java
@@ -22,20 +22,60 @@
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
import java.lang.reflect.Field;
import javax.xml.XMLConstants;
+import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
+import javax.xml.parsers.ParserConfigurationException;
import org.junit.jupiter.api.Assumptions;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.condition.DisabledInNativeImage;
@Tag("dom")
class SecureDocumentBuilderFactoryTest {
+ /**
+ * Test JAXP provider that delegates builder creation to a Mockito mock.
+ */
+ public static final class MockDocumentBuilderFactory extends
DocumentBuilderFactory {
+
+ private static DocumentBuilderFactory delegate;
+
+ @Override
+ public Object getAttribute(final String name) {
+ return null;
+ }
+
+ @Override
+ public boolean getFeature(final String name) {
+ return false;
+ }
+
+ @Override
+ public DocumentBuilder newDocumentBuilder() throws
ParserConfigurationException {
+ return delegate.newDocumentBuilder();
+ }
+
+ @Override
+ public void setAttribute(final String name, final Object value) {
+ // no-op
+ }
+
+ @Override
+ public void setFeature(final String name, final boolean value) {
+ // no-op
+ }
+ }
+
/**
* System property naming the {@link DocumentBuilderFactory}
implementation, the JVM's mechanism for reconfiguring the default parser.
*/
@@ -78,6 +118,32 @@ void createsSecureBuildersFromEveryStaticEntryPoint()
throws Exception {
assertInstanceOf(SecureDocumentBuilder.class,
SecureDocumentBuilderFactory.newDefaultNSInstance().newDocumentBuilder());
}
+ @Test
+ void createsBuildersDirectly() {
+ final DocumentBuilder builder =
SecureDocumentBuilderFactory.newNSDocumentBuilder();
+ assertTrue(builder.isNamespaceAware());
+ if (AttackTestSupport.DOM_RESOLVES_INTERNAL_ENTITIES) {
+ assertInstanceOf(SecureDocumentBuilder.class, builder);
+ }
+ }
+
+ @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 explicitFactoryClassSelectsThatImplementation() throws Exception {
Assumptions.assumeFalse(AttackTestSupport.IS_ANDROID, "Skipped on
Android: the platform factory is used unwrapped");
diff --git
a/src/test/java/org/apache/commons/xml/secure/SecureExceptionTest.java
b/src/test/java/org/apache/commons/xml/secure/SecureExceptionTest.java
index c635870..9c32a80 100644
--- a/src/test/java/org/apache/commons/xml/secure/SecureExceptionTest.java
+++ b/src/test/java/org/apache/commons/xml/secure/SecureExceptionTest.java
@@ -30,7 +30,7 @@ void formatsFailuresAndReadsUnresolvedProperty() {
final RuntimeException cause = new RuntimeException("cause");
assertSame(cause, SecureException.featureFailed("feature", this,
cause).getCause());
assertTrue(SecureException.forbidden("type", "namespace", "public",
"system", "base").contains("system"));
- assertSame(cause, SecureException.readerFailed(cause).getCause());
+ assertSame(cause, SecureException.creationFailed(Object.class,
cause).getCause());
System.clearProperty(SecureException.THROW_ON_UNRESOLVED);
assertFalse(SecureException.throwOnUnresolved());
System.setProperty(SecureException.THROW_ON_UNRESOLVED, "true");
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 ebd5547..cfa69b0 100644
---
a/src/test/java/org/apache/commons/xml/secure/SecureSAXParserFactoryTest.java
+++
b/src/test/java/org/apache/commons/xml/secure/SecureSAXParserFactoryTest.java
@@ -47,6 +47,7 @@
import org.xml.sax.InputSource;
import org.xml.sax.SAXException;
import org.xml.sax.XMLReader;
+import org.xml.sax.helpers.DefaultHandler;
@Tag("sax")
public class SecureSAXParserFactoryTest {
@@ -79,16 +80,19 @@ public void setFeature(final String name, final boolean
value) {
*/
private static final String FACTORY_ID =
"javax.xml.parsers.SAXParserFactory";
+ private static final String NAMESPACES_FEATURE =
"http://xml.org/sax/features/namespaces";;
+
/**
- * Asserts {@link SecureSAXParserFactory#newXMLReader(boolean)} on the
given delegate throws {@link IllegalStateException} with the given cause.
+ * Asserts every {@code newXMLReader} and {@code newNSXMLReader} method on
the given delegate throws {@link IllegalStateException} with the given cause.
*
* @param cause The checked exception the delegate is stubbed to throw.
* @param delegate The stubbed factory to route {@link
MockSAXParserFactory} to.
*/
private static void assertNewXmlReaderWraps(final Exception cause, final
SAXParserFactory delegate) {
MockSAXParserFactory.delegate = delegate;
- final IllegalStateException exception =
assertThrows(IllegalStateException.class, () ->
SecureSAXParserFactory.newXMLReader(false));
- assertSame(cause, exception.getCause());
+ assertSame(cause, assertThrows(IllegalStateException.class, () ->
SecureSAXParserFactory.newXMLReader(false)).getCause());
+ assertSame(cause, assertThrows(IllegalStateException.class, () ->
SecureSAXParserFactory.newNSXMLReader()).getCause());
+ assertSame(cause, assertThrows(IllegalStateException.class, () ->
SecureSAXParserFactory.newNSXMLReader(null)).getCause());
}
/**
@@ -127,6 +131,23 @@ void createsSecureParsersFromEveryStaticEntryPoint()
throws Exception {
assertNotNull(SecureSAXParserFactory.newDefaultNSInstance().newSAXParser());
}
+ @Test
+ void createsParsersAndReadersDirectly() throws Exception {
+ final SAXParser parser = SecureSAXParserFactory.newNSSAXParser();
+ assertInstanceOf(SecureSAXParser.class, parser);
+ assertTrue(parser.isNamespaceAware());
+ final DefaultHandler handler = new DefaultHandler();
+ final XMLReader reader =
SecureSAXParserFactory.newNSXMLReader(handler);
+ assertInstanceOf(SecureXMLReader.class, reader);
+ assertSame(handler, reader.getContentHandler());
+ assertTrue(reader.getFeature(NAMESPACES_FEATURE));
+ final XMLReader noHandlerReader =
SecureSAXParserFactory.newNSXMLReader();
+ assertInstanceOf(SecureXMLReader.class, noHandlerReader);
+ assertNull(noHandlerReader.getContentHandler());
+ assertTrue(noHandlerReader.getFeature(NAMESPACES_FEATURE));
+
assertNull(SecureSAXParserFactory.newNSXMLReader(null).getContentHandler());
+ }
+
@Test
void forwardsFactoryConfigurationAndCreatesNamespaceAwareParsers() throws
Exception {
final SAXParserFactory factory = SecureSAXParserFactory.newInstance();
@@ -201,10 +222,12 @@ void newXmlReaderWrapsDeclaredExceptions() throws
Exception {
SAXParserFactory factory = mock(SAXParserFactory.class);
when(factory.newSAXParser()).thenThrow(notConfigurable);
assertNewXmlReaderWraps(notConfigurable, factory);
+ assertSame(notConfigurable,
assertThrows(IllegalStateException.class,
SecureSAXParserFactory::newNSSAXParser).getCause());
final SAXException noParser = new SAXException("test");
factory = mock(SAXParserFactory.class);
when(factory.newSAXParser()).thenThrow(noParser);
assertNewXmlReaderWraps(noParser, factory);
+ assertSame(noParser, assertThrows(IllegalStateException.class,
SecureSAXParserFactory::newNSSAXParser).getCause());
// SAXParser.getXMLReader() declares SAXException
final SAXException noReader = new SAXException("test");
factory = mock(SAXParserFactory.class);
