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 fb87d7e Substitute an InputSource when an XMLFilter is parsed with
`null` (#86)
fb87d7e is described below
commit fb87d7e8471e850600f5e043c789e8c226856a27
Author: Piotr P. Karwasz <[email protected]>
AuthorDate: Sun Sep 13 19:02:51 2026 +0200
Substitute an InputSource when an XMLFilter is parsed with `null` (#86)
* Substitute an InputSource when an XMLFilter is parsed with none.
A caller that drives an XMLFilter from a self-driven parent reader has no
InputSource to hand it: unmarshalling a SAXSource that carries only a
reader enters parse with a null one. Most TrAX implementations dereference
the InputSource of a SAXSource without a check, so pass on an empty one
under a placeholder URN rather than the null.
Assisted-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01AaDyR9HjZLWkFn42x7kje3
* fix: add reference to Xalan bug
* fix: add reference to PR in changelog
---
src/changes/changes.xml | 1 +
.../apache/commons/xml/secure/SecureXMLFilter.java | 7 +-
.../apache/commons/xml/secure/XMLFilterTest.java | 129 ++++++++++++---------
3 files changed, 84 insertions(+), 53 deletions(-)
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 007e79e..708c972 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -35,6 +35,7 @@ The <action> type attribute can be add, update, fix, or
remove.
<!-- 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>
+ <action type="fix" dev="pkarwasz" due-to="Piotr P. Karwasz, Gary
Gregory">Fix a NullPointerException when an XMLFilter with a self-driven parent
reader is parsed with a null InputSource. #86</action>
<!-- ADD -->
<!-- UPDATE -->
<action dev="ggregory" type="update" due-to="Gary Gregory">Bump
org.apache.commons:commons-parent from 104 to 105.</action>
diff --git a/src/main/java/org/apache/commons/xml/secure/SecureXMLFilter.java
b/src/main/java/org/apache/commons/xml/secure/SecureXMLFilter.java
index 3f8f89c..1287c65 100644
--- a/src/main/java/org/apache/commons/xml/secure/SecureXMLFilter.java
+++ b/src/main/java/org/apache/commons/xml/secure/SecureXMLFilter.java
@@ -49,6 +49,9 @@
*/
final class SecureXMLFilter extends XMLFilterImpl implements ErrorListener {
+ /** System id of the {@link InputSource} substituted for a caller's {@code
null} one in {@link #parse(InputSource)}; a URN, so nothing can fetch it. */
+ static final String NO_INPUT_SYSTEM_ID =
"urn:uuid:a47f732e-9111-42db-b648-5e24b7d663f3";
+
/**
* Bridges a TrAX report to the SAX callback shape.
*
@@ -137,7 +140,9 @@ public void parse(final InputSource input) throws
SAXException, IOException {
final Transformer transformer = templates.newTransformer();
// The filter is the listener, so TrAX error reports reach the
caller-set ErrorHandler like the parent reader's SAX reports do.
transformer.setErrorListener(this);
- transformer.transform(new SAXSource(parent, input), result);
+ // A self-driven parent needs no InputSource, so a caller may pass
null here; most TrAX implementations dereference the one they get unchecked.
+ // See: https://issues.apache.org/jira/browse/XALANJ-2851
+ transformer.transform(new SAXSource(parent, input != null ? input
: new InputSource(NO_INPUT_SYSTEM_ID)), result);
} catch (final TransformerException e) {
// The parent reader's parse errors and the handler's own
exceptions arrive wrapped; rethrow the original rather than nesting the
hierarchies.
final Throwable cause = e.getCause();
diff --git a/src/test/java/org/apache/commons/xml/secure/XMLFilterTest.java
b/src/test/java/org/apache/commons/xml/secure/XMLFilterTest.java
index 9cc55c8..d57e355 100644
--- a/src/test/java/org/apache/commons/xml/secure/XMLFilterTest.java
+++ b/src/test/java/org/apache/commons/xml/secure/XMLFilterTest.java
@@ -56,6 +56,66 @@
@Tag("trax")
class XMLFilterTest {
+ /**
+ * Parent reader that drives itself, the shape a caller hands the filter
when the source document does not come from an {@code InputSource}.
+ *
+ * <p>Records the callbacks the filter wires onto it, and the {@code
InputSource} it is driven with.</p>
+ */
+ private static final class SelfDrivenParent extends XMLFilterImpl {
+
+ final List<InputSource> parsed = new ArrayList<>();
+
+ final List<Object> wired = new ArrayList<>();
+
+ @Override
+ public boolean getFeature(final String name) {
+ // Accept the namespace probes implementations make on a SAXSource
reader; there is no parent to delegate to.
+ return "http://xml.org/sax/features/namespaces".equals(name);
+ }
+
+ @Override
+ public Object getProperty(final String name) {
+ return null;
+ }
+
+ @Override
+ public void parse(final InputSource input) throws SAXException {
+ parsed.add(input);
+ // Minimal well-formed document for the transformation to consume;
no real parser behind this parent.
+ final ContentHandler handler = getContentHandler();
+ handler.startDocument();
+ handler.startElement("", "root", "root", new AttributesImpl());
+ handler.endElement("", "root", "root");
+ handler.endDocument();
+ }
+
+ @Override
+ public void setDTDHandler(final DTDHandler handler) {
+ wired.add(handler);
+ super.setDTDHandler(handler);
+ }
+
+ @Override
+ public void setEntityResolver(final EntityResolver resolver) {
+ wired.add(resolver);
+ super.setEntityResolver(resolver);
+ }
+
+ @Override
+ public void setErrorHandler(final ErrorHandler handler) {
+ wired.add(handler);
+ super.setErrorHandler(handler);
+ }
+
+ @Override
+ public void setFeature(final String name, final boolean value) {
+ }
+
+ @Override
+ public void setProperty(final String name, final Object value) {
+ }
+ }
+
/** Copies the input through unchanged, so external-entity content in the
input would surface in the filter's output. */
private static final String IDENTITY_XSLT = "<xsl:stylesheet
version=\"1.0\" xmlns:xsl=\"http://www.w3.org/1999/XSL/Transform\";>\n"
+ " <xsl:template match=\"/\"><xsl:copy-of
select=\".\"/></xsl:template>\n"
@@ -133,6 +193,20 @@ void secureFilterFromTemplatesDoesNotLeakDocument() throws
Exception {
assertFalse(filterAndCapture(filter,
"<root/>").contains(AttackTestSupport.LEAKED_MARKER), "document() through
XMLFilter(Templates) leaked");
}
+ @Test
+ void secureFilterParsesWithoutAnInputSource() throws Exception {
+ // A SAXSource carrying only the filter unmarshals as
parse((InputSource) null): the input comes from the parent, and implementations
that dereference
+ // the InputSource the filter passes on must still get one.
+ final XMLFilter filter =
SaxSurfaceTestSupport.secureFactory().newXMLFilter(AttackTestSupport.streamSource(IDENTITY_XSLT));
+ final SelfDrivenParent parent = new SelfDrivenParent();
+ filter.setParent(parent);
+ filter.setContentHandler(new DefaultHandler());
+ filter.parse((InputSource) null);
+ final InputSource substituted = parent.parsed.get(0);
+ assertNotNull(substituted, "the parent must be handed an InputSource,
not the caller's null");
+ assertEquals(SecureXMLFilter.NO_INPUT_SYSTEM_ID,
substituted.getSystemId(), "the substituted InputSource must carry the
placeholder system id");
+ }
+
@Test
void secureFilterRethrowsHandlerSAXException() throws Exception {
final XMLFilter filter =
SaxSurfaceTestSupport.secureFactory().newXMLFilter(AttackTestSupport.streamSource(IDENTITY_XSLT));
@@ -170,60 +244,11 @@ void secureFilterWiresCallbacksToParent() throws
Exception {
// parse must perform the XMLFilterImpl.setupParse wiring on the
parent for the resolver, DTD and error callbacks (the ContentHandler is owned
by the
// transformer). The wiring calls are asserted directly on a recording
parent: which of them the implementation later consults or overwrites varies.
final XMLFilter filter =
SaxSurfaceTestSupport.secureFactory().newXMLFilter(AttackTestSupport.streamSource(IDENTITY_XSLT));
- final List<Object> wired = new ArrayList<>();
- final XMLFilterImpl parent = new XMLFilterImpl() {
-
- @Override
- public boolean getFeature(final String name) {
- // Accept the namespace probes implementations make on a
SAXSource reader; there is no parent to delegate to.
- return "http://xml.org/sax/features/namespaces".equals(name);
- }
-
- @Override
- public Object getProperty(final String name) {
- return null;
- }
-
- @Override
- public void parse(final InputSource input) throws SAXException {
- // Minimal well-formed document for the transformation to
consume; no real parser behind this parent.
- final ContentHandler handler = getContentHandler();
- handler.startDocument();
- handler.startElement("", "root", "root", new AttributesImpl());
- handler.endElement("", "root", "root");
- handler.endDocument();
- }
-
- @Override
- public void setDTDHandler(final DTDHandler handler) {
- wired.add(handler);
- super.setDTDHandler(handler);
- }
-
- @Override
- public void setEntityResolver(final EntityResolver resolver) {
- wired.add(resolver);
- super.setEntityResolver(resolver);
- }
-
- @Override
- public void setErrorHandler(final ErrorHandler handler) {
- wired.add(handler);
- super.setErrorHandler(handler);
- }
-
- @Override
- public void setFeature(final String name, final boolean value) {
- }
-
- @Override
- public void setProperty(final String name, final Object value) {
- }
- };
+ final SelfDrivenParent parent = new SelfDrivenParent();
filter.setParent(parent);
assertEquals("", filterAndCapture(filter, "<ignored/>"));
- assertEquals(3, wired.stream().filter(callback -> callback ==
filter).count(),
- "parse should wire the filter as the parent's EntityResolver,
DTDHandler and ErrorHandler: " + wired);
+ assertEquals(3, parent.wired.stream().filter(callback -> callback ==
filter).count(),
+ "parse should wire the filter as the parent's EntityResolver,
DTDHandler and ErrorHandler: " + parent.wired);
}
@Test
