CAMEL-25188: camel-util - URISupport.normalizeUri gives the same uri when normalizing a normalized uri (space, = and # in values, + in keys) - #27131
Merged
Conversation
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: 575 of 697 tested, 27 compile-only — current: 575 all testedMaveniverse Scalpel detected 575 affected modules (current approach: 575). Skip-tests mode would test 575 modules (2 direct + 574 downstream), skip tests for 27 (generated code, meta-modules) Modules Scalpel would test (575)
Modules with tests skipped (27)
Build reactor — dependencies compiled but only changed modules were tested (2 modules, 23.2s total)Total reactor time: 23.2s
Top 20 slowest modules:
|
oscerd
reviewed
Oct 1, 2026
oscerd
left a comment
oscerd
left a comment
Contributor
There was a problem hiding this comment.
Thanks for chasing this down. The fix is correct, and spaces now normalize the same way as in 4.22.1.
What I checked:
- On the fast path a
%20can only come from a space.CamelURIParserrejects%,URIScanner.parseQueryonly turns+into a space, and the other escapes in a fast-path value (%23for#,%3D,%26) can't match%20. So the replace can't change anything else. - I compared
normalizeUrifrom camel-4.22.1, current main and this branch on about 40 URIs: every spelling of a space,a++b, a lone+, repeated keys,RAW(...), quartz cron with+, a#in a value. For all the space cases, this branch matches 4.22.1 and normalizing twice gives the same URI. The value the endpoint gets is the same in every case. - With a
DefaultCamelContext: on main,getEndpoint(ep.getEndpointUri())forlog:foo?marker=a+bcreates a second endpoint andhasEndpoint(ep.getEndpointUri())returns null. This branch finds the cached endpoint. - Both new tests fail on current main, so they guard the regression.
- No upgrade-guide entry needed:
%20was never released, and the CAMEL-24524 note in the 4.23 guide doesn't mention spaces. - No backport needed: CAMEL-24524 is only on main (
camel-4.22.xandcamel-4.18.xdon't havebuildSafeQueryString), so the fixVersion is 4.23.0 only.
One non-blocking point inline: normalizing twice still changes the URI when a value has = or # together with /, : or '. It's the same mechanism, and it also comes from CAMEL-24524, so it isn't released yet either.
Minor, also from CAMEL-24524: sb.append(key) (L942) writes the decoded key as is, so a + in a key becomes a raw space. log:foo?a+b=1 normalizes to log://foo?a b=1, then back to log://foo?a+b=1. 4.22.1 kept log://foo?a+b=1. It's rare because keys are option names, but worth adding to the same follow-up.
Nit: the PR description could link CAMEL-25190 for the %2B case it leaves out.
This review checks the project's rules and conventions; it doesn't replace static analysis or dedicated review tools.
Claude Code on behalf of oscerd
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
… query value as + in the fast path too Since CAMEL-24524 the fast path wrote a space as %20 while the complex path writes +, so normalizing a normalized uri changed it, and the same endpoint written as a+b or a%20b got two endpoint keys. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
…key or value needs a percent escape A value with = or # got a percent escape in the fast path, and a uri with % goes to the complex path, which form-encodes the whole query, so normalizing a normalized uri changed it again (eg selector=somekey='somevalue' or secretKey=abc/def==) and the same endpoint could get two keys. The fast path now form-encodes the query the same way in that case, and encodes keys as values, so a + in a key is kept. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Claus Ibsen <claus.ibsen@gmail.com>
davsclaus
force-pushed
the
fix/CAMEL-25188
branch
from
October 1, 2026 13:32
9b52046 to
9a2e5da
Compare
Contributor
Author
|
Claude Code on behalf of davsclaus @oscerd thanks for the thorough review. Both points are now fixed here:
The description now links CAMEL-25190 for Tests after rebasing on main: camel-util 292, camel-core 8036 (0 failures), camel-rest-openapi 145, camel-quartz 105, plus a full root |
davsclaus
changed the title
gnodet-bot
approved these changes
Oct 1, 2026
gnodet-bot
left a comment
gnodet-bot
left a comment
Contributor
There was a problem hiding this comment.
Solid fix. The two-stage strategy — buildSafeQueryString first, then fall back to createQueryString when % appears — correctly aligns the fast path's output with the complex normalizer, guaranteeing idempotency.
Verified:
UnsafeUriCharactersEncoder.encode()only touches ASCII < 128, so%20in its output always comes from a space — the%20→+replacement insafeEncodeQueryPartis safe.- The fallback to
createQueryString(URLEncoder-based) fires exactly when the fast parser would reject the output (it rejects URIs with%), so both paths converge on the same form-encoded representation. - Key encoding via
safeEncodeQueryPartfixes the+-in-key regression (log:foo?a+b=1stayslog://foo?a+b=1). - The 19-URI
testNormalizeTwiceGivesTheSameUritest covers the key edge cases (spaces,=/#in values,+in keys, RAW parameters, repeated keys, non-ASCII, nested URLs). - Upgrade guide updated to document the behaviour change.
Both previous oscerd findings (idempotency for =/# values, + in keys) are addressed.
This review was generated by an AI agent, Hermès on behalf of @gnodet.
oscerd
approved these changes
Oct 1, 2026
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.
Claude Code on behalf of davsclaus
CAMEL-25188
Since CAMEL-24524 (#25810, on main only and not released), normalizing a normalized URI could change it, so the same endpoint could get two keys, and endpoint registry and cache lookups could miss (
getEndpoint(ep.getEndpointUri())created a second endpoint). The fast path ofURISupport.normalizeUriwrites%escapes, and a URI with%goes to the complex path, which encodes differently:log:foo?marker=a+blog://foo?marker=a%20blog://foo?marker=a+blog://foo?marker=a+bjms:queue:foo?selector=somekey='somevalue'...selector=somekey%3D'somevalue'...selector=somekey%3D%27somevalue%27...selector=somekey%3D%27somevalue%27log:foo?secretKey=abc/def==...secretKey=abc/def%3D%3D...secretKey=abc%2Fdef%3D%3D...secretKey=abc%2Fdef%3D%3Dlog:foo?a+b=1(a+in a key)log://foo?a b=1log://foo?a+b=1log://foo?a+b=1The fix in the fast path:
+, as the complex path and all released versions do;=or#in a value), the whole query is form-encoded, the same way as the complex path, so the second normalization gives the same string. Without such an escape the query keeps the readable form from CAMEL-24524 (produces=application/json,localhost:19092);+in a key is kept.Compared with 4.22.1, the output only differs where 4.22.1 itself depended on the key order. The CAMEL-24524 note in the 4.23 upgrade guide now says when the query is form-encoded. No backport: CAMEL-24524 is only on main.
Tests
URISupportTest: every spelling of a space normalizes the same; values with=/#form-encode the query; a+in a key; normalizing twice gives the same URI (19 URIs, fast and complex paths, RAW, repeated keys).testNormalizeEndpointWithEqualSignInParameteris back to the 4.22.1 value.mvn clean install -DskipTests.Not changed here: an encoded plus (
%2B) still reaches the endpoint as a space, as in every release since at least 4.10. That is older behaviour, tracked in CAMEL-25190.🤖 Generated with Claude Code