oscerd commented on code in PR #28160:
URL: https://github.com/apache/camel/pull/28160#discussion_r4376009041
##########
components/camel-xslt-saxon/src/main/java/org/apache/camel/component/xslt/saxon/XsltSaxonHelper.java:
##########
@@ -55,6 +68,64 @@ public static void
configureSecureProcessing(TransformerFactoryImpl factory, boo
factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING,
secureProcessing);
}
+ /**
+ * Restricts the external resource access of the given factory to the
empty set, so that stylesheet compilation and
+ * the {@code document()} function do not reach external resources. This
is the same configuration the
+ * {@code xslt-saxon} endpoint applies to the factory it creates; it is
shared so the endpoint and
+ * {@link XsltSaxonAggregationStrategy} cannot drift apart.
+ */
+ public static void configureExternalAccess(TransformerFactory factory) {
+ try {
+ factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_DTD, "");
+ } catch (IllegalArgumentException e) {
+ LOG.debug("TransformerFactory does not support {}",
XMLConstants.ACCESS_EXTERNAL_DTD);
+ }
+ try {
+ factory.setAttribute(XMLConstants.ACCESS_EXTERNAL_STYLESHEET, "");
+ } catch (IllegalArgumentException e) {
+ LOG.debug("TransformerFactory does not support {}",
XMLConstants.ACCESS_EXTERNAL_STYLESHEET);
+ }
+ }
+
+ /**
+ * Returns a source whose parsing does not resolve references to external
resources, for an input that is still to
+ * be parsed. The factory's {@code ACCESS_EXTERNAL_*} attributes do not
cover this parse, because the transformer
+ * factory has a {@link javax.xml.transform.URIResolver} installed that
resolves the reference itself, so the input
+ * is parsed with a dedicated reader instead.
+ * <p>
+ * A {@link SAXSource} that already carries its own {@link XMLReader}, and
any source that is not stream-based (for
+ * example a {@code DOMSource} or a {@code StAXSource}), is returned
unchanged; its content has already been parsed
+ * or is read through a reader the caller controls. Only a stream-based
source still to be parsed
+ * ({@code StreamSource}, including Camel's {@code StringSource}/{@code
BytesSource}, or a reader-less
+ * {@code SAXSource}) is wrapped with the restricted reader.
+ */
+ public static Source secureSource(Source source) {
+ if (source == null) {
+ return null;
+ }
+ if (source instanceof SAXSource saxSource && saxSource.getXMLReader()
!= null) {
+ return source;
+ }
+ InputSource inputSource = SAXSource.sourceToInputSource(source);
+ if (inputSource == null) {
+ return source;
+ }
+ try {
+ SAXParserFactory factory = SAXParserFactory.newInstance();
+ factory.setNamespaceAware(true);
+ factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+ factory.setFeature(EXTERNAL_GENERAL_ENTITIES, false);
+ factory.setFeature(EXTERNAL_PARAMETER_ENTITIES, false);
+ factory.setFeature(LOAD_EXTERNAL_DTD, false);
+ XMLReader xmlReader = factory.newSAXParser().getXMLReader();
+ return new SAXSource(xmlReader, inputSource);
+ } catch (ParserConfigurationException | SAXException e) {
+ LOG.warn("Cannot create a restricted XMLReader for the input
source, transforming it as is: {}",
Review Comment:
Fixed in 0b732ee. `secureSource` now fails closed: if a restricted
`XMLReader` cannot be built it throws a `RuntimeCamelException` instead of
logging and parsing the body unrestricted. The `SAXParserFactory` is configured
once in a static field and handed out through the public `XMLReaderPool` (the
same one `XmlConverter.toSAXSourceFromStream` uses), so a classpath parser that
rejects one of the features (for example the Xerces-specific
`load-external-dtd`) fails on first use rather than per exchange, and there is
no per-exchange `FactoryFinder` lookup.
_Claude Code on behalf of oscerd_
##########
components/camel-xslt-saxon/src/main/java/org/apache/camel/component/xslt/saxon/XsltSaxonEndpoint.java:
##########
@@ -258,6 +249,7 @@ protected XsltSaxonBuilder createXsltBuilder() throws
Exception {
xslt.setEntityResolver(getEntityResolver());
xslt.setAllowStAX(allowStAX);
xslt.setUseJsonBody(useJsonBody);
+ xslt.setSecureProcessing(secureProcessing);
Review Comment:
Done in 0b732ee. `secureProcessing` is now `label = "advanced,security",
security = "insecure:dev", insecureValue = "false"` on both `XsltSaxonEndpoint`
(`@UriParam`) and `XsltSaxonComponent` (`@Metadata`), so
`camel.main.profile=prod` reports it when it is turned off — the same pattern
as camel-smooks' `allowExternalEntities`. The catalog, component/endpoint DSL
and `SecurityUtils` are regenerated (xslt-saxon and xj, which inherits the
option).
I kept it on the single option rather than adding a second knob; the setter
javadoc and the upgrade guide now state that `secureProcessing=false` also
restores the unrestricted parsing of a `Source` body.
_Claude Code on behalf of oscerd_
##########
docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_24.adoc:
##########
@@ -871,6 +871,26 @@ expanded, and the declaration itself is not passed on to
Smooks.
Set `allowExternalEntities=true` on the endpoint or on the data format for
documents that rely on external entities or
an external DTD.
+=== camel-xslt-saxon - external resource access is restricted for a Source
message body
+
+`XsltSaxonAggregationStrategy` now configures its Saxon `TransformerFactory`
the same way the `xslt-saxon`
+endpoint configures the factory it creates: `FEATURE_SECURE_PROCESSING` is
enabled and `ACCESS_EXTERNAL_DTD`
+and `ACCESS_EXTERNAL_STYLESHEET` are set to an empty string. Previously the
strategy used the Saxon defaults,
+so the same stylesheet ran with different settings depending on whether it was
used from the endpoint or from
+the strategy.
+
+In addition, both the strategy and the `xslt-saxon` endpoint now parse a
message body that is already a
+`javax.xml.transform.Source` still to be parsed, for example a `StreamSource`
or the body after a
+`convertBodyTo(Source.class)`, with a reader that does not resolve references
to external resources declared
+in the document. A `DOMSource`, a `StAXSource`, or a `Source` that already
carries its own reader is left
+unchanged, and a declaration internal to the document is still applied. The
other body types (`String`,
+`byte[]`, `InputStream`) are unaffected, as they already reach the transformer
through Camel's StAX/SAX
+conversion.
+
+On the `xslt-saxon` endpoint, set `secureProcessing=false` to restore the
previous parsing. A route that needs
Review Comment:
Reworded in 0b732ee. The guide now says `XsltSaxonAggregationStrategy` has
no switch for the `Source` parsing — `setTransformerFactory(...)` replaces the
factory, but `XsltSaxonBuilder` keeps `secureProcessing=true`, so the body is
still parsed with the restricted reader — and adds that
`FEATURE_SECURE_PROCESSING` disables reflexive Java extension functions on
Saxon PE/EE.
_Claude Code on behalf of oscerd_
--
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]
