Claus Ibsen created CAMEL-24524:
-----------------------------------
Summary: camel-core: URISupport.normalizeUri fast path skips value
encoding when parameters are already sorted, causing duplicate endpoints
Key: CAMEL-24524
URL: https://issues.apache.org/jira/browse/CAMEL-24524
Project: Camel
Issue Type: Bug
Reporter: Claus Ibsen
h3. Summary
{{URISupport.normalizeUri()}} is used to compute the registry/cache key for
Camel endpoints, and is documented to reorder query parameters alphabetically
so that two endpoint URIs differing only in parameter order are treated as
identical. This holds for most URIs, but breaks whenever a parameter value
contains a character that requires URL-encoding (e.g. a colon in a
{{host:port}} value). In that case, the *original* parameter order silently
affects the normalized output, so two logically identical URIs can normalize to
two different strings - causing duplicate endpoints (and duplicate
producers/consumers/connections) to be created silently, with no error or
warning.
Originally reported/analyzed here:
https://tushar-c23.github.io/posts/camel-uri-normalization/
h3. Reproduction
{code:java}
String uri1 = "kafka:mytopic?brokers=localhost:19092&groupId=mygroup"; //
already alphabetical (brokers < groupId)
String uri2 = "kafka:mytopic?groupId=mygroup&brokers=localhost:19092"; // needs
reordering
System.out.println(URISupport.normalizeUri(uri1));
System.out.println(URISupport.normalizeUri(uri2));
{code}
Actual output (verified on {{main}}, camel-util 4.22.1-SNAPSHOT):
{noformat}
kafka://mytopic?brokers=localhost:19092&groupId=mygroup
kafka://mytopic?brokers=localhost%3A19092&groupId=mygroup
{noformat}
The two normalized strings differ (raw {{:}} vs {{%3A}}), even though the input
URIs are semantically identical. Any code that keys a map/registry by the
normalized URI (e.g. the endpoint cache) will treat these as two different
endpoints.
h3. Root cause
{{URISupport.normalizeUri()}} has two code paths (see
{{core/camel-util/src/main/java/org/apache/camel/util/URISupport.java}}):
* *Fast path* - {{doFastNormalizeUri()}} -> {{buildReorderingParameters()}}
(~line 799). This path only rebuilds the query string via
{{createQueryString(array, parameters, true)}} - the *only* place encoding (via
{{URLEncoder.encode}}) is applied - when it detects that the parameter keys are
*not* already in alphabetical order:
{code:java}
boolean sort = false;
String prev = null;
for (String key : entries) {
if (prev != null) {
int comp = key.compareTo(prev);
if (comp < 0) {
sort = true;
break;
}
}
prev = key;
}
if (sort) {
// only place where createQueryString(...) / encoding happens
query = URISupport.createQueryString(array, parameters, true);
}
{code}
If the keys are already sorted, or the query has a single parameter (no {{&}}),
the original raw query string is returned completely untouched - i.e. *not
encoded at all*.
* *Complex/legacy path* - {{doComplexNormalizeUri()}} (used when the URI
contains characters outside the fast parser's "safe" set). This path *always*
calls {{createQueryString(...)}} regardless of whether reordering was needed,
so it always produces a consistently encoded, canonical result. This path is
*not* affected by this bug.
Because a colon is considered "safe" by
{{UnsafeUriCharactersEncoder.isSafeFastParser()}} (so URIs containing it still
take the fast path), but {{URLEncoder.encode()}} (used inside
{{createQueryString}}) *does* encode colons, the fast path's output for the
same logical URI depends on whether the original parameter order happened to
require a rebuild.
This was introduced as a performance optimization in CAMEL-14648 (2020) - the
intent was purely to avoid rebuilding the query string when reordering wasn't
necessary; encoding consistency was not part of the design consideration at the
time. It is not a deliberate behavioral decision.
Checked for duplicates: CAMEL-24187 (already fixed, released in 4.22.0/4.18.4)
covers a related but distinct bug - non-idempotent double-normalization of
{{%}} characters. It does not cover this parameter-order/encoding inconsistency.
h3. Impact
* Silent duplicate {{Endpoint}} instances (and therefore duplicate
producers/consumers, connections, threads) for logically identical URIs, with
no exception or log warning.
* Reported real-world impact: a Kafka producer leak where varying parameter
order (due to a colon in the broker address) caused multiple {{KafkaProducer}}
instances to be created for what should have been a single shared endpoint.
* Affects any component/DSL usage where endpoint URIs are hand-assembled with
parameters in non-canonical order and at least one value requires encoding
(colons, spaces, etc.). Endpoints built via the fluent Java DSL (which stores
parameters in a {{TreeMap}}, always alphabetical) are not affected in practice,
but this is incidental, not guaranteed by contract.
h3. Suggested fix direction
Align the fast path's behavior with the complex path's guarantee that the
result is always canonically encoded, without regressing the performance win
from CAMEL-14648 for the common case:
* When scanning parameter keys to decide whether a re-sort is needed, also
detect whether any value requires encoding (e.g. by comparing each value to its
encoded form, or checking against the same "unsafe" character set used
elsewhere). If either condition is true, rebuild via {{createQueryString(...)}}.
* Alternatively (simpler, slightly more conservative on performance): always
rebuild the query via {{createQueryString(...)}} in
{{buildReorderingParameters()}}, only keeping the sort-detection to choose
whether the keys array itself needs sorting first. This removes the "skip
rebuild" fast-path micro-optimization but keeps the larger win of using the
lightweight {{CamelURIParser}} instead of {{java.net.URI}}.
* Also fix the single-parameter short-circuit (query with no {{&}}) which
currently returns the raw, unencoded value verbatim - for consistency with the
complex path, which always encodes single-parameter queries too.
A regression test should assert that {{URISupport.normalizeUri()}} produces the
*same* output for two URIs that differ only in parameter order, for values
containing characters requiring encoding (colon, space, etc.), including the
single-parameter case.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
