CAMEL-25188: camel-util - URISupport.normalizeUri gives the same uri when normalizing a normalized uri (space, = and # in values, + in keys)

core/camel-util/src/main/java/org/apache/camel/util/URISupport.java

@@ -900,6 +900,12 @@ private static String buildReorderingParameters(String scheme, String path, Stri
         // createQueryString()/URLEncoder, which would needlessly percent-encode characters that are
         // legal unescaped in a URI query, such as ':' (eg host:port) or '/' (eg produces=application/json)
         query = buildSafeQueryString(keys, parameters);
+        if (query.indexOf('%') != -1) {
+            // a key or value needed a percent escape (such as = or # in a value), and a uri with % is normalized by
+            // the complex normalizer, which form-encodes the whole query; encode the same way here so normalizing a
+            // normalized uri gives the same uri (the fast parser only takes uris without %, so all % come from here)
+            query = createQueryString(keys, parameters, true);
+        }
         return buildUri(scheme, path, query);
     }
 
@@ -939,7 +945,7 @@ private static String buildSafeQueryString(String[] sortedKeys, Map<String, Obje
     }
 
     private static void appendSafeQueryStringParameter(String key, String value, StringBuilder sb) {
-        sb.append(key);
+        sb.append(safeEncodeQueryPart(key));
         if (value == null) {
             return;
         }
@@ -950,15 +956,21 @@ private static void appendSafeQueryStringParameter(String key, String value, Str
             // need to replace % with %25 to avoid losing "%" when decoding
             sb.append(URIScanner.replacePercent(value));
         } else {
-            // '&' and '=' are structurally significant in Camel's key=value&key=value query syntax
-            // and must stay escaped inside a value even though they are otherwise legal, unescaped
-            // 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");
-            sb.append(encoded);
+            sb.append(safeEncodeQueryPart(value));
         }
     }
 
+    private static String safeEncodeQueryPart(String text) {
+        // '&' and '=' are structurally significant in Camel's key=value&key=value query syntax
+        // and must stay escaped inside a key or value even though they are otherwise legal, unescaped
+        // 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(text).replace("&", "%26").replace("=", "%3D");
+        // a space as +, as the complex normalizer (createQueryString) writes it; the fast parser only takes uris
+        // without %, so %20 here is always a space
+        return encoded.replace("%20", "+");
+    }
+
     private static String buildUri(String scheme, String path, String query) {
         // must include :// to do a correct URI all components can work with
         int len = scheme.length() + 3 + path.length();

core/camel-util/src/test/java/org/apache/camel/util/URISupportTest.java

@@ -148,6 +148,67 @@ public void testNormalizeHttpEndpointURLEncodedParameter() throws Exception {
         assertEquals("http://www.google.com?q=S%C3%B8ren+Hansen", out);
     }
 
+    @Test
+    public void testNormalizeSpaceTheSameInEverySpelling() throws Exception {
+        // CAMEL-25188: the fast and the complex normalizer both write a space in a value as +
+        assertThat(URISupport.normalizeUri("log:foo?marker=a+b")).isEqualTo("log://foo?marker=a+b");
+        assertThat(URISupport.normalizeUri("log:foo?marker=a%20b")).isEqualTo("log://foo?marker=a+b");
+        assertThat(URISupport.normalizeUri("log:foo?marker=a b")).isEqualTo("log://foo?marker=a+b");
+        assertThat(URISupport.normalizeUri("log:foo?showAll=true&marker=a+b"))
+                .isEqualTo("log://foo?marker=a+b&showAll=true");
+        assertThat(URISupport.normalizeUri("log:foo?marker=a++b")).isEqualTo("log://foo?marker=a++b");
+    }
+
+    @Test
+    public void testNormalizeValueWithPercentEscapeFormEncodesTheQuery() throws Exception {
+        // CAMEL-25188: a value with = or # needs a percent escape, and a uri with % is normalized by the complex
+        // normalizer, so the fast normalizer form-encodes the whole query the same way, whatever the key order
+        assertThat(URISupport.normalizeUri("log:foo?secretKey=abc/def=="))
+                .isEqualTo("log://foo?secretKey=abc%2Fdef%3D%3D");
+        assertThat(URISupport.normalizeUri("log:foo?marker=a#b/c")).isEqualTo("log://foo?marker=a%23b%2Fc");
+        assertThat(URISupport.normalizeUri("jms:queue:foo?foo=bar&selector=somekey='somevalue'"))
+                .isEqualTo("jms://queue:foo?foo=bar&selector=somekey%3D%27somevalue%27");
+        // without a percent escape the query stays readable
+        assertThat(URISupport.normalizeUri("foo:bar?produces=application/json&host=http://h"))
+                .isEqualTo("foo://bar?host=http://h&produces=application/json");
+    }
+
+    @Test
+    public void testNormalizeSpaceInKey() throws Exception {
+        // CAMEL-25188: a + in a key is a space, written back as + as in a value
+        assertThat(URISupport.normalizeUri("log:foo?a+b=1")).isEqualTo("log://foo?a+b=1");
+        assertThat(URISupport.normalizeUri("log:foo?a+b=1&c=2")).isEqualTo("log://foo?a+b=1&c=2");
+    }
+
+    @Test
+    public void testNormalizeTwiceGivesTheSameUri() throws Exception {
+        // CAMEL-25188: a normalized uri normalizes to itself, so endpoint keys and lookups agree
+        String[] uris = {
+                "http://localhost:8080/foo?a=1&b=2",
+                "http://localhost:8080/foo?b=hello world&a=1",
+                "http://localhost:8080/foo?q=a+b",
+                "http://localhost:8080/foo?q=a%20b",
+                "http://localhost:8080/foo?q=a+b&x=1",
+                "ftp://user@host.com:21/dir?password=se+cret&binary=true",
+                "ftp://user@host.com:21/dir?password=RAW(se+cret)&binary=true",
+                "log:foo?level=INFO&showAll=true",
+                "timer:tick?period=1s&delay=2s",
+                "direct:start?b=\u00f8&a=1",
+                "file:target/in?include=.*\\.txt&noop=true",
+                "http://h/p?x=a&x=b&y=1",
+                "mock:a?b=x+y+z&a=1",
+                "jms:queue:foo?selector=somekey='somevalue'&foo=bar",
+                "log:foo?secretKey=abc/def==",
+                "log:foo?webhookExternalUrl=https://example.com/hook?token=abc",
+                "log:foo?marker=a#b/c",
+                "log:foo?a+b=1",
+                "foo:bar?host=http://h&produces=application/json&x=a=b" };
+        for (String uri : uris) {
+            String once = URISupport.normalizeUri(uri);
+            assertThat(URISupport.normalizeUri(once)).as("normalizing %s twice", uri).isEqualTo(once);
+        }
+    }
+
     @Test
     public void testParseParametersURLEncodedValue() throws Exception {
         String out = URISupport.normalizeUri("http://www.google.com?q=S%C3%B8ren%20Hansen");
@@ -205,9 +266,9 @@ public void testCreateURIWithQueryEmptyQueryString() throws Exception {
     public void testNormalizeEndpointWithEqualSignInParameter() throws Exception {
         String out = URISupport.normalizeUri("jms:queue:foo?selector=somekey='somevalue'&foo=bar");
         assertNotNull(out);
-        // Camel will safe encode the URI - '=' stays escaped as it is structurally significant in
-        // the query syntax, but the single quotes (legal unescaped in a URI query) are left as-is
-        assertEquals("jms://queue:foo?foo=bar&selector=somekey%3D'somevalue'", out);
+        // Camel will safe encode the URI - a value with '=' needs a percent escape, so the query is form-encoded
+        // as the complex normalizer does, and normalizing the uri again gives the same uri
+        assertEquals("jms://queue:foo?foo=bar&selector=somekey%3D%27somevalue%27", out);
     }
 
     @Test

docs/user-manual/modules/ROOT/pages/camel-4x-upgrade-guide-4_23.adoc

@@ -953,8 +953,10 @@ reusing the cached one, with no error or log warning.
 Normalization is now always order-independent. As part of the fix, the encoding applied when rebuilding the
 query string is also less aggressive: characters that are legal unescaped in a URI query per RFC 3986
 (`:`, `/`, `,`, `'`, etc. - for example a MIME type such as `produces=application/json`, or a `host:port`
-value) are no longer percent-encoded, while `&` and `=` remain escaped inside a value since they are
-structurally significant in Camel's own `key=value&key=value` query syntax.
+value) are no longer percent-encoded, as long as no key or value in the query needs a percent escape.
+A value with `=` or `#` (for example `secretKey=abc/def==`) needs one, and then the whole query is
+form-encoded as in earlier releases, the same way as an endpoint URI that is already percent-encoded, so
+normalizing a normalized URI gives the same URI.
 
 Code that asserts a literal, fully-normalized endpoint URI string containing one of those characters in a
 query value may need to update the expected string to the (now consistently) unencoded form.