davsclaus commented on code in PR #26456: URL: https://github.com/apache/camel/pull/26456#discussion_r4025400666
########## components/camel-kamelet/src/test/java/org/apache/camel/component/kamelet/KameletToDUrlEncodingTest.java: ########## @@ -0,0 +1,100 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.camel.component.kamelet; + +import org.apache.camel.RoutesBuilder; +import org.apache.camel.builder.RouteBuilder; +import org.apache.camel.component.mock.MockEndpoint; +import org.apache.camel.test.junit6.CamelTestSupport; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * Verifies that kamelet parameters containing URL-special characters are preserved intact when the kamelet is invoked + * via {@code toD}, matching the behaviour of the static {@code to} DSL (CAMEL-24747). + * + * <p> + * Prior to the fix, {@code SendDynamicProcessor.prepareRecipient()} created a + * {@link org.apache.camel.support.NormalizedUri} whose normalized (URL-encoded) form was then passed to + * {@code doGetEndpoint} as if it were the raw URI. Components that declare {@code useRawUri()=true} (such as + * {@link KameletComponent}) therefore received the encoded form ({@code http%3A%2F%2F…}) instead of the original value. + */ +class KameletToDUrlEncodingTest extends CamelTestSupport { + + /** + * A parameter value that contains URL-special characters: {@code %}, {@code +}, {@code ?} and {@code =}. These must + * survive the round-trip through {@code toD} unmodified. + */ + private static final String PARAM_WITH_SPECIAL_CHARS = "http://example.com?key=abc%+def";; + + @Test + void toDPreservesSpecialCharsLikeTo() { + // Use toD with a literal URI containing URL-special characters in a parameter value. + // The result should be identical to what the static `to` DSL produces. + String resultTo = template.requestBody("direct:via-to", (Object) null, String.class); + String resultToD = template.requestBody("direct:via-tod", (Object) null, String.class); + + assertThat(resultTo) + .as("to: parameter value must not be URL-encoded") + .isEqualTo(PARAM_WITH_SPECIAL_CHARS); + + assertThat(resultToD) + .as("toD: parameter value must equal what to produces (CAMEL-24747)") + .isEqualTo(resultTo); + } + + /** + * Negative test: components that do NOT declare {@code useRawUri()=true} continue to receive the normalised URI via + * {@code toD}, proving that the fix is surgically scoped to {@code useRawUri} components only. + */ + @Test + void toDNonRawUriComponentIsUnaffected() throws Exception { + MockEndpoint mock = getMockEndpoint("mock:result"); + mock.expectedMessageCount(1); + mock.expectedBodiesReceived("hello"); + + template.sendBody("direct:via-tod-mock", "hello"); + + MockEndpoint.assertIsSatisfied(context); + } + + @Override + protected RoutesBuilder createRouteBuilder() { + return new RouteBuilder() { + @Override + public void configure() { + routeTemplate("echo-uri") + .templateParameter("uri") + .from("kamelet:source") + .setBody().constant("{{uri}}"); + + // static `to` — baseline: known to pass the raw URI correctly + from("direct:via-to") + .to("kamelet:echo-uri?uri=" + PARAM_WITH_SPECIAL_CHARS); + + // dynamic `toD` — was broken before the fix (CAMEL-24747) + from("direct:via-tod") + .toD("kamelet:echo-uri?uri=" + PARAM_WITH_SPECIAL_CHARS); Review Comment: This test passes without the core fix. `from("direct:via-to").to("kamelet:echo-uri?uri=…")` resolves its endpoint at context start and registers it under the normalized key; at runtime `SendDynamicProcessor` calls `getExistingEndpoint()` → `hasEndpoint(NormalizedUri)` first (`SendDynamicProcessor.java:191`), gets a cache hit on that same key, and reuses the endpoint `to` created with the raw URI — `doGetEndpoint` is never reached for this route. Verified locally: with `core/` reverted to the merge-base (jar has no `getRawUri`), this class is 2/2 green. The same value in a `toD`-only route fails with `but was: "http%3A%2F%2Fexample.com%3Fkey%3Dabc%25+def"` and passes with your fix. Suggested change (three spots, so no suggestion block): add a second constant with a distinct value, e.g. ```java private static final String PARAM_WITH_SPECIAL_CHARS_TOD = "http://example.com?key=xyz%+def";; ``` use it here — `.toD("kamelet:echo-uri?uri=" + PARAM_WITH_SPECIAL_CHARS_TOD)` — and assert `resultToD` against `PARAM_WITH_SPECIAL_CHARS_TOD` instead of `isEqualTo(resultTo)`. The `to` route can stay as the baseline showing the static path is unaffected. (`toDNonRawUriComponentIsUnaffected` also passes either way — fine as a smoke test, but it does not demonstrate the scoping its Javadoc describes.) ########## core/camel-support/src/main/java/org/apache/camel/support/NormalizedUri.java: ########## @@ -41,12 +47,23 @@ private NormalizedUri(String value) { */ public static NormalizedUri newNormalizedUri(String uri, boolean normalized) { if (normalized) { - return new NormalizedUri(uri); + return new NormalizedUri(uri, uri); Review Comment: Nit: on this path the caller says the URI is already normalized, so the raw form is simply unknown — but `getRawUri()` will return the normalized string, contradicting its Javadoc ("the raw (un-normalized) URI"). Behaviour is identical either way since `doGetEndpoint` already falls back with `explicitRawUri != null ? explicitRawUri : uri`, so storing `null` here (and noting in the getter Javadoc that it may be `null` when the raw form is not known) keeps the contract honest. ```suggestion return new NormalizedUri(uri, null); ``` -- 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]
