CAMEL-24187: camel-core - CamelContext.getEndpoint() never caches a uri containing a % character#24805
Merged
Croway merged 1 commit intoJul 16, 2026
Conversation
…ri containing a % character AbstractCamelContext.doGetEndpoint() normalizes the uri once for its cache lookup, but on a cache miss handed that already-normalized uri to addEndpointToRegistry(), which normalized it a second time before storing the registry key. EndpointHelper.normalizeEndpointUri() is not idempotent for uris containing a literal '%' (it re-encodes it to '%25' on every pass), so the stored key never matched the lookup key computed on a later call, and a brand-new Endpoint was created and started on every single getEndpoint() call for such a uri instead of the cached singleton being reused. Add a normalized-aware overload of getEndpointKey()/addEndpointToRegistry() so only the doGetEndpoint() call site (whose uri is provably already normalized) skips the redundant second normalization pass. The public hasEndpoint(String), addEndpoint(String, Endpoint) and removeEndpoints(String) APIs still receive raw, caller-supplied uris and keep normalizing exactly as before. Discovered while investigating CAMEL-24171 (camel-plc4x S7 tag addresses like RAW(%DB1.DBX0.0:BOOL) contain a '%', unlike Modbus' RAW(coil:1), which is why only the S7 route reloaded its DefaultPlcDriverManager on every poll). Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
gnodet
approved these changes
Jul 16, 2026
gnodet
left a comment
gnodet
left a comment
Contributor
There was a problem hiding this comment.
Claude Code on behalf of gnodet
Review — CAMEL-24187: CamelContext.getEndpoint() never caches URIs containing %
Excellent bug analysis and clean, minimal fix. The root cause is clearly traced from the real-world plc4x S7 symptom down to the non-idempotent double-normalization in AbstractCamelContext.
The bug
Verified the flow on main:
doGetEndpoint()line ~810:uri = EndpointHelper.normalizeEndpointUri(uri)— first normalization- Line ~819:
NormalizedUri.newNormalizedUri(uri, true)— lookup key,true= "already normalized" - On cache miss, line ~876:
addEndpointToRegistry(uri, answer)→getEndpointKey(uri, endpoint)→NormalizedUri.newNormalizedUri(uri, false)— second normalization
For URIs with %, the second pass turns %41 into %2541, so the stored key never matches the lookup key. Every subsequent getEndpoint() call creates a new endpoint. ✅
The fix
Adding a normalized boolean parameter to addEndpointToRegistry() and getEndpointKey() is the right approach:
- Only the
doGetEndpoint()call site passesnormalized=true(its URI is provably already normalized at line ~810) - The existing no-arg overloads delegate with
normalized=false, preserving backward compatibility forhasEndpoint(),addEndpoint(),removeEndpoints()which receive raw, caller-supplied URIs - Javadoc on the new parameter clearly explains the non-idempotency issue
This is minimal, targeted, and doesn't change any existing public API behavior. ✅
Tests
Core test (DefaultEndpointRegistryTest#testGetEndpointIsCachedForUriContainingPercentCharacter):
- Uses
controlbus:route?routeId=RAW(%41)&action=status— clean isolation of the bug with no component dependencies - Verifies both
assertSame(same instance) and endpoint count (no registry accumulation) - ✅
Plc4x reproducer (Plc4XPollEnrichDoStartReproducerTest):
directGetEndpointCachingProbe— verifiesgetEndpoint()caching for S7-style URIdoStartRunsOnlyOnceEvenThoughBothRoutesArePolledManyTimes— reproduces the real-world consequence: S7 route (%in tag) and Modbus route (no%) both verifydoStart()runs only once across 10+ poll cyclesCountingPlc4XComponentsubclass countsdoStart()invocations without modifying production code — good pattern- Uses
MockEndpoint.assertIsSatisfied(context, 5, TimeUnit.SECONDS)correctly (not Awaitility) - ✅
LGTM — well-analyzed root cause, minimal fix, excellent test coverage at both the core and component level.
Contributor
|
🌟 Thank you for your contribution to the Apache Camel project! 🌟 🐫 Apache Camel Committers, please review the following items:
|
Contributor
|
The PR has a plc4j test as well |
davsclaus
approved these changes
Jul 16, 2026
Contributor
Author
yeah, that is expected, the issue was reported in https://issues.apache.org/jira/browse/CAMEL-24171 |
Contributor
|
🧪 CI tested the following changed modules:
🔬 Scalpel shadow comparison — Scalpel: 545 tested, 29 compile-only — current: 545 all testedMaveniverse Scalpel detected 574 affected modules (current approach: 545).
|
1 task
Contributor
Author
|
the |
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.
Summary
AbstractCamelContext.doGetEndpoint()normalizes the uri once for its cache lookup (NormalizedUri.newNormalizedUri(uri, true)- "already normalized"), but on a cache miss it handed that already normalized uri toaddEndpointToRegistry(), which calledgetEndpointKey(uri, endpoint)->NormalizedUri.newNormalizedUri(uri, false)- "not normalized yet, please normalize it" - re-runningEndpointHelper.normalizeEndpointUri()a second time before storing the registry key.EndpointHelper.normalizeEndpointUri()(viaURISupport.normalizeUri/UnsafeUriCharactersEncoder) is not idempotent for any uri containing a literal%: it encodes it to%25on every pass, so a second pass turns%41into%2541. Because the stored key ends up double-normalized while the lookup key computed fresh on a later call is only single-normalized, they never match again -CamelContext#getEndpoint(String)creates and starts a brand-newEndpointon every single call for any uri containing a%, instead of reusing the cached singleton.%(e.g.RAW(%DB1.DBX0.0:BOOL)), unlike Modbus' (RAW(coil:1)), which is why only the S7 route reloaded itsDefaultPlcDriverManageron every poll while the structurally identical Modbus route did not. camel-plc4x itself is not at fault - the same symptom applies to any component whose endpoint uri legitimately contains a%and is re-resolved by string on every use, aspollEnrich/toD/recipientListdo.Fix
Added a
normalized-aware overload ofgetEndpointKey()/addEndpointToRegistry()so only thedoGetEndpoint()call site (whose uri is provably already normalized) skips the redundant second normalization pass. The publichasEndpoint(String),addEndpoint(String, Endpoint)andremoveEndpoints(String)APIs still receive raw, caller-supplied uris directly and keep normalizing exactly as before - flipping the existing method wholesale would have silently broken those three call sites.Test plan
DefaultEndpointRegistryTest#testGetEndpointIsCachedForUriContainingPercentCharacterisolates the bug with no plc4x involvement (controlbus:route?routeId=RAW(%41)&action=statusresolved twice returns two different instances before the fix, the same instance after).Plc4XPollEnrichDoStartReproducerTestreproduces the real-world consequence: with the fix, both a S7-shaped and a Modbus-shapedpollEnrichroute only start theirPlc4XEndpointonce, no matter how many poll cycles run.camel-coretest suite run locally with the fix: 7300 tests, 18 failures / 3 errors - all individually verified to fail identically with the fix reverted (pre-existing onorigin/main, unrelated to this change). Zero regressions introduced.Claude Sonnet 5 on behalf of Croway