CAMEL-24298: add an optional allowedSchemes allow-list to the toD dynamic-URI EIP - #25315
Open
oscerd wants to merge 1 commit into
Open
CAMEL-24298: add an optional allowedSchemes allow-list to the toD dynamic-URI EIP#25315oscerd wants to merge 1 commit into
oscerd wants to merge 1 commit into
Conversation
…amic-URI EIP The dynamic-URI EIP toD computes its recipient endpoint uri from a route-author expression at runtime, with no way to restrict which component schemes a dynamic recipient may resolve to. Add an optional allowedSchemes attribute (comma-separated component-scheme allow-list) on ToDynamicDefinition, wired through ToDynamicReifier into SendDynamicProcessor, which rejects a resolved recipient whose scheme is not in the list (independently of ignoreInvalidEndpoint). Default unset = any scheme allowed (no behavioural change). Useful for low-code / Kamelet deployments. wireTap extends ToDynamicDefinition and shares the reifier/processor path, so it inherits and enforces the same option. The 5 sibling dynamic-URI EIPs (recipientList, routingSlip, dynamicRouter, enrich, pollEnrich) can follow in later PRs once this pattern is reviewed. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
gnodet
reviewed
Aug 3, 2026
gnodet
left a comment
gnodet
left a comment
Contributor
There was a problem hiding this comment.
Nice work on this defence-in-depth feature — the enforcement placement between prepareRecipient() and resolveEndpoint() is exactly right, and the deliberate bypass of ignoreInvalidEndpoint for disallowed schemes is a sound security decision. Two non-blocking suggestions below.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of @gnodet
| return true; | ||
| } | ||
| for (String allowed : allowedSchemes.split(",")) { | ||
| if (allowed.trim().equals(scheme)) { |
Contributor
There was a problem hiding this comment.
Performance nit: allowedSchemes.split(",") allocates a new String[] on every exchange. For a high-throughput route this adds up. Consider pre-parsing into a Set<String> once in setAllowedSchemes():
private Set<String> allowedSchemesSet;
public void setAllowedSchemes(String allowedSchemes) {
this.allowedSchemes = allowedSchemes;
if (allowedSchemes != null) {
this.allowedSchemesSet = Arrays.stream(allowedSchemes.split(","))
.map(String::trim)
.filter(s -> !s.isEmpty())
.collect(Collectors.toUnmodifiableSet());
} else {
this.allowedSchemesSet = null;
}
}
private boolean isSchemeAllowed(String scheme) {
if (allowedSchemesSet == null) {
return true;
}
return allowedSchemesSet.contains(scheme);
}This also makes the whitespace-trimming behavior consistent for all entries.
| } | ||
| }; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Test coverage suggestion: The test covers the core positive/negative paths well, but the route only configures a single allowed scheme ("mock"). Since the documented use case is a comma-separated list ("http,https"), it would strengthen confidence to also test:
- Multiple allowed schemes (e.g.
"mock,seda") — verify both are accepted - Whitespace in the list (e.g.
"mock, seda") — the code handles this viaallowed.trim()but it's untested
Could be a follow-up — not blocking.
davsclaus
reviewed
Aug 3, 2026
davsclaus
left a comment
davsclaus
left a comment
Contributor
There was a problem hiding this comment.
Nice feature — clean model/reifier/processor layering, correct placement of the security check (after prepareRecipient, before resolveEndpoint, outside the ignoreInvalidEndpoint catch), and sound decision to always hard-fail a disallowed scheme.
I agree with the two suggestions from the prior review (pre-parse allowedSchemes into a Set<String> to avoid per-exchange split(","), and add a multi-scheme test). Two additional non-blocking observations below.
This review does not replace specialized tools such as CodeRabbit, Sourcery, or SonarCloud.
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of @davsclaus
| public void configure() { | ||
| from("direct:start").toD().allowedSchemes("mock").uri("${header.target}"); | ||
| } | ||
| }; |
Contributor
There was a problem hiding this comment.
Missing test: ignoreInvalidEndpoint independence.
The PR description, the code comment in SendDynamicProcessor, and the documentation all explicitly state that a disallowed scheme is rejected independently of ignoreInvalidEndpoint. This is a security-relevant contract worth locking down with a test.
Consider adding a third test method with a separate route that sets both options:
@Test
void disallowedSchemeIsRejectedEvenWhenIgnoreInvalidEndpoint() {
assertThatThrownBy(() -> template.sendBodyAndHeader("direct:lenient", "Hello", "target", "seda:blocked"))
.isInstanceOf(CamelExecutionException.class)
.cause()
.isInstanceOf(ResolveEndpointFailedException.class)
.hasMessageContaining("not in the allowed schemes");
}with the route:
from("direct:lenient").toD().allowedSchemes("mock").ignoreInvalidEndpoint("true").uri("${header.target}");Non-blocking — can be a follow-up.
| <from uri="direct:start"/> | ||
| <toD uri="${header.target}" allowedSchemes="http,https"/> | ||
| </route> | ||
| ---- |
Contributor
There was a problem hiding this comment.
Consider adding a YAML DSL example.
The section targets "low-code / Kamelet-style deployments" which predominantly use YAML DSL, but only Java and XML examples are shown. A YAML example would serve the primary target audience:
- route:
from:
uri: direct:start
steps:
- toD:
uri: "${header.target}"
allowedSchemes: "http,https"Non-blocking.
Contributor
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
Contributor
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 549 tested, 26 compile-only — current: 546 all testedMaveniverse Scalpel detected 575 affected modules (current approach: 546).
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The dynamic-URI EIP
toDcomputes its recipient endpoint uri from a route-author expression at runtime (e.g.toD("${header.target}")), with no way to restrict which component schemes the recipient may resolve to. This adds an optionalallowedSchemesattribute (comma-separated component-scheme allow-list) onToDynamicDefinition, wired throughToDynamicReifierintoSendDynamicProcessor.A resolved recipient whose scheme is not in the list is rejected with a
ResolveEndpointFailedException— independently ofignoreInvalidEndpoint, so a disallowed scheme always hard-fails. Default unset = any scheme allowed, so there is no behavioural change. Useful for low-code / Kamelet deployments that want to constrain e.g..toD("${header.dest}")to a fixed set of components.Scope (MVP)
This is the
toD-only MVP of CAMEL-24298.wireTapextendsToDynamicDefinitionand shares the reifier/processor path, so it inherits and enforces the same option automatically. The 5 sibling dynamic-URI EIPs (recipientList,routingSlip,dynamicRouter,enrich,pollEnrich) can follow in separate PRs once this pattern is reviewed and accepted — so the JIRA stays open after this merges.Tests
ToDynamicAllowedSchemesTest: an allowed scheme is sent; a disallowed scheme is rejected withResolveEndpointFailedException. Full-reactormvn clean install -DskipTestsis green (model JSON, XML/YAML schemas, DSL writers/parsers/deserializers, and the toD EIP doc regenerated).Docs
Added an
allowedSchemessection to the toD EIP doc (Java + XML examples).Backport
main only — new additive feature (default unrestricted), not a bug fix.
Partially addresses CAMEL-24298 (toD MVP; sibling EIPs to follow).
Claude Code on behalf of Andrea Cosentino (@oscerd)