gnodet-bot commented on code in PR #27832:
URL: https://github.com/apache/camel/pull/27832#discussion_r4271718776


##########
components/camel-kamelet/src/main/java/org/apache/camel/component/kamelet/Kamelet.java:
##########
@@ -184,6 +207,268 @@ public static void 
extractKameletEnvironmentVariables(Map<String, Object> proper
         }
     }
 
+    /**
+     * Reads the original advanced options map stored under a single internal 
key.
+     */
+    private static Map<String, Object> extractAdvancedOptions(Map<String, 
Object> parameters, String key) {
+        Map<String, Object> options = new LinkedHashMap<>();
+        if (parameters == null) {
+            return options;
+        }
+
+        Object value = parameters.get(key);
+        if (value instanceof Map<?, ?> map) {
+            for (Map.Entry<?, ?> entry : map.entrySet()) {
+                if (entry.getKey() != null) {
+                    options.put(entry.getKey().toString(), entry.getValue());
+                }
+            }
+        }
+
+        return options;
+    }
+
+    /**
+     * Drops Kamelet template properties from the advanced-option candidates, 
treating camelCase and kebab-case names as
+     * aliases the same way {@code DefaultModel} does for route templates.
+     */
+    private static Map<String, Object> excludeTemplateParameters(
+            Map<String, Object> candidates, RouteTemplateDefinition template) {
+        if (candidates == null || candidates.isEmpty()) {
+            return new LinkedHashMap<>();
+        }
+
+        Set<String> templateNames = templateParameterNames(template);
+        if (templateNames.isEmpty()) {
+            return candidates;
+        }
+
+        Map<String, Object> filtered = new LinkedHashMap<>();
+        for (Map.Entry<String, Object> entry : candidates.entrySet()) {
+            String name = entry.getKey();
+            if (name != null && !isTemplateParameter(name, templateNames)) {
+                filtered.put(name, entry.getValue());
+            }
+        }
+        return filtered;
+    }
+
+    private static Set<String> templateParameterNames(RouteTemplateDefinition 
template) {
+        Set<String> names = new HashSet<>();
+        if (template == null || template.getTemplateParameters() == null) {
+            return names;
+        }
+        for (RouteTemplateParameterDefinition param : 
template.getTemplateParameters()) {
+            if (param.getName() != null) {
+                names.add(param.getName());
+                names.add(StringHelper.dashToCamelCase(param.getName()));
+                names.add(StringHelper.camelCaseToDash(param.getName()));
+            }
+        }
+        return names;
+    }
+
+    private static boolean isTemplateParameter(String name, Set<String> 
templateNames) {
+        return templateNames.contains(name)
+                || templateNames.contains(StringHelper.dashToCamelCase(name))
+                || templateNames.contains(StringHelper.camelCaseToDash(name));
+    }
+
+    /**
+     * Preserves the user-supplied option name. When both camelCase and 
kebab-case forms are present, the kebab-case
+     * form is kept so the same option is not appended twice.
+     */
+    private static Map<String, Object> normalizeParameterNames(Map<String, 
Object> parameters) {
+        Map<String, Object> normalized = new LinkedHashMap<>();
+        Map<String, String> seenOriginal = new HashMap<>();
+        Map<String, Boolean> seenIsCanonical = new HashMap<>();
+
+        if (parameters == null) {
+            return normalized;
+        }
+
+        for (Map.Entry<String, Object> entry : parameters.entrySet()) {
+            String originalName = entry.getKey();
+            String name = StringHelper.camelCaseToDash(originalName);
+            boolean isCanonical = originalName.equals(name);
+
+            String existingOriginal = seenOriginal.get(name);
+            if (existingOriginal == null) {
+                seenOriginal.put(name, originalName);
+                seenIsCanonical.put(name, isCanonical);
+                normalized.put(originalName, entry.getValue());
+            } else {
+                boolean existingIsCanonical = seenIsCanonical.get(name);
+                if (isCanonical && !existingIsCanonical) {
+                    normalized.remove(existingOriginal);
+                    seenOriginal.put(name, originalName);
+                    seenIsCanonical.put(name, isCanonical);
+                    normalized.put(originalName, entry.getValue());
+                }
+            }
+        }
+
+        return normalized;
+    }
+
+    /**
+     * Appends query parameters without parsing or re-encoding the endpoint 
URI. This preserves property placeholders
+     * and RAW expressions that may appear in the URI.
+     */
+    private static String appendParametersToURI(String uri, Map<String, 
Object> parameters) {
+        if (parameters == null || parameters.isEmpty()) {
+            return uri;
+        }
+
+        String result = uri;
+
+        // First remove all parameters that will be overridden.
+        for (String name : parameters.keySet()) {
+            result = removeQueryParameter(result, name);
+        }
+
+        // Then append the options to the preserved URI.
+        StringBuilder sb = new StringBuilder(result);
+        char sep = result.indexOf('?') < 0 ? '?' : '&';
+
+        for (Map.Entry<String, Object> entry : parameters.entrySet()) {
+            sb.append(sep).append(entry.getKey());
+            if (entry.getValue() != null) {
+                
sb.append('=').append(wrapRawIfNeeded(String.valueOf(entry.getValue())));
+            }
+            sep = '&';
+        }
+
+        return sb.toString();
+    }
+
+    /**
+     * Re-wraps values that contain URI-sensitive characters. RAW wrappers may 
already have been stripped by
+     * {@link URISupport#resolveRawParameterValues(Map)} before options reach 
this method.
+     */
+    private static String wrapRawIfNeeded(String value) {
+        if (value == null || value.isEmpty() || isRawToken(value)) {
+            return value;
+        }
+        if (value.contains("&") || value.contains("=") || value.contains("?")
+                || value.contains("+") || value.contains("%")) {
+            if (value.contains(")&") && !value.startsWith("{") && 
!value.endsWith("}")
+                    && !value.contains("}&")) {
+                return "RAW{" + value + "}";
+            }
+            if (value.contains(")&")) {
+                // fits in neither RAW(...) nor RAW{...}; keep the value out 
of the message, it may be a secret
+                throw new IllegalArgumentException(
+                        "The value of advanced options cannot contain \")&\" 
when it also starts with '{',"
+                                                   + " ends with '}' or 
contains \"}&\"");
+            }
+            return "RAW(" + value + ")";
+        }
+
+        return value;
+    }
+
+    private static boolean isRawToken(String value) {
+        return value != null
+                && ((value.startsWith("RAW(") && value.endsWith(")"))
+                        || (value.startsWith("RAW{") && value.endsWith("}")));
+    }
+
+    /**
+     * Removes a query parameter by name, treating camelCase and kebab-case 
names as aliases. Returns the original URI
+     * unchanged when the parameter is not present, so encoded values in the 
query are not rewritten.
+     */
+    static String removeQueryParameter(String uri, String parameterName) {
+        int queryIndex = uri.indexOf('?');
+        if (queryIndex < 0) {
+            return uri;
+        }
+
+        String query = uri.substring(queryIndex + 1);
+
+        Map<String, Object> params;
+        try {
+            params = URISupport.parseQuery(query);
+        } catch (URISyntaxException e) {
+            throw new IllegalArgumentException("Invalid URI query: " + query, 
e);
+        }
+
+        String normalizedName = StringHelper.camelCaseToDash(parameterName);
+        boolean present = params.keySet().removeIf(
+                name -> 
StringHelper.camelCaseToDash(name).equals(normalizedName));
+        if (!present) {
+            return uri;
+        }
+
+        String base = uri.substring(0, queryIndex);
+        return params.isEmpty() ? base : base + "?" + 
URISupport.createQueryString(params, false);

Review Comment:
   ⚠️ **Encoding bug:** `URISupport.parseQuery()` URL-decodes parameter values, 
and `createQueryString(params, false)` writes them back decoded. Any 
`%xx`-encoded characters in the remaining (non-removed) parameters are silently 
corrupted.
   
   The method Javadoc says encoded values are preserved — consider using 
string-based filtering instead of parse→re-encode to actually guarantee that.



##########
components/camel-kamelet/src/main/java/org/apache/camel/component/kamelet/Kamelet.java:
##########
@@ -184,6 +207,268 @@ public static void 
extractKameletEnvironmentVariables(Map<String, Object> proper
         }
     }
 
