oscerd commented on code in PR #27131:
URL: https://github.com/apache/camel/pull/27131#discussion_r4155305130
##########
core/camel-util/src/main/java/org/apache/camel/util/URISupport.java:
##########
@@ -955,6 +955,9 @@ private static void appendSafeQueryStringParameter(String
key, String value, Str
// characters in a URI query per RFC 3986 -
UnsafeUriCharactersEncoder does not escape them
// as it is also used outside of this query-value context
String encoded =
UnsafeUriCharactersEncoder.encode(value).replace("&", "%26").replace("=",
"%3D");
+ // a space as +, as the complex normalizer (createQueryString)
writes it, so normalizing a normalized
+ // uri gives the same uri; the fast parser only takes uris without
%, so %20 here is always a space
+ encoded = encoded.replace("%20", "+");
Review Comment:
This fixes the space case, but the wider claim in the comment ("normalizing
a normalized uri gives the same uri") doesn't hold yet. Any `%` escape the fast
path writes sends the second normalization to the complex path, because the
fast parser rejects `%`. The complex path then form-encodes `/`, `:`, `'` and
`?`, which the fast path leaves alone. `%20` was the most common of those
escapes. `%3D` (for `=`) and `%23` (for `#`) remain.
The URI from the existing `testNormalizeEndpointWithEqualSignInParameter`
shows it:
```
jms:queue:foo?selector=somekey='somevalue'&foo=bar
once : jms://queue:foo?foo=bar&selector=somekey%3D'somevalue'
twice: jms://queue:foo?foo=bar&selector=somekey%3D%27somevalue%27
```
Adding that URI to `testNormalizeTwiceGivesTheSameUri` makes it fail on this
branch. Base64 padding (`secretKey=abc/def==`) and a URL with a query as a
value (`webhookExternalUrl=https://example.com/hook?token=abc`) behave the same
way. 4.22.1 gave the same string both times for all of them.
With a `DefaultCamelContext` (4.23.0-SNAPSHOT core jars, camel-util from
this branch), `getEndpoint(ep.getEndpointUri())` creates a second endpoint for
`log:foo?marker=a='b'` and `log:foo?marker=abc/def==`, and
`hasEndpoint(ep.getEndpointUri())` returns null. That's the duplicate-endpoint
symptom CAMEL-24524 set out to fix.
This comes from CAMEL-24524, not from this PR, so a follow-up Jira is fine.
It will ship in 4.23.0 unless it's fixed first, though. If this PR stays
limited to spaces, maybe reword the comment so it only claims that a space now
normalizes the same on both paths.
--
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]
