allthingssecurity opened a new pull request, #27468: URL: https://github.com/apache/camel/pull/27468
# Description [CAMEL-25345](https://issues.apache.org/jira/browse/CAMEL-25345) Reported and analysed by @oscerd. CAMEL-25188 made the query side of `URISupport.normalizeUri` idempotent, 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 `#bean` reference is enough): | URI | first pass (main) | second pass (main) | |---|---|---| | `sftp://[email protected]@host/in?password=pa=ss` | `sftp://[email protected]@host/in?password=pa%3Dss` | `sftp://me%40example.com@host/in?password=pa%3Dss` | | `sql:select+*+from+t+where+id=:#id?dataSource=#ds` | `sql://select+*+from+t+where+id=:#id?dataSource=%23ds` | `sql://select+*+from+t+where+id=:%23id?dataSource=%23ds` | The 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, and `hasEndpoint` returns `null`. 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 with `createQueryString` that 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 that `getEndpoint(endpoint.getEndpointUri())` already looked up. The path and the parameters that a component gets are the same, because `DefaultComponent.createEndpoint` encodes `#` before parsing and the URI decoding gives back `@` (a component with `useRawUri()` gets the URI before normalization anyway). One thing does change, for the better: with more than one `@` in the user info, `java.net.URI` cannot parse `[email protected]@host` as a server authority, so on main a component that reads the user and host from the URI got `null` for both. With a camel-ftp endpoint, `sftp://[email protected]@host/in?password=pa=ss` had `host=null` and `username=null` on main, and has `host=host` and `[email protected]` with this change (the values main gave the duplicate endpoint). A URI whose query needs no `%` (`sftp://me@exa mple.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 `#bean` reference or an `=` in a value now takes the complex normalizer, which is slower than the fast path plus the `createQueryString` re-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=#ds` 0.27 to 0.66 microseconds per `normalizeUri`, `jms:queue:orders?connectionFactory=#cf&concurrentConsumers=5` 0.63 to 1.37 microseconds; URIs without such values are unchanged. `normalizeUri` runs 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 (`%2B` in a query value decoded to a space, left for Camel 5) is not affected: a URI with `%2B` already 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: - The model reproduces the outputs in the table, and main is not idempotent for every short path with `#` or two `@` combined with every value with `=` or `#` (checked exhaustively). - From the three properties of the normalizers that the dispatch needs, it proves for every URI that the fix is idempotent, that it equals main whenever the fast result has no `%`, 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: - the fix never makes an idempotent URI non-idempotent; - the 475 URIs that stay non-idempotent all have a key starting with `?` or an empty key (a separate `prepareQuery` quirk that main has too); - the endpoint values (path and parameters as `DefaultComponent` parses 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` (`getEndpoint` and `hasEndpoint` with `endpoint.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`, `SplitPropertiesFileIssueTest` and `BackgroundTaskTest`, 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 `sftp` example, and its last sentence no longer says "unencoded form" (the doc nit in the ticket). # Target - [x] I checked that the commit is targeting the correct branch (Camel 4 uses the `main` branch) # Tracking - [x] If this is a large change, bug fix, or code improvement, I checked there is a [JIRA issue](https://issues.apache.org/jira/browse/CAMEL) filed for the change (usually before you start working on it). # Apache Camel coding standards and style - [x] I checked that each commit in the pull request has a meaningful subject line and body. - [ ] I have run `mvn clean install -DskipTests` locally from root folder and I have committed all auto-generated changes. (I built and tested `core/camel-util` and `core/camel-core`, including the formatter and import-sort plugins. No generated files change. I did not run the full root build.) # AI-assisted contributions - [x] If this PR includes AI-generated code, commits have proper co-authorship attribution (e.g., `Co-authored-by` trailers) 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-By` trailer. _Claude Code on behalf of allthingssecurity_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
