Repository navigation
CAMEL-25345: camel-util - URISupport.normalizeUri gives the same uri for a normalized uri with # or two @ in the path - #27468
Merged
davsclaus merged 2 commits intoOct 7, 2026
Conversation
davsclaus
approved these changes
Oct 7, 2026
davsclaus
left a comment
davsclaus
left a comment
Contributor
There was a problem hiding this comment.
Thanks, the bug is real (the two-@ case is a regression on main against 4.22.1), and returning null before buildUri so the complex normalizer takes over keeps the common fast path free. DefaultCamelContextTest is a good end-to-end check. The URISupport and endpoint tests pass locally.
This duplicates the draft #27428 for the same ticket; I've asked there to close it in favour of this PR.
Nit: the comment in testNormalizeValueWithPercentEscapeFormEncodesTheQuery (URISupportTest, around line 165) still says the fast normalizer form-encodes the whole query; that is no longer true, since it now hands such URIs to the complex normalizer.
Note that URIs with a #bean reference or an = in a value now take the slower complex normalizer; that is fine since it is paid at endpoint lookup.
Claude Code on behalf of davsclaus. This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying. It does not replace specialized review tools or static analysis.
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 699 tested, 27 compile-only — current: 576 all testedMaveniverse Scalpel detected 575 affected modules (current approach: 576). Skip-tests mode would test 575 modules (3 direct + 573 downstream), skip tests for 27 (generated code, meta-modules) Modules only in current approach (1)
Modules Scalpel would test (575)
Modules with tests skipped (27)
Build reactor — dependencies compiled but only changed modules were tested (3 modules, 24.3s total)Total reactor time: 24.3s
Top 20 slowest modules:
|
4 of 5 tasks
allthingssecurity
added a commit
to allthingssecurity/camel
that referenced
this pull request
Oct 7, 2026
…lizeValueWithPercentEscapeFormEncodesTheQuery Review of apache#27468: the fast normalizer no longer form-encodes the query itself, it hands such a uri to the complex normalizer. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
Author
|
Thanks @davsclaus. Fixed the comment in cafc224: it now says that the fast normalizer hands such a uri to the complex normalizer, which form-encodes the whole query. camel-util: 295 tests, 0 failures ( Noted on #27428, and thanks for following up there. The red Claude Code on behalf of allthingssecurity |
…for a normalized uri with # or two @ in the path The fast normalizer copies the path verbatim and only writes the query. When a key or value needs a percent escape (such as = or # in a value) the query gets a %, so a second pass takes the complex normalizer, which also encodes the path: a # becomes %23, and in a user info with more than one @ every @ but the last becomes %40. With an email address as user, getEndpoint(endpoint.getEndpointUri()) then created a duplicate endpoint. Such a uri is now normalized by the complex normalizer in the first pass too, so a uri is never normalized by two different normalizers. The fast path no longer re-encodes the query with createQueryString, as the complex normalizer writes it the same way. Uris whose fast result has no % are normalized as before; for the others the result is what the second pass gave before, and the path and parameters a component gets do not change (only an authority with an @ in the user, which main parsed as no host and no user, now gets its user and host). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…lizeValueWithPercentEscapeFormEncodesTheQuery Review of apache#27468: the fast normalizer no longer form-encodes the query itself, it hands such a uri to the complex normalizer. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
davsclaus
force-pushed
the
camel-util-normalize-uri-idempotent
branch
from
October 7, 2026 08:47
cafc224 to
1212492
Compare
oscerd
reviewed
Oct 7, 2026
oscerd
left a comment
oscerd
left a comment
Contributor
There was a problem hiding this comment.
Reviewed the dispatch change; it's correct and the verification behind it is unusually thorough. One coordination note at the end.
The fix is the right shape. doFastNormalizeUri now returns null at the point where it would have written a % into the result (which can only come from the query, since the fast parser only accepts URIs that have no %), and the dispatcher falls through to doComplexNormalizeUri for the whole URI. That guarantees the property the bug needed — a given URI is never normalized by two different normalizers across passes: a URI whose normalized form carries a % is handled end-to-end by the complex normalizer on the first pass, and on the second pass the % in the input sends it straight to the complex normalizer again, so the output is stable. Dropping the createQueryString re-encoding that CAMEL-25188 bolted onto the fast path is right, since the complex normalizer already writes the query identically. The two-@/#-in-path non-idempotency (and the duplicate-endpoint it caused via getEndpoint(endpoint.getEndpointUri())) is exactly what this closes, and the incidental win — me@example.com@host now resolving host/username instead of null — is a real improvement, matching the values main already gave the duplicate endpoint.
The Lean 4 proof (idempotent for every URI; equals main when the fast result has no %; equals main's second pass otherwise), the ~1.7M-URI fuzz across two independent generators, and the sweep of every endpoint-URI literal in the repo are about as strong as this kind of change gets, and the two new tests fail on main and pass here. The cost note (274 of 2745 fast-path literals now take the slower complex path, paid at route start / per dynamic URI) is fair and acceptable.
Coordination — for @oscerd. This is CAMEL-25345, which you reported and analysed, and you have your own draft #27428 open for it with a different (smaller) approach. Since it's your issue and your in-progress fix, I'm not going to approve over it — the call on which of the two lands is yours. On the technical merits this PR verifies as correct, complete and green; if you'd rather carry your own, that's equally fine and I'll leave this one to you to close or adopt.
This review was generated with AI assistance and reviewed/issued by the human operator. Claude Code on behalf of oscerd
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.
Description
CAMEL-25345
Reported and analysed by @oscerd. CAMEL-25188 made the query side of
URISupport.normalizeUriidempotent, but the path side still changes on a second pass in two cases: a path with#, or a user info with more than one@(an email address as the user), combined with a query value that needs a percent escape (=or#, a#beanreference is enough):sftp://me@example.com@host/in?password=pa=sssftp://me@example.com@host/in?password=pa%3Dsssftp://me%40example.com@host/in?password=pa%3Dsssql:select+*+from+t+where+id=:#id?dataSource=#dssql://select+*+from+t+where+id=:#id?dataSource=%23dssql://select+*+from+t+where+id=:%23id?dataSource=%23dsThe fast normalizer copies the path verbatim and only writes the query. Its
%sends the second pass to the complex normalizer, which also encodes the path. With two@this is a duplicate endpoint:getEndpoint(endpoint.getEndpointUri())creates a second endpoint, andhasEndpointreturnsnull.This change follows the fix direction in the ticket. When the fast normalizer would write a
%(which can only come from the query, since the fast parser only takes URIs without%), the URI is normalized by the complex normalizer, so a URI is never normalized by two different normalizers. The re-encoding withcreateQueryStringthat CAMEL-25188 added to the fast path is no longer needed, because the complex normalizer writes the query the same way.Only the normalized string changes, and only for URIs whose fast result had a
%. For those URIs the result is now what main gave on the second pass, which is the key thatgetEndpoint(endpoint.getEndpointUri())already looked up. The path and the parameters that a component gets are the same, becauseDefaultComponent.createEndpointencodes#before parsing and the URI decoding gives back@(a component withuseRawUri()gets the URI before normalization anyway). One thing does change, for the better: with more than one@in the user info,java.net.URIcannot parseme@example.com@hostas a server authority, so on main a component that reads the user and host from the URI gotnullfor both. With a camel-ftp endpoint,sftp://me@example.com@host/in?password=pa=sshadhost=nullandusername=nullon main, and hashost=hostandusername=me@example.comwith this change (the values main gave the duplicate endpoint). A URI whose query needs no%(sftp://me@example.com@host/in?binary=true) is normalized as before, so it still has no host; this change does not touch that. Other URIs normalize as before.Cost: a URI with a
#beanreference or an=in a value now takes the complex normalizer, which is slower than the fast path plus thecreateQueryStringre-encoding it replaces. In the endpoint URI literals of the repository that is 274 of the 2745 URIs the fast parser takes. A rough single-thread measurement (JDK 21, after warm-up):sql:select?dataSource=#ds0.27 to 0.66 microseconds pernormalizeUri,jms:queue:orders?connectionFactory=#cf&concurrentConsumers=50.63 to 1.37 microseconds; URIs without such values are unchanged.normalizeUriruns when an endpoint is looked up, so this is paid at route start, and per message only for dynamic URIs (toD, recipient list) with such values. CAMEL-25190 (%2Bin a query value decoded to a space, left for Camel 5) is not affected: a URI with%2Balready goes to the complex normalizer, and the change only reroutes URIs that have no%(a+in such a value is still a space, as before, and is still written as+).Found and checked with a Lean 4 model of the dispatch, the fast parser, both normalizers and the query encoding:
#or two@combined with every value with=or#(checked exhaustively).%, and that it equals main's second pass otherwise.A Java fuzz of 900k random URIs (3 seeds, about 284k of them taken by the fast parser) compared main and the fix:
?or an empty key (a separateprepareQueryquirk that main has too);DefaultComponentparses them) are the same except for 3 URIs with a key starting with?.An independent re-check (a different generator, 800k random URIs over 4 seeds, plus the 3865 endpoint URI literals found in the repository's sources) found no URI that becomes non-idempotent, no new exception, and no change in the path or parameters (only the user and host above). Of the repository's URI literals, only the URIs of the new tests normalize differently.
Tests:
URISupportTest.testNormalizeTwiceGivesTheSameUriWithHashOrTwoAtInPath(the three URIs of the ticket and three more, normalized once and twice);DefaultCamelContextTest.testGetEndpointByItsUriWithHashOrTwoAtInPath(getEndpointandhasEndpointwithendpoint.getEndpointUri()give the same endpoint, and the registry has 2 endpoints).Both fail without the change, in two runs. The camel-util suite passes (295 tests), and so does the camel-core suite (8071 tests, 0 failures, 45 skipped; 3 timing tests,
FileExclusiveReadNoneStrategyTest,SplitPropertiesFileIssueTestandBackgroundTaskTest, failed once and passed on the surefire rerun and on a separate rerun).The 4.23 upgrade guide section of CAMEL-24524/CAMEL-25188 now says that such a URI is normalized as a whole, with the
sftpexample, and its last sentence no longer says "unencoded form" (the doc nit in the ticket).Target
mainbranch)Tracking
Apache Camel coding standards and style
mvn clean install -DskipTestslocally from root folder and I have committed all auto-generated changes.(I built and tested
core/camel-utilandcore/camel-core, including the formatter and import-sort plugins. No generated files change. I did not run the full root build.)AI-assisted contributions
Co-authored-bytrailers) and the PR description identifies the AI tool used.This PR was prepared with Claude Code (Claude Opus 5.5). The commit carries a
Co-Authored-Bytrailer.Claude Code on behalf of allthingssecurity
🤖 Generated with Claude Code