jamesnetherton commented on code in PR #9116:
URL: https://github.com/apache/camel-quarkus/pull/9116#discussion_r3947179234
##########
extensions-support/xalan/runtime/src/main/java/org/apache/camel/quarkus/support/xalan/XalanTransformerFactory.java:
##########
@@ -138,12 +198,224 @@ public TemplatesHandler newTemplatesHandler() throws
TransformerConfigurationExc
@Override
public XMLFilter newXMLFilter(Source source) throws
TransformerConfigurationException {
- return delegate.newXMLFilter(source);
+ return secure(delegate.newXMLFilter(source));
}
@Override
public XMLFilter newXMLFilter(Templates templates) throws
TransformerConfigurationException {
- return delegate.newXMLFilter(templates);
+ return secure(delegate.newXMLFilter(unwrap(templates)));
+ }
+
+ /**
+ * The SAX push entry points hand the caller a {@link Transformer} to
configure rather than one to call,
+ * and Xalan only copies the factory's {@link URIResolver} onto some of
them, so {@code document()} is
+ * restricted here for the same reason it is in {@link
#secure(Transformer)}. The document being
+ * transformed is parsed by the {@link XMLReader} the caller drives the
handler with, which is the
+ * caller's own choice just as a {@link SAXSource} carrying a reader is.
+ */
+ private TransformerHandler secure(TransformerHandler handler) {
+ handler.getTransformer().setURIResolver(restrictingUriResolver);
+ return handler;
+ }
+
+ /**
+ * {@link XMLFilter} has no accessor for the {@link Transformer} behind
it, so the restriction can only be
+ * installed on Xalan's own implementation. Guarded rather than cast
blindly so that a Xalan upgrade
+ * returning something else is reported instead of silently dropping the
restriction.
+ */
+ private XMLFilter secure(XMLFilter filter) {
+ if (filter instanceof TrAXFilter) {
+ ((TrAXFilter)
filter).getTransformer().setURIResolver(restrictingUriResolver);
+ } else {
+ LOGGER.warn("Expected an {} from the Xalan TransformerFactory but
got {}. The document() function"
+ + " may resolve external resources when transforming
through this XMLFilter.",
+ TrAXFilter.class.getName(), filter == null ? null :
filter.getClass().getName());
+ }
+ return filter;
+ }
+
+ /**
+ * Xalan casts {@link Templates} to its own {@code TemplatesImpl}
internally, so the wrapper has to be
+ * peeled off before handing one back to the delegate.
+ */
+ private static Templates unwrap(Templates templates) {
+ return templates instanceof SecuredTemplates ? ((SecuredTemplates)
templates).delegate : templates;
+ }
+
+ /**
+ * Parses {@code source} with a hardened {@link XMLReader} unless it has
already been parsed, or the
+ * caller supplied its own reader. Mirrors what {@code
XmlConverter.createSAXParserFactory()} does for
+ * the bodies camel-xslt converts itself, so that {@link Source}-shaped
bodies get the same treatment.
+ */
+ private static Source secureInputSource(Source source) throws
TransformerException {
+ if (source instanceof StreamSource) {
+ final StreamSource streamSource = (StreamSource) source;
+ final InputStream inputStream = streamSource.getInputStream();
+ final Reader reader = streamSource.getReader();
+ if (inputStream == null && reader == null) {
+ // Nothing but a systemId; let the delegate resolve it as
before
+ return source;
Review Comment:
Good catch, confirmed and fixed. I reproduced it with a system-ID-only
`StreamSource` over a document carrying a `SYSTEM` entity: the entity was
resolved and the fetch went out, so this was a real hole rather than a
theoretical one.
Fixed as you suggested. The branch now builds an `InputSource` with the
system ID and returns `new SAXSource(createSecureXmlReader(), inputSource)`, so
the document is parsed by the secured reader instead of being opened by Xalan.
I also moved `setSystemId` out of the stream/reader branches so it is carried
in every case, which keeps relative references resolving against the document.
The early return survives only when there is no stream, no reader and no system
id, where there is nothing to parse and the delegate reports it as before.
Two regression tests cover it:
`externalEntityInSystemIdOnlyStreamSourceIsNotResolved` for the entity, and
`systemIdOnlyStreamSourceIsStillTransformed` to pin that loading by system id
still works. Both fail on the previous code and pass now.
--
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]
