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 0d884ad Reject a foreign Templates in newTransformerHandler in the
TrAX shape (#94)
0d884ad is described below
commit 0d884ad8ca5edac14584baa13f93ea5180025bc3
Author: Piotr P. Karwasz <[email protected]>
AuthorDate: Mon Sep 14 00:01:26 2026 +0200
Reject a foreign Templates in newTransformerHandler in the TrAX shape (#94)
* Reject a foreign Templates in newTransformerHandler in the TrAX shape.
An implementation builds a TransformerHandler by casting the Templates, or
the Transformer it produces, to its own type, so one it did not compile
cannot be used at all. Apache Xalan and the stock JDK signal that with an
undeclared ClassCastException; report it as the
TransformerConfigurationException Saxon already raises for it.
Assisted-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01AaDyR9HjZLWkFn42x7kje3
* Improve assertion for foreign Templates handling
Co-authored-by: Copilot Autofix powered by AI
<[email protected]>
* Keep the failure message free of interpretation.
A ClassCastException out of the delegate need not mean the Templates was
refused for being foreign, so the message now states only what was attempted
and with what, and leaves the cause to say the rest.
The assertion follows: it no longer pins the whole message, and it asks for
the cause and the named Templates only where the ClassCastException actually
escaped. Saxon reports the refusal itself, in its own words and without that
cause.
Assisted-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01AaDyR9HjZLWkFn42x7kje3
* Reject a null Templates the same way on every implementation.
newTransformerHandler(Templates) raised NullPointerException everywhere but
on Saxon, which reported the null as a TransformerConfigurationException.
Rejecting the argument up front makes that check independent of the
implementation, as it already is on the other creation methods, and a test
now pins null handling across the whole creation surface.
The ClassCastException translation cannot meet a null: casting one never
throws, so the message never reads the class of a null Templates.
Assisted-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01AaDyR9HjZLWkFn42x7kje3
---------
Co-authored-by: Gary Gregory <[email protected]>
Co-authored-by: Copilot Autofix powered by AI
<[email protected]>
---
src/changes/changes.xml | 1 +
.../xml/secure/SecureTransformerFactory.java | 18 ++++++++--
.../xml/secure/SecureTransformerFactoryTest.java | 39 ++++++++++++++++++++++
3 files changed, 56 insertions(+), 2 deletions(-)
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 708c972..cbe6d71 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 rejection behavior of foreign Templates in
SAXTransformerFactory.newTransformerHandler.</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 -->
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 b55a68f..a609e1e 100644
--- a/src/main/java/org/apache/commons/xml/secure/SecureTransformerFactory.java
+++ b/src/main/java/org/apache/commons/xml/secure/SecureTransformerFactory.java
@@ -308,10 +308,24 @@ public TransformerHandler newTransformerHandler(final
Source source) throws Tran
return
secure(delegate.newTransformerHandler(SecureSAXParserFactory.secure(source,
overrideDefaultParser())));
}
+ /**
+ * {@inheritDoc}
+ *
+ * <p>Most implementations reject a {@link Templates} they did not
compile, some in the TrAX shape, others by casting it or its Transformer to
their
+ * own type. Both reach the caller as a {@link
TransformerConfigurationException}.</p>
+ */
@Override
public TransformerHandler newTransformerHandler(final Templates
templates) throws TransformerConfigurationException {
- // Implementations cast templates.newTransformer() to their own
Transformer type, so hand them the wrapped implementation Templates, not the
wrapper.
- return secure(delegate.newTransformerHandler(unwrap(templates)));
+ // Implementations:
+ // - cast templates.newTransformer() to their own Transformer
type, so hand them the wrapped implementation Templates, not the wrapper;
+ // - raise NullPointerException for a null argument, except Saxon;
rejecting it here keeps that uniform, as on the other methods.
+ final Templates unwrapped =
unwrap(Objects.requireNonNull(templates, "templates"));
+ try {
+ return secure(delegate.newTransformerHandler(unwrapped));
+ } catch (final ClassCastException e) {
+ throw new TransformerConfigurationException("Failed to create
a TransformerHandler from a Templates of type "
+ + unwrapped.getClass().getName(), e);
+ }
}
/**
diff --git
a/src/test/java/org/apache/commons/xml/secure/SecureTransformerFactoryTest.java
b/src/test/java/org/apache/commons/xml/secure/SecureTransformerFactoryTest.java
index 1c309b8..3c81f76 100644
---
a/src/test/java/org/apache/commons/xml/secure/SecureTransformerFactoryTest.java
+++
b/src/test/java/org/apache/commons/xml/secure/SecureTransformerFactoryTest.java
@@ -22,10 +22,12 @@
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 java.io.StringReader;
import java.util.HashMap;
import java.util.Map;
+import java.util.Properties;
import javax.xml.XMLConstants;
import javax.xml.parsers.DocumentBuilderFactory;
@@ -192,6 +194,43 @@ void rejectsDelegatesThatCannotEnableSecureProcessing() {
assertThrows(SecureException.class, () ->
SecureTransformerFactory.secure(new RejectingFeatureFactory()));
}
+ @Test
+ void rejectsNullInputOnEveryFactoryMethod() {
+ final SAXTransformerFactory factory = (SAXTransformerFactory)
SecureTransformerFactory.newInstance();
+ assertThrows(NullPointerException.class, () ->
factory.newTemplates(null));
+ assertThrows(NullPointerException.class, () ->
factory.newTransformer(null));
+ assertThrows(NullPointerException.class, () ->
factory.newTransformerHandler((Source) null));
+ assertThrows(NullPointerException.class, () ->
factory.newTransformerHandler((Templates) null));
+ assertThrows(NullPointerException.class, () ->
factory.newXMLFilter((Source) null));
+ assertThrows(NullPointerException.class, () ->
factory.newXMLFilter((Templates) null));
+ }
+
+ @Test
+ void rejectsForeignTemplatesFromNewTransformerHandler() throws Exception {
+ final SAXTransformerFactory factory = (SAXTransformerFactory)
SecureTransformerFactory.newInstance();
+ final Templates own = factory.newTemplates(stylesheet());
+ // A caller's own Templates wrapper, the shape a framework uses to
carry parameters onto the Transformer it hands out.
+ final Templates foreign = new Templates() {
+
+ @Override
+ public Properties getOutputProperties() {
+ return own.getOutputProperties();
+ }
+
+ @Override
+ public Transformer newTransformer() throws
TransformerConfigurationException {
+ return own.newTransformer();
+ }
+ };
+ // Xalan and XSLTC reject a Templates they did not compile with an
undeclared ClassCastException, where Saxon uses the TrAX shape.
+ final TransformerConfigurationException exception =
assertThrows(TransformerConfigurationException.class,
+ () -> factory.newTransformerHandler(foreign));
+ // Saxon reports it in its own words; where the ClassCastException
escaped instead, it must arrive as the cause, under a message naming the
Templates.
+ if (exception.getCause() instanceof ClassCastException) {
+
assertTrue(exception.getMessage().contains(foreign.getClass().getName()),
exception.getMessage());
+ }
+ }
+
@Test
void securesAssociatedStylesheetSourcesOfEverySupportedShape() throws
Exception {
final SAXTransformerFactory factory = (SAXTransformerFactory)
SecureTransformerFactory.newInstance();
