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]

Reply via email to