oscerd opened a new pull request, #25683:
URL: https://github.com/apache/camel/pull/25683

   Fixes [CAMEL-24475](https://issues.apache.org/jira/browse/CAMEL-24475).
   
   ## The two document types disagreed on parser hardening
   
   `XPathBuilder` resolves the message payload to the configured `documentType` 
before evaluating the
   expression.
   
   With the default `documentType` of `org.w3c.dom.Document`, the type 
converter builds the DOM through
   `XMLConverterHelper.createDocumentBuilderFactory()` — 
`FEATURE_SECURE_PROCESSING`,
   `disallow-doctype-decl`, `external-general-entities=false`, and empty 
`ACCESS_EXTERNAL_DTD` /
   `ACCESS_EXTERNAL_SCHEMA`. `XPathFeatureTest#testXPathDocTypeDisallowed` has 
pinned that behaviour for
   years.
   
   When `documentType` is set to `org.xml.sax.InputSource` — documented as a 
way to use SAX streams — the
   resolved object was handed straight to `XPathExpression`:
   
   ```java
   answer = xpathExpression.evaluate(inputSource, resultQName);
   ```
   
   The JDK implementation then creates a `DocumentBuilder` of its own with the 
plain defaults
   (`com.sun.org.apache.xpath.internal.jaxp.XPathImplUtil.getDocument`), so 
none of the above applied on
   that path. Four sites did this: two in `doInEvaluateAs` (the evaluation 
path) and two in
   `logNamespaces` (the `logNamespaces` option), one of which also discarded 
the reader its `SAXSource`
   was carrying.
   
   Measured end-to-end, before the change — an `InputStream` body with a 
`DOCTYPE` naming a local file
   returns **the contents of that file** as the XPath result:
   
   ```
   >>>> PROOF result = [CANARY-SHOULD-NOT-BE-READ]
   ```
   
   After the change the same input raises `TypeConversionException` with a 
`SAXParseException` root cause
   ("DOCTYPE is disallowed").
   
   `documentType` is a performance choice. Neither its name nor its javadoc 
suggests it also changes the
   parser configuration, so the two paths should agree.
   
   ## The change
   
   All four sites now go through `toHardenedDocument()`, which converts the 
`InputSource` with the type
   converter — the same `toDOMDocument(InputSource, Exchange)` the default 
`documentType` already uses, so
   the hardened factory and any user customisation of it apply identically.
   
   This does **not** add a document parse. 
`XPathExpression.evaluate(InputSource)` already built a full DOM
   internally, confirmed by evaluating an expression matching the *first* 
element of a document malformed
   only in its *tail*: it fails with a `SAXParseException` raised from 
`XPathImplUtil.getDocument`, so the
   whole document was being parsed before evaluation either way.
   
   ## Compatibility
   
   A message carrying a `DOCTYPE` that previously evaluated under 
`documentType=InputSource` is now
   rejected — the same behaviour the default `documentType` has always had. 
Upgrade-guide entry included,
   naming the existing 
`org.apache.camel.xmlconverter.documentBuilderFactory.feature:` system 
properties as
   the escape hatch for a deployment that genuinely parses documents with a 
`DOCTYPE`.
   
   No public API change: `toHardenedDocument` is a new `protected` method 
alongside the existing
   `getDocument` strategy method.
   
   ## Testing
   
   Three tests added to `XPathFeatureTest`, next to the existing 
default-`documentType` coverage:
   
   * `docTypeIsAlsoDisallowedForAnInputSourceDocumentType` — covers **both** 
`doInEvaluateAs` branches
     (with and without a result QName). Verified RED against unfixed code with
     *"Expecting code to raise a throwable"* — the unfixed code completed and 
returned the file contents.
   * `theDocumentBuilderFactoryFeaturesAlsoRelaxTheInputSourceDocumentType` — 
verifies the escape hatch the
     upgrade guide documents actually works on this path, rather than assuming 
it does.
   * `anInputStreamBodyConvertsToAnInputSourceDocumentType` — guards the 
assumption the first test rests
     on. Worth having: an earlier draft used a `String` body and "failed" 
against unfixed code for the
     wrong reason, since `String` has no converter to `InputSource`.
   
   Suites run: `XPathFeatureTest` 7/7 · `core/camel-core` `*XPath*,*Xml*,*XML*` 
287/287 ·
   `camel-saxon` 84/84 (the Saxon paths exercise the same builder) · full `mvn 
clean install -DskipTests`
   across the reactor.
   
   _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]

Reply via email to