CAMEL-24414: camel-core - resolve pollEnrich property placeholders at build time - #25569
CAMEL-24414: camel-core - resolve pollEnrich property placeholders at build time#25569oscerd wants to merge 1 commit into
Conversation
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 562 tested, 28 compile-only — current: 561 all testedMaveniverse Scalpel detected 590 affected modules (current approach: 561).
|
… build time
CAMEL-24282 stopped resolving property placeholders ({{...}}) on the per-message
evaluated recipient for toD and enrich, and stated that aligning pollEnrich was
deferred to a follow-up. This is that follow-up.
pollEnrich resolves its static endpoint uri at build time like toD and enrich,
but its per-message recipient went through ProcessorHelper.prepareRecipient,
which ends in CamelContextExtension.normalizeUri() and therefore
EndpointHelper.resolveEndpointUriPropertyPlaceholders(). PollEnricher now
overrides prepareRecipient with the same build-time behaviour
SendDynamicProcessor uses, so a {{...}} token appearing only in the runtime
value is treated as a literal part of the endpoint uri.
recipientList, routingSlip and dynamicRouter are deliberately left unchanged.
An earlier revision of this change altered the shared ProcessorHelper and so
covered them too; that broke camel-ftp's FtpProducerRecipientListIT, which sends
a "ftp://admin@localhost:{{ftp.server.port}}/list?password=admin" template
through the recipient header. That is an in-tree example of a legitimate use:
the recipient is supplied at runtime but the placeholder comes from
configuration, not from an untrusted sender. Whether the engine should still
expand it is a trade-off worth deciding on the dev list rather than in this
change.
Adds recipientListPlaceholderInRouteTextIsResolved, which records the related
fact that a placeholder written in the route is resolved at build time by the
model for these EIPs as well, independently of the per-message path.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
514e40e to
a1edacb
Compare
Review (Bugbot + Grok + manual)AI-generated review. Verdict: Approve the change with minor doc/test nits. CI red is unrelated flaky infrastructure tests, not this PR. What the PR doesAligns Scope is correctly narrowed to BugbotNo bugs found. SecurityNo medium-or-higher framework vulnerabilities. This change strengthens the CAMEL-24282 security posture: Low — residual bypass on PollDynamicAware path: Low — missing scheme validation: Test coverageGood for the main scenario ( Gaps (non-blocking):
CI failure analysis — not caused by this PRBoth Java 17 and Java 25 incremental builds failed on unrelated flaky tests (Surefire rerun count = 2 exhausted). Changed modules passed; failures are elsewhere in the 561-module matrix. Java 25:
Java 17:
These are known CI flakes on The lightweight DocumentationUpgrade guide entry for 4.23 is clear. Please fix the stale test class Javadoc to match pollEnrich-only scope. Summary
|
Fixes CAMEL-24414.
What changes
CAMEL-24282 stopped resolving property placeholders (
{{...}}) on the per-message evaluated recipient fortoDandenrich, and its upgrade-guide entry says aligningpollEnrich"is deferred to a follow-up". This is that follow-up — so it needs no reversal argument.pollEnrichresolves its static endpoint URI at build time exactly liketoDandenrich, but its per-message recipient went through the sharedProcessorHelper.prepareRecipient, which ends innormalizeUri()→resolveEndpointUriPropertyPlaceholders().PollEnrichernow overridesprepareRecipientwith the same build-time behaviourSendDynamicProcessoralready uses.What I got wrong
I originally changed the shared
ProcessorHelper, which covered all four remaining dynamic-URI EIPs. I argued the 4.22 exclusion was unnecessary because a placeholder written in the route resolves at build time anyway:That probe was correct but incomplete. It missed the pattern that actually matters — a configuration-derived URI template flowing through a header, which this repository itself relies on:
The recipient is supplied at runtime, but the placeholder comes from configuration rather than from an untrusted sender. The 4.22 decision was right; only the reason it gave ("no static template resolved at build time") is inaccurate.
recipientList,routingSlipanddynamicRouterare therefore untouched here and their tests restored.Worth a dev-list discussion, separately
Should those three keep expanding placeholders in a runtime recipient at all? The engine cannot distinguish an operator's config template from a sender-supplied
{{env:SECRET}}arriving in a header. There is a real argument for aligning them, andFtpProducerRecipientListITis the concrete migration cost. That trade-off belongs to the PMC, not to this PR.Tests
DynamicEndpointMessagePlaceholderTestgoes 7 → 8.pollEnrichPlaceholderInHeaderStillResolvedbecomes...NotResolved; the other three keep their existing expectations. The newrecipientListPlaceholderInRouteTextIsResolvedis kept because the fact it records is worth pinning down either way.Rebased onto current main (the guide conflict came from #25566 merging).
Backport
main only, matching CAMEL-24282.
_Claude Code on behalf of