gnodet-bot commented on code in PR #28558:
URL: https://github.com/apache/camel/pull/28558#discussion_r4440010929
##########
components/camel-xslt-saxon/src/main/java/org/apache/camel/component/xslt/saxon/XsltSaxonAggregationStrategy.java:
##########
@@ -50,7 +51,14 @@ public class XsltSaxonAggregationStrategy extends
XsltAggregationStrategy {
public XsltSaxonAggregationStrategy(String xslFileLocation) {
super(xslFileLocation);
- setTransformerFactory(new TransformerFactoryImpl());
+ TransformerFactoryImpl factory = new TransformerFactoryImpl();
+ try {
+ XsltSaxonHelper.configureSecureProcessing(factory, true);
+ } catch (Exception e) {
Review Comment:
`configureSecureProcessing()` declares `throws Exception` but its only
operation is `factory.setFeature()`, which throws
`TransformerConfigurationException`. `catch (Exception e)` is broader than
needed.
```suggestion
} catch (TransformerConfigurationException e) {
```
Minor nit, not a correctness issue.
##########
docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_18.adoc:
##########
@@ -98,6 +98,29 @@ Additionally:
- When `alwaysReauthenticate=true` (the default), no caller identity stays
bound to the processing thread after the
policy check completes.
+=== 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`
Review Comment:
`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.
This is inaccurate for the endpoint path on 4.18.x. Looking at
`registerSaxonExtensionFunctions()` in `XsltSaxonHelper.java`:
```java
if (saxonExtensionFunctions != null && !saxonExtensionFunctions.isEmpty()) {
factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING,
secureProcessing);
// ...
}
```
`FEATURE_SECURE_PROCESSING` is only set on the endpoint's factory when
extension functions are configured. When no extension functions are present
(the common case), the endpoint's factory does **not** have
`FEATURE_SECURE_PROCESSING` set. The aggregation strategy always sets it
unconditionally in its constructor.
Two options to resolve this:
**Option A** — unconditionally set `FEATURE_SECURE_PROCESSING` on the
endpoint's factory, making the behavior match the strategy and the upgrade
guide text:
```suggestion
`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,
```
Wait — this is the text, not the code. For a code fix, call
`XsltSaxonHelper.configureSecureProcessing(tf, secureProcessing)`
unconditionally in `XsltSaxonEndpoint.createXsltBuilder()` (inside the `if
(factory instanceof TransformerFactoryImpl)` block, before
`registerSaxonExtensionFunctions`).
**Option B** — correct the upgrade guide to reflect the actual behavior: the
strategy always enables `FEATURE_SECURE_PROCESSING`; the endpoint only enables
it when extension functions are configured.
--
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]