ppkarwasz commented on code in PR #95:
URL: https://github.com/apache/commons-secure-xml/pull/95#discussion_r4001100599


##########
src/main/java/org/apache/commons/xml/secure/SecureXMLFilter.java:
##########
@@ -156,6 +163,51 @@ public void parse(final InputSource input) throws 
SAXException, IOException {
         }
     }
 
+    /**
+     * {@inheritDoc}
+     *
+     * <p>Builds the destination the transformation writes to, so a parse only 
has to run it. A handler that is also a {@link LexicalHandler} receives the
+     * result's comments and CDATA boundaries too, the way {@link 
javax.xml.transform.sax.SAXResult} expects them to be supplied.</p>
+     */
+    @Override
+    public void setContentHandler(final ContentHandler handler) {
+        super.setContentHandler(handler);
+        result = handler == null ? null : new SAXResult(handler);
+        if (handler instanceof LexicalHandler) {
+            result.setLexicalHandler((LexicalHandler) handler);
+        }
+    }
+
+    /**
+     * {@inheritDoc}
+     *
+     * <p>Wires the filter onto the new parent the way {@link 
XMLFilterImpl#setupParse()} would, minus the ContentHandler: the transformer 
owns the parent's
+     * content events and delivers the transformed stream to the caller's 
handler through a {@link SAXResult} instead. Wiring the parent here rather than 
per
+     * parse is enough because it is the filter that is installed, not the 
caller's callbacks, so a callback the caller sets afterwards is still 
reached.</p>
+     */
+    @Override
+    public void setParent(final XMLReader parent) {
+        super.setParent(parent);
+        // XMLFilterImpl tolerates a null parent, so do not wire one.
+        if (parent != null) {
+            parent.setEntityResolver(this);
+            parent.setDTDHandler(this);
+            parent.setErrorHandler(this);
+        }
+    }
+
+    /**
+     * Fails: events pushed into the {@link ContentHandler} role inherited 
from {@link XMLFilterImpl} would reach the caller's handler untransformed.
+     *
+     * <p>The stock filters make that role inert too, by dropping the events 
(Apache Xalan, the JDK) or by not implementing it at all (Saxon).</p>
+     *
+     * @throws SAXException Always.
+     */
+    @Override
+    public void startDocument() throws SAXException {
+        throw new SAXException("This XMLFilter only implements ContentHandler 
for technical reasons. To push SAX events, use newTransformerHandler instead.");

Review Comment:
   Only the **first** SAX event of an XML document is covered, to provide a 
better feedback to users that use the filter as `ContentHandler`.
   
   This method might be too protective and we could remove it. To trigger this 
a user would need to call:
   
   ```java
   XMLFilter filter = factory.newXMLFilter(templates);
   if (filter instanceof ContentHandler handler) {
     ... do something with handler ...
   }
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to