atiaomar1978-hub commented on PR #25569:
URL: https://github.com/apache/camel/pull/25569#issuecomment-5377446586
## 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 does
Aligns `pollEnrich` with CAMEL-24282 (`toD` / `enrich`): property
placeholders in the **per-message** recipient (e.g. from a header) are no
longer expanded at runtime. `PollEnricher.prepareRecipient()` now mirrors
`SendDynamicProcessor` — normalize URI without calling
`ProcessorHelper.prepareRecipient()` → placeholder resolution.
Scope is correctly narrowed to **`pollEnrich` only** after the first
revision broke `FtpProducerRecipientListIT`. The PR description explains that
well.
### Bugbot
No bugs found.
### Security
No medium-or-higher framework vulnerabilities. This change **strengthens**
the CAMEL-24282 security posture: `{{...}}` in an untrusted header is no longer
expanded on the main `pollEnrich().simple("${header.uri}")` path.
**Low — residual bypass on PollDynamicAware path:** `resolveUri()` still
calls `EndpointHelper.resolveEndpointUriPropertyPlaceholders()` when the
PollDynamicAware optimization runs before `prepareRecipient()`. Worth a
follow-up test or aligning `resolveUri()` for parity.
**Low — missing scheme validation:**
`SendDynamicProcessor.prepareRecipient()` validates `scheme:path`;
`PollEnricher`'s override does not. Pre-existing gap, not introduced here.
### Test coverage
Good for the main scenario (`pollEnrichPlaceholderInHeaderNotResolved`,
retained EIP tests, new `recipientListPlaceholderInRouteTextIsResolved`).
**Gaps (non-blocking):**
- No `pollEnrich("seda:{{secretTarget}}")` static-template test (upgrade
guide shows this; only `toD` has `placeholderInStaticTemplateIsResolved`)
- No PollDynamicAware pollEnrich test
- **Stale class Javadoc (lines 38–41):** says CAMEL-24414 extends to
`recipientList` / `routingSlip` / `dynamicRouter` — incorrect after scope
reduction; only `pollEnrich` changed
### CI failure analysis — not caused by this PR
Both 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:**
| Test | Failure |
|------|---------|
| `FileProducerNoForcedWritesTest.testNoForcedWrites` | Expected 1 message,
got 3 |
| `ThreadsRejectedExecutionTest.testThreadsRejectedAbortNoRedelivery` |
Expected 0 at mock:error, got 1 |
| `PQCSignatureOnlyKeyStoreTest` / `PQCSignatureWithKeyStoreTest` |
`InvalidKeyException: unknown private key passed to ML-DSA` (3 reruns) |
**Java 17:**
| Test | Failure |
|------|---------|
| `SplitPropertiesFileIssueTest.testSplitPropertiesFileAndRoute` | Assertion
failure |
| `VertxWebsocketTest` (2 tests) | Timing/assertion failures |
These are known CI flakes on `main` — **re-run CI** recommended; no code
change needed in this PR.
The lightweight `build` job passed; only the full incremental test matrix
failed.
### Documentation
Upgrade guide entry for 4.23 is clear. Please fix the stale test class
Javadoc to match pollEnrich-only scope.
### Summary
| Dimension | Result |
|-----------|--------|
| Correctness | Pass |
| Security | Pass (improves CAMEL-24282 alignment) |
| Tests | Pass (minor gaps) |
| CI | Unrelated flakes — re-run recommended |
| Bugbot | No bugs |
--
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]
