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 81138e6 Keep a URIResolver a wrapped Transformer already carries (#96)
81138e6 is described below
commit 81138e66cda571f44d77d277e9ea0ffa53470476
Author: Piotr P. Karwasz <[email protected]>
AuthorDate: Mon Sep 14 01:09:20 2026 +0200
Keep a URIResolver a wrapped Transformer already carries (#96)
* Keep a URIResolver a wrapped Transformer already carries.
The floor was installed with delegate.setURIResolver(floor) and seeded from
the factory's resolver alone, so a resolver the delegate already carried was
dropped unconsulted: document() then resolved to empty content instead of
going through it. That contradicts the documented contract, where a caller's
resolver is consulted before the floor and never replaced by it.
The floor is now seeded with the resolver the delegate carries, falling back
to the factory's where it carries none. A floor already in place came from
this library, so it is skipped rather than nested, and no implementation
seeds a fresh Transformer with a resolver that fetches: Xalan and Saxon
leave it null, XSLTC copies only what its factory carried, and Saxon's own
default resolution lives on the Configuration.
The resolver threaded down from the factory is named factoryUriResolver
throughout, since it is now the fallback rather than the only seed.
Assisted-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01AaDyR9HjZLWkFn42x7kje3
* Remove unused import for TemplatesHandler
Co-authored-by: Copilot Autofix powered by AI
<[email protected]>
* Pin what a reset restores.
The floor is re-seeded on reset from the resolver the delegate carried, and
the existing reset test only asked whether the floor still blocks, which a
reset to the factory resolver would also satisfy. The new test opts a URI in
through the carried resolver and expects it answered after the reset.
Drops an unused import a test picked up along the way. Checkstyle does not
see it: the pom leaves includeTestSourceDirectory commented out, so the
rules run over main sources alone.
Assisted-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01B5Zpo8ypraR2meSvGudHQm
---------
Co-authored-by: Copilot Autofix powered by AI
<[email protected]>
Co-authored-by: Gary Gregory <[email protected]>
---
src/changes/changes.xml | 1 +
.../apache/commons/xml/secure/SecureTemplates.java | 10 ++---
.../commons/xml/secure/SecureTemplatesHandler.java | 10 ++---
.../commons/xml/secure/SecureTransformer.java | 25 ++++++-----
.../xml/secure/SecureTransformerHandler.java | 6 +--
.../xml/secure/SecureTransformerHandlerTest.java | 14 +++++++
.../commons/xml/secure/SecureTransformerTest.java | 45 ++++++++++++++++++++
.../commons/xml/secure/TransformerHandlerTest.java | 49 ++++++++++++++++++++++
.../apache/commons/xml/secure/XMLFilterTest.java | 27 ++++++++++++
9 files changed, 164 insertions(+), 23 deletions(-)
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index 837d9cd..f0b8ab4 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -37,6 +37,7 @@ The <action> type attribute can be add, update, fix, or
remove.
<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>
+ <action type="fix" dev="pkarwasz" due-to="Piotr P. Karwasz, Gary
Gregory" issue="COMMONSXML-17">Keep a URIResolver already set on a Transformer
when it is wrapped, instead of replacing it with the resolver floor.</action>
<action type="fix" dev="pkarwasz" due-to="Piotr P. Karwasz, Gary
Gregory">Create the Transformer of an XMLFilter eagerly and reuse it for every
parse.</action>
<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>
<!-- ADD -->
diff --git a/src/main/java/org/apache/commons/xml/secure/SecureTemplates.java
b/src/main/java/org/apache/commons/xml/secure/SecureTemplates.java
index c276c0c..7025906 100644
--- a/src/main/java/org/apache/commons/xml/secure/SecureTemplates.java
+++ b/src/main/java/org/apache/commons/xml/secure/SecureTemplates.java
@@ -44,7 +44,7 @@ final class SecureTemplates implements Templates {
/**
* Compile-time URIResolver snapshot; the underlying implementation does
not propagate the factory's resolver onto Transformers obtained from Templates.
*/
- private final URIResolver uriResolver;
+ private final URIResolver factoryUriResolver;
/**
* Empty-{@link Source} supplier for the produced Transformer's floor;
{@code null} means the default empty DOM.
@@ -61,14 +61,14 @@ final class SecureTemplates implements Templates {
* Constructs a new instance.
*
* @param delegate The delegate to wrap; must not be {@code null}.
- * @param uriResolver The compile-time URIResolver snapshot to
restore onto Transformers produced from the compiled Templates; may be {@code
null}.
+ * @param factoryUriResolver The factory's compile-time URIResolver
snapshot to restore onto Transformers produced from the compiled Templates; may
be {@code null}.
* @param emptySource The empty-{@link Source} supplier for the
produced Transformers.
* @param overrideDefaultParser whether the produced Transformers' source
rewrites should use the pluggable parser lookup instead of the platform's
built-in parser.
* @throws NullPointerException Thrown if {@code delegate} is {@code null}.
*/
- SecureTemplates(final Templates delegate, final URIResolver uriResolver,
final Supplier<Source> emptySource, final boolean overrideDefaultParser) {
+ SecureTemplates(final Templates delegate, final URIResolver
factoryUriResolver, final Supplier<Source> emptySource, final boolean
overrideDefaultParser) {
this.delegate = Objects.requireNonNull(delegate, "delegate");
- this.uriResolver = uriResolver;
+ this.factoryUriResolver = factoryUriResolver;
this.emptySource = emptySource;
this.overrideDefaultParser = overrideDefaultParser;
}
@@ -92,6 +92,6 @@ public Transformer newTransformer() throws
TransformerConfigurationException {
final Transformer transformer = delegate.newTransformer();
// Some implementations return null rather than throw, so preserve the
delegate's behavior instead of enforcing the contract.
// For example, https://issues.apache.org/jira/browse/XALANJ-2410
- return transformer != null ? new SecureTransformer(transformer,
uriResolver, emptySource, overrideDefaultParser) : null;
+ return transformer != null ? new SecureTransformer(transformer,
factoryUriResolver, emptySource, overrideDefaultParser) : null;
}
}
diff --git
a/src/main/java/org/apache/commons/xml/secure/SecureTemplatesHandler.java
b/src/main/java/org/apache/commons/xml/secure/SecureTemplatesHandler.java
index 3b686d6..ec45cb8 100644
--- a/src/main/java/org/apache/commons/xml/secure/SecureTemplatesHandler.java
+++ b/src/main/java/org/apache/commons/xml/secure/SecureTemplatesHandler.java
@@ -44,7 +44,7 @@ final class SecureTemplatesHandler implements
TemplatesHandler {
/**
* Compile-time URIResolver snapshot, restored onto Transformers produced
from the compiled Templates.
*/
- private final URIResolver uriResolver;
+ private final URIResolver factoryUriResolver;
/**
* Empty-{@link Source} supplier for the produced Templates' floor; {@code
null} means the default empty DOM.
@@ -60,15 +60,15 @@ final class SecureTemplatesHandler implements
TemplatesHandler {
* Constructs a new instance.
*
* @param delegate The delegate to wrap; must not be {@code null}.
- * @param uriResolver The compile-time URIResolver snapshot to restore
onto Transformers produced from the compiled Templates; may be {@code null}.
+ * @param factoryUriResolver The factory's compile-time URIResolver
snapshot to restore onto Transformers produced from the compiled Templates; may
be {@code null}.
* @param emptySource The empty-{@link Source} supplier for the produced
Templates; may be {@code null} for the default empty DOM document.
* @param overrideDefaultParser whether the produced Templates' source
rewrites should use the pluggable parser lookup instead of the platform's
built-in parser.
* @throws NullPointerException Thrown if {@code delegate} is {@code null}.
*/
- SecureTemplatesHandler(final TemplatesHandler delegate, final URIResolver
uriResolver, final Supplier<Source> emptySource,
+ SecureTemplatesHandler(final TemplatesHandler delegate, final URIResolver
factoryUriResolver, final Supplier<Source> emptySource,
final boolean overrideDefaultParser) {
this.delegate = Objects.requireNonNull(delegate, "delegate");
- this.uriResolver = uriResolver;
+ this.factoryUriResolver = factoryUriResolver;
this.emptySource = emptySource;
this.overrideDefaultParser = overrideDefaultParser;
}
@@ -102,7 +102,7 @@ public String getSystemId() {
public Templates getTemplates() {
// Null before the stylesheet's endDocument (and on a failed compile
in some implementations).
final Templates templates = delegate.getTemplates();
- return templates == null ? null : new SecureTemplates(templates,
uriResolver, emptySource, overrideDefaultParser);
+ return templates == null ? null : new SecureTemplates(templates,
factoryUriResolver, emptySource, overrideDefaultParser);
}
@Override
diff --git a/src/main/java/org/apache/commons/xml/secure/SecureTransformer.java
b/src/main/java/org/apache/commons/xml/secure/SecureTransformer.java
index d84127c..37a06ca 100644
--- a/src/main/java/org/apache/commons/xml/secure/SecureTransformer.java
+++ b/src/main/java/org/apache/commons/xml/secure/SecureTransformer.java
@@ -34,9 +34,9 @@
* {@link SecureSAXParserFactory#secure(Source, boolean)} before delegating,
and keeps an ignore-all {@link URIResolver} floor so runtime {@code document()}
calls a
* caller does not resolve return empty rather than being fetched.
* <p>
- * The floor is installed on the delegate transformer at construction, seeded
with the factory's compile-time resolver; {@link #setURIResolver(URIResolver)}
- * routes a caller's resolver through it rather than replacing it, so the
block cannot be dropped. {@link #reset()} re-establishes the floor, seeded
again with
- * the factory's compile-time resolver, matching the just-constructed state.
+ * The floor is installed on the delegate transformer at construction, seeded
with the resolver the delegate already carried, or with the factory's
+ * compile-time resolver where it carried none; {@link
#setURIResolver(URIResolver)} routes a caller's resolver through it rather than
replacing it, so the
+ * block cannot be dropped. {@link #reset()} re-establishes the floor with
that same seed, matching the just-constructed state.
* </p>
*/
final class SecureTransformer extends Transformer {
@@ -44,9 +44,10 @@ final class SecureTransformer extends Transformer {
private final Transformer delegate;
/**
- * Compile-time URIResolver snapshot the floor is seeded with, both at
construction and again on {@link #reset()}.
+ * URIResolver the floor is seeded with, both at construction and again on
{@link #reset()}: the one the delegate carried, else the factory's compile-time
+ * snapshot.
*/
- private final URIResolver uriResolver;
+ private final URIResolver initialUriResolver;
private final FallbackIgnoreURIResolver floor;
@@ -60,16 +61,20 @@ final class SecureTransformer extends Transformer {
* Constructs a new instance.
*
* @param delegate The delegate to wrap; must not be {@code null}.
- * @param uriResolver The compile-time URIResolver snapshot to seed
the floor with; may be {@code null}.
+ * @param factoryUriResolver The factory's compile-time URIResolver
snapshot, used where the delegate carries none of its own; may be {@code null}.
* @param emptySource The empty-{@link Source} supplier for the
produced Transformers; {@code null} for the default empty DOM document.
* @param overrideDefaultParser whether the source rewrites should use the
pluggable parser lookup instead of the platform's built-in parser.
* @throws NullPointerException Thrown if {@code delegate} is {@code null}.
*/
- SecureTransformer(final Transformer delegate, final URIResolver
uriResolver, final Supplier<Source> emptySource, final boolean
overrideDefaultParser) {
+ SecureTransformer(final Transformer delegate, final URIResolver
factoryUriResolver, final Supplier<Source> emptySource,
+ final boolean overrideDefaultParser) {
this.delegate = Objects.requireNonNull(delegate, "delegate");
- this.uriResolver = uriResolver;
this.overrideDefaultParser = overrideDefaultParser;
- this.floor = new FallbackIgnoreURIResolver(uriResolver, emptySource,
() -> overrideDefaultParser);
+ // A caller may have configured the delegate before it reached us;
chain the floor onto that resolver rather than dropping it. A floor already
there
+ // came from this library, so it is the factory's resolver that seeds
the new one.
+ final URIResolver carried = delegate.getURIResolver();
+ this.initialUriResolver = carried == null || carried instanceof
FallbackIgnoreURIResolver ? factoryUriResolver : carried;
+ this.floor = new FallbackIgnoreURIResolver(initialUriResolver,
emptySource, () -> overrideDefaultParser);
delegate.setURIResolver(floor);
}
@@ -106,7 +111,7 @@ public URIResolver getURIResolver() {
@Override
public void reset() {
delegate.reset();
- floor.setDelegate(uriResolver);
+ floor.setDelegate(initialUriResolver);
delegate.setURIResolver(floor);
}
diff --git
a/src/main/java/org/apache/commons/xml/secure/SecureTransformerHandler.java
b/src/main/java/org/apache/commons/xml/secure/SecureTransformerHandler.java
index f90cb53..7daaefc 100644
--- a/src/main/java/org/apache/commons/xml/secure/SecureTransformerHandler.java
+++ b/src/main/java/org/apache/commons/xml/secure/SecureTransformerHandler.java
@@ -52,15 +52,15 @@ final class SecureTransformerHandler implements
TransformerHandler {
* Constructs a new instance.
*
* @param delegate The delegate to wrap; must not be {@code null}.
- * @param uriResolver The compile-time URIResolver snapshot to restore
onto the live transformer; may be {@code null}.
+ * @param factoryUriResolver The factory's compile-time URIResolver
snapshot to restore onto the live transformer; may be {@code null}.
* @param emptySource The empty-{@link Source} supplier for the produced
Transformer's floor; {@code null} means the default empty DOM.
* @param overrideDefaultParser whether the live transformer's source
rewrites should use the pluggable parser lookup instead of the platform's
built-in parser.
* @throws NullPointerException Thrown if {@code delegate} is {@code null}.
*/
- SecureTransformerHandler(final TransformerHandler delegate, final
URIResolver uriResolver, final Supplier<Source> emptySource,
+ SecureTransformerHandler(final TransformerHandler delegate, final
URIResolver factoryUriResolver, final Supplier<Source> emptySource,
final boolean overrideDefaultParser) {
this.delegate = Objects.requireNonNull(delegate, "delegate");
- this.transformer = new SecureTransformer(delegate.getTransformer(),
uriResolver, emptySource, overrideDefaultParser);
+ this.transformer = new SecureTransformer(delegate.getTransformer(),
factoryUriResolver, emptySource, overrideDefaultParser);
}
@Override
diff --git
a/src/test/java/org/apache/commons/xml/secure/SecureTransformerHandlerTest.java
b/src/test/java/org/apache/commons/xml/secure/SecureTransformerHandlerTest.java
index d65b86d..f0300d8 100644
---
a/src/test/java/org/apache/commons/xml/secure/SecureTransformerHandlerTest.java
+++
b/src/test/java/org/apache/commons/xml/secure/SecureTransformerHandlerTest.java
@@ -19,11 +19,14 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertSame;
import java.io.StringWriter;
import javax.xml.transform.TransformerFactory;
+import javax.xml.transform.URIResolver;
import javax.xml.transform.sax.SAXTransformerFactory;
+import javax.xml.transform.sax.TransformerHandler;
import javax.xml.transform.stream.StreamResult;
import org.junit.jupiter.api.Tag;
@@ -34,6 +37,17 @@
@Tag("trax")
class SecureTransformerHandlerTest {
+ @Test
+ void adoptsAResolverTheHandlersTransformerCarries() throws Exception {
+ final SAXTransformerFactory factory = (SAXTransformerFactory)
TransformerFactory.newInstance();
+ final TransformerHandler delegate = factory.newTransformerHandler();
+ // An implementation may seed the handler's transformer from the
Templates it was built with; that resolver must survive the wrapping.
+ final URIResolver carried = (href, base) -> null;
+ delegate.getTransformer().setURIResolver(carried);
+ final SecureTransformerHandler handler = new
SecureTransformerHandler(delegate, null, null, false);
+ assertSame(carried, handler.getTransformer().getURIResolver(), "the
resolver the handler's transformer carried must survive the wrapping");
+ }
+
@Test
void forwardsEveryTransformerHandlerMethod() throws Exception {
final SAXTransformerFactory factory = (SAXTransformerFactory)
TransformerFactory.newInstance();
diff --git
a/src/test/java/org/apache/commons/xml/secure/SecureTransformerTest.java
b/src/test/java/org/apache/commons/xml/secure/SecureTransformerTest.java
index dbf6009..e50996b 100644
--- a/src/test/java/org/apache/commons/xml/secure/SecureTransformerTest.java
+++ b/src/test/java/org/apache/commons/xml/secure/SecureTransformerTest.java
@@ -17,7 +17,10 @@
package org.apache.commons.xml.secure;
+import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import java.io.StringReader;
import java.io.StringWriter;
@@ -26,8 +29,10 @@
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.transform.ErrorListener;
import javax.xml.transform.OutputKeys;
+import javax.xml.transform.Transformer;
import javax.xml.transform.TransformerException;
import javax.xml.transform.TransformerFactory;
+import javax.xml.transform.URIResolver;
import javax.xml.transform.dom.DOMSource;
import javax.xml.transform.stream.StreamResult;
import javax.xml.transform.stream.StreamSource;
@@ -38,6 +43,46 @@
@Tag("trax")
class SecureTransformerTest {
+ /** A transformer a caller configured before this library saw it, the
shape a caller's own Templates hands out. */
+ private static SecureTransformer wrap(final URIResolver carried) throws
Exception {
+ final Transformer delegate =
TransformerFactory.newInstance().newTransformer(AttackTestSupport.resourceSource("with-document.xsl"));
+ delegate.setURIResolver(carried);
+ return new SecureTransformer(delegate, null, null, false);
+ }
+
+ @Test
+ void adoptsAResolverTheDelegateAlreadyCarries() throws Exception {
+ final URIResolver carried = (href, base) -> new StreamSource(new
StringReader("<opted-in/>"));
+ final SecureTransformer transformer = wrap(carried);
+ assertSame(carried, transformer.getURIResolver(), "the resolver the
delegate carried must survive the wrapping");
+ final StringWriter output = new StringWriter();
+ transformer.transform(AttackTestSupport.streamSource("<root/>"), new
StreamResult(output));
+ assertTrue(output.toString().contains("opted-in"), "the carried
resolver must answer document()");
+
assertFalse(output.toString().contains(AttackTestSupport.LEAKED_MARKER), "the
real resource must not be fetched");
+ }
+
+ @Test
+ void keepsTheFloorUnderACarriedResolverThatDeclines() throws Exception {
+ // Adopting the caller's resolver must not make the floor reachable
around: what the resolver declines stays unfetched.
+ final SecureTransformer transformer = wrap((href, base) -> null);
+ final StringWriter output = new StringWriter();
+ transformer.transform(AttackTestSupport.streamSource("<root/>"), new
StreamResult(output));
+
assertFalse(output.toString().contains(AttackTestSupport.LEAKED_MARKER),
"document() the resolver declined must not be fetched");
+ }
+
+ @Test
+ void carriesTheAdoptedResolverThroughReset() throws Exception {
+ final URIResolver carried = (href, base) -> new StreamSource(new
StringReader("<opted-in/>"));
+ final SecureTransformer transformer = wrap(carried);
+ // reset() re-seeds the floor, and the seed is the resolver the
delegate carried, not the factory's.
+ transformer.reset();
+ assertSame(carried, transformer.getURIResolver(), "reset must restore
the resolver the delegate carried");
+ final StringWriter output = new StringWriter();
+ transformer.transform(AttackTestSupport.streamSource("<root/>"), new
StreamResult(output));
+ assertTrue(output.toString().contains("opted-in"), "the carried
resolver must still answer document() after a reset");
+
assertFalse(output.toString().contains(AttackTestSupport.LEAKED_MARKER), "the
real resource must not be fetched");
+ }
+
@Test
void forwardsEveryTransformerMethod() throws Exception {
final TransformerFactory factory = TransformerFactory.newInstance();
diff --git
a/src/test/java/org/apache/commons/xml/secure/TransformerHandlerTest.java
b/src/test/java/org/apache/commons/xml/secure/TransformerHandlerTest.java
index 5ac1a9a..06d35be 100644
--- a/src/test/java/org/apache/commons/xml/secure/TransformerHandlerTest.java
+++ b/src/test/java/org/apache/commons/xml/secure/TransformerHandlerTest.java
@@ -19,16 +19,22 @@
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertTrue;
import java.io.StringWriter;
+import java.util.Properties;
import javax.xml.transform.Templates;
+import javax.xml.transform.Transformer;
+import javax.xml.transform.TransformerConfigurationException;
import javax.xml.transform.TransformerFactory;
+import javax.xml.transform.URIResolver;
import javax.xml.transform.sax.SAXTransformerFactory;
import javax.xml.transform.sax.TransformerHandler;
import javax.xml.transform.stream.StreamResult;
+import org.junit.jupiter.api.Assumptions;
import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
@@ -65,6 +71,49 @@ void secureTransformerHandlerDoesNotLeakDocument() throws
Exception {
assertFalse(transformViaHandler(handler).contains(AttackTestSupport.LEAKED_MARKER),
"document() through TransformerHandler leaked");
}
+ /** A caller's own Templates that only configures the Transformer it hands
out, the shape Apache CXF's XSLTJaxbProvider builds. */
+ private static Templates callersTemplates(final Templates compiled, final
URIResolver carried) {
+ return new Templates() {
+
+ @Override
+ public Properties getOutputProperties() {
+ return compiled.getOutputProperties();
+ }
+
+ @Override
+ public Transformer newTransformer() throws
TransformerConfigurationException {
+ final Transformer transformer = compiled.newTransformer();
+ transformer.setURIResolver(carried);
+ return transformer;
+ }
+ };
+ }
+
+ /** Skips the test where the implementation refuses a Templates it did not
compile itself, as Saxon does. */
+ private static TransformerHandler assumeAcceptsForeignImplementation(final
Templates callers) {
+ try {
+ return ((SAXTransformerFactory)
TransformerFactory.newInstance()).newTransformerHandler(callers);
+ } catch (final TransformerConfigurationException e) {
+ Assumptions.abort("the implementation does not accept a foreign
Templates: " + e.getMessage());
+ return null;
+ }
+ }
+
+ @Test
+ void secureHandlerKeepsAResolverTheCallersTemplatesSet() throws Exception {
+ final Templates compiled =
TransformerFactory.newInstance().newTemplates(AttackTestSupport.resourceSource("with-document.xsl"));
+ final URIResolver carried = (href, base) -> null;
+ final Templates callers = callersTemplates(compiled, carried);
+ // What the implementation itself ends up with: XSLTC leaves the
caller's resolver in place, Apache Xalan overwrites it with its factory's own.
+ final TransformerHandler nativeTransformerHandler =
assumeAcceptsForeignImplementation(callers);
+
Assumptions.assumeTrue(nativeTransformerHandler.getTransformer().getURIResolver()
== carried,
+ "the implementation does not keep a resolver set by the
caller's Templates");
+
+ final TransformerHandler handler =
SaxSurfaceTestSupport.secureFactory().newTransformerHandler(callers);
+ assertSame(carried, handler.getTransformer().getURIResolver(), "the
securing must not lose a resolver the implementation kept");
+
assertFalse(transformViaHandler(handler).contains(AttackTestSupport.LEAKED_MARKER),
"what that resolver declined must not be fetched");
+ }
+
@Test
void secureTransformerHandlerFromTemplatesDoesNotLeakDocument() throws
Exception {
final SAXTransformerFactory factory =
SaxSurfaceTestSupport.secureFactory();
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 e1cedd3..9af57dd 100644
--- a/src/test/java/org/apache/commons/xml/secure/XMLFilterTest.java
+++ b/src/test/java/org/apache/commons/xml/secure/XMLFilterTest.java
@@ -26,12 +26,16 @@
import java.io.StringReader;
import java.util.ArrayList;
import java.util.List;
+import java.util.Properties;
import javax.xml.XMLConstants;
import javax.xml.transform.Templates;
+import javax.xml.transform.Transformer;
+import javax.xml.transform.TransformerConfigurationException;
import javax.xml.transform.TransformerException;
import javax.xml.transform.TransformerFactory;
import javax.xml.transform.sax.SAXTransformerFactory;
+import javax.xml.transform.stream.StreamSource;
import org.junit.jupiter.api.Assumptions;
import org.junit.jupiter.api.Tag;
@@ -203,6 +207,29 @@ void secureFilterFromTemplatesDoesNotLeakDocument() throws
Exception {
assertFalse(filterAndCapture(filter,
"<root/>").contains(AttackTestSupport.LEAKED_MARKER), "document() through
XMLFilter(Templates) leaked");
}
+ @Test
+ void secureFilterKeepsAResolverTheCallersTemplatesSet() throws Exception {
+ final Templates compiled =
TransformerFactory.newInstance().newTemplates(AttackTestSupport.resourceSource("with-document.xsl"));
+ // A caller's own Templates that configures the Transformer it hands
out, the shape Apache CXF's XSLTJaxbProvider builds.
+ final Templates callers = new Templates() {
+
+ @Override
+ public Properties getOutputProperties() {
+ return compiled.getOutputProperties();
+ }
+
+ @Override
+ public Transformer newTransformer() throws
TransformerConfigurationException {
+ final Transformer transformer = compiled.newTransformer();
+ transformer.setURIResolver((href, base) -> new
StreamSource(new StringReader("<opted-in>resolver-applied</opted-in>")));
+ return transformer;
+ }
+ };
+ final String output =
filterAndCapture(SaxSurfaceTestSupport.secureFactory().newXMLFilter(callers),
"<root/>");
+ assertTrue(output.contains("resolver-applied"), "the resolver the
caller's Templates set must answer document()");
+ assertFalse(output.contains(AttackTestSupport.LEAKED_MARKER), "the
real resource must not be fetched");
+ }
+
@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