+    /**
+     * Reads the original advanced options map stored under a single internal 
key.
+     */
+    private static Map<String, Object> extractAdvancedOptions(Map<String, 
Object> parameters, String key) {
+        Map<String, Object> options = new LinkedHashMap<>();
+        if (parameters == null) {
+            return options;
+        }
+
+        Object value = parameters.get(key);
+        if (value instanceof Map<?, ?> map) {
+            for (Map.Entry<?, ?> entry : map.entrySet()) {
+                if (entry.getKey() != null) {
+                    options.put(entry.getKey().toString(), entry.getValue());
+                }
+            }
+        }
+
+        return options;
+    }
+
+    /**
+     * Drops Kamelet template properties from the advanced-option candidates, 
treating camelCase and kebab-case names as
+     * aliases the same way {@code DefaultModel} does for route templates.
+     */
+    private static Map<String, Object> excludeTemplateParameters(
+            Map<String, Object> candidates, RouteTemplateDefinition template) {
+        if (candidates == null || candidates.isEmpty()) {
+            return new LinkedHashMap<>();
+        }
+
+        Set<String> templateNames = templateParameterNames(template);
+        if (templateNames.isEmpty()) {
+            return candidates;
+        }
+
+        Map<String, Object> filtered = new LinkedHashMap<>();
+        for (Map.Entry<String, Object> entry : candidates.entrySet()) {
+            String name = entry.getKey();
+            if (name != null && !isTemplateParameter(name, templateNames)) {
+                filtered.put(name, entry.getValue());
+            }
+        }
+        return filtered;
+    }
+
+    private static Set<String> templateParameterNames(RouteTemplateDefinition 
template) {
+        Set<String> names = new HashSet<>();
+        if (template == null || template.getTemplateParameters() == null) {
+            return names;
+        }
+        for (RouteTemplateParameterDefinition param : 
template.getTemplateParameters()) {
+            if (param.getName() != null) {
+                names.add(param.getName());
+                names.add(StringHelper.dashToCamelCase(param.getName()));
+                names.add(StringHelper.camelCaseToDash(param.getName()));
+            }
+        }
+        return names;
+    }
+
+    private static boolean isTemplateParameter(String name, Set<String> 
templateNames) {
+        return templateNames.contains(name)
+                || templateNames.contains(StringHelper.dashToCamelCase(name))
+                || templateNames.contains(StringHelper.camelCaseToDash(name));
+    }
+
+    /**
+     * Preserves the user-supplied option name. When both camelCase and 
kebab-case forms are present, the kebab-case
+     * form is kept so the same option is not appended twice.
+     */
+    private static Map<String, Object> normalizeParameterNames(Map<String, 
Object> parameters) {
+        Map<String, Object> normalized = new LinkedHashMap<>();
+        Map<String, String> seenOriginal = new HashMap<>();
+        Map<String, Boolean> seenIsCanonical = new HashMap<>();
+
+        if (parameters == null) {
+            return normalized;
+        }
+
+        for (Map.Entry<String, Object> entry : parameters.entrySet()) {
+            String originalName = entry.getKey();
+            String name = StringHelper.camelCaseToDash(originalName);
+            boolean isCanonical = originalName.equals(name);
+
+            String existingOriginal = seenOriginal.get(name);
+            if (existingOriginal == null) {
+                seenOriginal.put(name, originalName);
+                seenIsCanonical.put(name, isCanonical);
+                normalized.put(originalName, entry.getValue());
+            } else {
+                boolean existingIsCanonical = seenIsCanonical.get(name);
+                if (isCanonical && !existingIsCanonical) {
+                    normalized.remove(existingOriginal);
+                    seenOriginal.put(name, originalName);
+                    seenIsCanonical.put(name, isCanonical);
+                    normalized.put(originalName, entry.getValue());
+                }
+            }
+        }
+
+        return normalized;
+    }
+
+    /**
+     * Appends query parameters without parsing or re-encoding the endpoint 
URI. This preserves property placeholders
+     * and RAW expressions that may appear in the URI.
+     */
+    private static String appendParametersToURI(String uri, Map<String, 
Object> parameters) {
+        if (parameters == null || parameters.isEmpty()) {
+            return uri;
+        }
+
+        String result = uri;
+
+        // First remove all parameters that will be overridden.
+        for (String name : parameters.keySet()) {
+            result = removeQueryParameter(result, name);
+        }
+
+        // Then append the options to the preserved URI.
+        StringBuilder sb = new StringBuilder(result);
+        char sep = result.indexOf('?') < 0 ? '?' : '&';
+
+        for (Map.Entry<String, Object> entry : parameters.entrySet()) {
+            sb.append(sep).append(entry.getKey());
+            if (entry.getValue() != null) {
+                
sb.append('=').append(wrapRawIfNeeded(String.valueOf(entry.getValue())));
+            }
+            sep = '&';
+        }
+
+        return sb.toString();
+    }
+
+    /**
+     * Re-wraps values that contain URI-sensitive characters. RAW wrappers may 
already have been stripped by
+     * {@link URISupport#resolveRawParameterValues(Map)} before options reach 
this method.
+     */
+    private static String wrapRawIfNeeded(String value) {

Review Comment:
   ⚠️ **Potential double-encoding hazard:** The Javadoc here acknowledges that 
`resolveRawParameterValues` may have already stripped RAW wrappers (and in 
doing so converted `%25` → `%`). If such a pre-processed value reaches 
`wrapRawIfNeeded`, it gets re-wrapped with incomplete percent-encoding. Worth 
auditing the call chain — if callers never pre-process with 
`resolveRawParameterValues`, remove the Javadoc warning; if they do, add a test 
for the `%25` round-trip.



-- 
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]

Reply via email to