[backport camel-4.18.x] CAMEL-24677: camel-smooks - align XML reader configuration with the other XML components - #26329
Conversation
…ther XML components Backport of apache#26270 to camel-4.18.x. Adapted for this branch: - The endpoint option drops the @UriParam security/insecureValue attributes, which do not exist on camel-4.18.x. - The new component test uses JUnit 5 (camel-test-junit5), matching this branch. The runtime fix, the allowExternalEntities option, and the two new XML-security tests are otherwise identical to the merged PR. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
8f6386f to
048fbbd
Compare
…4.22 and 4.18 upgrade guides The fix was backported to camel-4.22.x (apache#26327, merged) and camel-4.18.x (apache#26329). The version-specific upgrade guides live on main, so add the 4.22.1 and 4.18.5 notes here to match. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
gnodet-bot
left a comment
There was a problem hiding this comment.
Clean security backport of #26270 to camel-4.18.x. Reviewed the 8 hand-written files (SmooksSecuritySupport, SmooksProcessor, SmooksEndpoint, SmooksDataFormat × model + runtime, SmooksDataFormatReifier, both security tests) and all generated artifacts.
Security architecture is sound:
SmooksSecuritySupportis identical tomain— correct use ofDocumentBuilderFactorywith all external entity features disabled, propersynchronizedaround the non-thread-safe factory,DocumentBuilder.parse()correctly outside the lock.hardenXmlInputflag computed once atdoStart()— no per-message overhead when the config uses a custom reader (EDI/CSV/JSON).getSource()reordering inSmooksProcessormatchesmain:Node→hardenXmlInput→byte[]→InputStream, ensuring DOM payloads bypass the security pre-parse (they're already parsed).
Backport adaptations are correct:
- No
security/insecureValueattributes on@UriParam(those don't exist on 4.18.x) — the security label is set via thelabelattribute instead. - JUnit 5 test framework (vs JUnit 6 on
main). - Generated files are consistent with the 4.18.x code generators.
Wiring verified: SmooksComponent.configureSmooksProcessor() calls setProperties(smooksProcessor, parameters) which injects allowExternalEntities from endpoint URI parameters into the processor.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
gnodet-bot
left a comment
There was a problem hiding this comment.
Solid backport of CAMEL-24677 (XXE hardening for camel-smooks) to camel-4.18.x. Reviewed the security implementation, API threading, concurrency, and backport adaptations — no issues found.
Security: SmooksSecuritySupport.createSecureDocumentBuilderFactory() correctly disables FEATURE_SECURE_PROCESSING, external general entities, external parameter entities, and external DTD loading. The defense-in-depth approach (swallowing ParserConfigurationException when a feature isn't recognized) is appropriate.
Concurrency: The shared DocumentBuilderFactory is correctly synchronized for newDocumentBuilder() calls, and the DocumentBuilder instance is used only within the calling thread.
Payload ordering in SmooksProcessor.getSource(): Moving the instanceof Node check before the hardenXmlInput path is correct — DOM payloads have no InputStream to parse and carry no XXE risk.
Backport adaptations: @UriParam uses label = "security" instead of the main-only security/insecureValue attributes. Tests correctly use camel-test-junit5. No SecurityUtils dependency (doesn't exist on this branch).
This review was generated by an AI agent, Hermès on behalf of @gnodet.
Backport of #26270 to
camel-4.18.x.Original PR: #26270 — CAMEL-24677
Original author: @oscerd
Not a straight cherry-pick — adapted for camel-4.18.x
camel-4.18.x's@UriParamdoes not have thesecurity/insecureValueattributes (that infrastructure is newer) andSecurityUtilsdoes not exist on this branch, so theallowExternalEntitiesendpoint option is declared without them.camel-test-junit5(this branch uses JUnit 5;mainuses JUnit 6).main-only 4.23 upgrade-guide entry and generated files that do not exist on this branch (newercamel-java-io/camel-yaml-iowriters, the canonical/model YAML schema variants) are excluded.The runtime fix (hardened DOM pre-parse via
SmooksSecuritySupport, applied only when the config uses the default XML reader) and theallowExternalEntitiesoption are otherwise identical to the merged PR.Verified on camel-4.18.x
A full
mvn clean install -DskipTestspasses on this branch, and all generated files (catalog, endpoint-DSL, YAML-DSL deserializers/schema, XSDs) are regenerated forcamel-4.18.x. The two new XML-security tests pass, confirming external entities are not resolved by default and are resolved withallowExternalEntities=true.As a non-mechanical backport, this needs a normal review (it is not eligible for the straight-cherry-pick fast path).
🤖 Generated with Claude Code