This is an automated email from the ASF dual-hosted git repository. davsclaus pushed a commit to branch fix/CAMEL-24917 in repository https://gitbox.apache.org/repos/asf/camel.git
commit 3332ca665af1d2fee70af683f971bcf933b7c835 Author: Claus Ibsen <[email protected]> AuthorDate: Wed Sep 23 08:18:52 2026 +0200 CAMEL-24917: camel-yaml-dsl-validator - a to: uri that holds an expression says to use toD: to: http://host/stock/${header.sku} sends the placeholder as text, url-encoded, because the endpoint of a to: is resolved once when the route starts. Every call then fails, and the benchmark runs show hundreds of redeliveries before anyone reads the log. The validator now says to write toD:, so camel validate and the camel-jbang tools catch it before the route runs. Only the path is checked, never the options after the ?, and the components that read their path as a script or a statement (language, the sql family, xslt, xquery) are left alone - the user manual's sql:SELECT ... :#${body.itemId} on a plain to: is correct. The to-eip page shows this very mistake on purpose ("This snippet is not valid code"), so a block can now be marked with // yaml-validator: skip to stay out of the validation and out of the samples. That also takes the invalid route out of eip-samples.json, which camel_catalog_sample hands to an agent as an example. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01Bp3538HRBPMQkb5ta9xRaj --- .../org/apache/camel/catalog/docs/to-eip.adoc | 1 + .../src/main/docs/modules/eips/pages/to-eip.adoc | 1 + .../dsl/jbang/core/commands/ai/eip-samples.json | 4 - .../dsl/yaml/validator/GenerateDocSamplesMojo.java | 13 +- .../camel/dsl/yaml/validator/YamlValidator.java | 82 ++++++++++ .../dsl/yaml/validator/EipDocExamplesTest.java | 6 + .../validator/YamlValidatorDynamicUriTest.java | 167 +++++++++++++++++++++ 7 files changed, 269 insertions(+), 5 deletions(-) diff --git a/catalog/camel-catalog/src/generated/resources/org/apache/camel/catalog/docs/to-eip.adoc b/catalog/camel-catalog/src/generated/resources/org/apache/camel/catalog/docs/to-eip.adoc index 13f04283fb71..3869fed116a7 100644 --- a/catalog/camel-catalog/src/generated/resources/org/apache/camel/catalog/docs/to-eip.adoc +++ b/catalog/camel-catalog/src/generated/resources/org/apache/camel/catalog/docs/to-eip.adoc @@ -126,6 +126,7 @@ XML:: YAML:: + +// yaml-validator: skip [source,yaml] ---- - route: diff --git a/core/camel-core-engine/src/main/docs/modules/eips/pages/to-eip.adoc b/core/camel-core-engine/src/main/docs/modules/eips/pages/to-eip.adoc index 13f04283fb71..3869fed116a7 100644 --- a/core/camel-core-engine/src/main/docs/modules/eips/pages/to-eip.adoc +++ b/core/camel-core-engine/src/main/docs/modules/eips/pages/to-eip.adoc @@ -126,6 +126,7 @@ XML:: YAML:: + +// yaml-validator: skip [source,yaml] ---- - route: diff --git a/dsl/camel-jbang/camel-jbang-core/src/generated/resources/org/apache/camel/dsl/jbang/core/commands/ai/eip-samples.json b/dsl/camel-jbang/camel-jbang-core/src/generated/resources/org/apache/camel/dsl/jbang/core/commands/ai/eip-samples.json index d8303bc79bc2..529cbbc4044d 100644 --- a/dsl/camel-jbang/camel-jbang-core/src/generated/resources/org/apache/camel/dsl/jbang/core/commands/ai/eip-samples.json +++ b/dsl/camel-jbang/camel-jbang-core/src/generated/resources/org/apache/camel/dsl/jbang/core/commands/ai/eip-samples.json @@ -1458,10 +1458,6 @@ "source": "to-eip.adoc", "yaml": "- route:\n from:\n uri: file:messages\/foo\n steps:\n - to:\n uri: jms:queue:foo\n" }, - { - "source": "to-eip.adoc", - "yaml": "- route:\n from:\n uri: file:messages\/foo\n steps:\n - to:\n uri: \"freemarker:\/\/templateHome\/${body.templateName}.ftl\"\n - to:\n uri: jms:queue:foo\n" - }, { "source": "to-eip.adoc", "yaml": "- route:\n from:\n uri: file:messages\/foo\n steps:\n - toD:\n uri: \"freemarker:\/\/templateHome\/${body.templateName}.ftl\"\n - to:\n uri: jms:queue:foo\n" diff --git a/dsl/camel-yaml-dsl/camel-yaml-dsl-validator-maven-plugin/src/main/java/org/apache/camel/dsl/yaml/validator/GenerateDocSamplesMojo.java b/dsl/camel-yaml-dsl/camel-yaml-dsl-validator-maven-plugin/src/main/java/org/apache/camel/dsl/yaml/validator/GenerateDocSamplesMojo.java index fef7f9458803..f2247c8b54de 100644 --- a/dsl/camel-yaml-dsl/camel-yaml-dsl-validator-maven-plugin/src/main/java/org/apache/camel/dsl/yaml/validator/GenerateDocSamplesMojo.java +++ b/dsl/camel-yaml-dsl/camel-yaml-dsl-validator-maven-plugin/src/main/java/org/apache/camel/dsl/yaml/validator/GenerateDocSamplesMojo.java @@ -63,6 +63,11 @@ public class GenerateDocSamplesMojo extends AbstractMojo { static final Pattern YAML_BLOCK = Pattern.compile("\\[source,yaml\\]\\s*\\n----\\n(.*?)\\n----", Pattern.DOTALL); private static final Pattern CALLOUT = Pattern.compile("[ \\t]*#[ \\t]*<\\d+>[ \\t]*$", Pattern.MULTILINE); + /** + * An AsciiDoc comment before a block keeps it out of the samples and out of the validation: a page that shows what + * to avoid (to-eip shows a to: with an expression in its uri to say why toD exists) must not become a sample. + */ + static final String SKIP_MARKER = "// yaml-validator: skip"; @Parameter(property = "project", required = true, readonly = true) protected MavenProject project; @@ -272,13 +277,19 @@ public class GenerateDocSamplesMojo extends AbstractMojo { Matcher m = YAML_BLOCK.matcher(doc); while (m.find()) { String yaml = CALLOUT.matcher(m.group(1)).replaceAll("").stripTrailing() + "\n"; - if (yaml.stripLeading().startsWith("- ")) { + if (yaml.stripLeading().startsWith("- ") && !markedToSkip(doc, m.start())) { answer.add(yaml); } } return answer; } + /** Whether the block at this offset carries {@link #SKIP_MARKER} in the lines before it. */ + static boolean markedToSkip(String doc, int blockStart) { + int from = Math.max(0, blockStart - 200); + return doc.substring(from, blockStart).contains(SKIP_MARKER); + } + private static boolean validate(YamlValidator validator, File page, String yaml, List<String> failures) throws Exception { List<Error> errors = validator.validate(yaml); diff --git a/dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/YamlValidator.java b/dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/YamlValidator.java index 2b47f748c2dc..fadf98b0b7a3 100644 --- a/dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/YamlValidator.java +++ b/dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/main/java/org/apache/camel/dsl/yaml/validator/YamlValidator.java @@ -563,6 +563,7 @@ public class YamlValidator { } if (errors.isEmpty()) { checkSimpleSyntaxInScripts(target, new NodePath(PathType.JSON_POINTER), errors); + checkDynamicUri(target, new NodePath(PathType.JSON_POINTER), errors); } if (canonical) { checkOneOfCardinality(target, new NodePath(PathType.JSON_POINTER), errors); @@ -668,6 +669,87 @@ public class YamlValidator { } } + /** + * to: http://host/stock/${header.sku}: the endpoint of a to: is resolved once when the route starts, so an + * expression in its path is never evaluated - it is sent as the text it is, url-encoded. That is what toD: is for + * (CAMEL-24917). + * <p/> + * Only the path is checked, never the options after the {@code ?}: an option such as the file component's + * {@code fileName=${date:now:yyyyMMdd}} is evaluated by the producer and is correct on a plain to:. + */ + void checkDynamicUri(JsonNode node, NodePath path, List<Error> errors) { + if (node == null) { + return; + } + if (node.isArray()) { + for (int i = 0; i < node.size(); i++) { + checkDynamicUri(node.get(i), path.append(i), errors); + } + return; + } + if (!node.isObject()) { + return; + } + var fields = node.fieldNames(); + while (fields.hasNext()) { + String name = fields.next(); + JsonNode value = node.get(name); + if ("to".equals(name)) { + String uri = null; + NodePath at = path.append(name); + if (value.isTextual()) { + uri = value.asText(); + } else if (value.isObject() && value.has("uri") && value.get("uri").isTextual()) { + uri = value.get("uri").asText(); + at = at.append("uri"); + } + String expression = expressionInPath(uri); + if (expression != null) { + errors.add(Error.builder() + .keyword("type") + .instanceLocation(at) + .messageKey("type") + .format(new MessageFormat("{0}")) + .arguments("to: the uri holds an expression (" + expression + ") but the endpoint of a to:" + + " is fixed when the route starts, so it is sent as text: write toD: to build" + + " the uri for each message") + .build()); + } + } + checkDynamicUri(value, path.append(name), errors); + } + } + + /** + * The first simple expression in the path of the uri (what comes before the options), or null when there is none. + */ + private static String expressionInPath(String uri) { + if (uri == null) { + return null; + } + int scheme = uri.indexOf(':'); + if (scheme > 0 && EVALUATED_PATH.contains(uri.substring(0, scheme))) { + return null; + } + String head = uri.indexOf('?') > 0 ? uri.substring(0, uri.indexOf('?')) : uri; + int start = head.indexOf("${"); + if (start < 0) { + return null; + } + if (start >= 2 && head.startsWith(":#", start - 2)) { + return null; // :#${...} is a parameter the component binds per message, not part of the address + } + int end = head.indexOf('}', start); + return end > 0 ? head.substring(start, end + 1) : head.substring(start); + } + + /** + * Components that read their path as a script, a statement or a template name and evaluate it for each message, + * where an expression in the path is what the component is for. + */ + private static final Set<String> EVALUATED_PATH = Set.of("language", "sql", "sql-stored", "elsql", "jdbc", + "spring-jdbc", "mybatis", "xquery", "xslt"); + /** Adds an error for every expression node in the tree that has neither expression: nor a language key. */ void checkRequiredExpressions(JsonNode node, NodePath path, List<Error> errors) { if (node == null) { diff --git a/dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/test/java/org/apache/camel/dsl/yaml/validator/EipDocExamplesTest.java b/dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/test/java/org/apache/camel/dsl/yaml/validator/EipDocExamplesTest.java index 248ea4363421..e1f111242002 100644 --- a/dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/test/java/org/apache/camel/dsl/yaml/validator/EipDocExamplesTest.java +++ b/dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/test/java/org/apache/camel/dsl/yaml/validator/EipDocExamplesTest.java @@ -55,6 +55,9 @@ class EipDocExamplesTest { */ private static final Map<String, String> EXAMPLES_SKIPPED = Map.of("yaml-dsl", "myStep:"); + /** The same marker the doc-samples plugin honours: a block that shows what to avoid is not an example. */ + private static final String SKIP_MARKER = "// yaml-validator: skip"; + private static CamelCatalog catalog; private static YamlValidator validator; @@ -141,6 +144,9 @@ class EipDocExamplesTest { if (skipped != null && yaml.contains(skipped)) { continue; } + if (doc.substring(Math.max(0, m.start() - 200), m.start()).contains(SKIP_MARKER)) { + continue; + } examples++; List<Error> errors = validator.validate(yaml); if (!errors.isEmpty()) { diff --git a/dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/test/java/org/apache/camel/dsl/yaml/validator/YamlValidatorDynamicUriTest.java b/dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/test/java/org/apache/camel/dsl/yaml/validator/YamlValidatorDynamicUriTest.java new file mode 100644 index 000000000000..16c722329460 --- /dev/null +++ b/dsl/camel-yaml-dsl/camel-yaml-dsl-validator/src/test/java/org/apache/camel/dsl/yaml/validator/YamlValidatorDynamicUriTest.java @@ -0,0 +1,167 @@ +/* + * 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.dsl.yaml.validator; + +import java.util.List; + +import com.networknt.schema.Error; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * CAMEL-24917: to: with an expression in the uri path sends the text as it stands, url-encoded, and every call fails at + * runtime. The validator says to write toD: instead. + */ +public class YamlValidatorDynamicUriTest { + + private static YamlValidator classic; + private static YamlValidator canonical; + + @BeforeAll + public static void setup() throws Exception { + classic = new YamlValidator(); + classic.init(); + canonical = new YamlValidator(true); + canonical.init(); + } + + @Test + public void testToWithAnExpressionInThePath() { + String yaml = """ + - route: + from: + uri: direct:start + steps: + - to: + uri: "http://localhost:8080/stock/${header.sku}"; + """; + assertHint(yaml, "${header.sku}", "write toD:"); + } + + @Test + public void testToInItsShortForm() { + // canonical mode has its own word about the short form, so the hint is the classic mode's + String yaml = """ + - route: + from: + uri: direct:start + steps: + - to: "http://localhost:8080/stock/${header.sku}"; + """; + List<Error> errors = validate(classic, yaml); + assertThat(errors).anyMatch(e -> e.getMessage().contains("${header.sku}")) + .anyMatch(e -> e.getMessage().contains("write toD:")); + } + + @Test + public void testToDIsWhatToWrite() { + String yaml = """ + - route: + from: + uri: direct:start + steps: + - toD: + uri: "http://localhost:8080/stock/${header.sku}"; + """; + assertNoHint(yaml); + } + + @Test + public void testAnOptionThatTheProducerEvaluatesIsFine() { + // the file component evaluates fileName per message, so this is correct on a plain to: + String yaml = """ + - route: + from: + uri: direct:start + steps: + - to: + uri: "file:out?fileName=${date:now:yyyyMMdd}.txt" + """; + assertNoHint(yaml); + } + + @Test + public void testASqlParameterIsFine() { + // the sql component binds :#${...} per message; this is what the user manual shows + String yaml = """ + - route: + from: + uri: direct:start + steps: + - to: + uri: "sql:SELECT * FROM inventory WHERE id = :#${body.itemId}" + """; + assertNoHint(yaml); + } + + @Test + public void testALanguageScriptIsFine() { + // the language component's path is the script it evaluates + String yaml = """ + - route: + from: + uri: direct:start + steps: + - to: + uri: "language:simple:Hello ${body}" + """; + assertNoHint(yaml); + } + + @Test + public void testAPropertyPlaceholderIsFine() { + String yaml = """ + - route: + from: + uri: direct:start + steps: + - to: + uri: "http://{{stock.host}}/stock"; + """; + assertNoHint(yaml); + } + + private void assertHint(String yaml, String... expectedInMessage) { + for (YamlValidator validator : List.of(classic, canonical)) { + String mode = validator.isCanonical() ? "canonical" : "classic"; + List<Error> errors = validate(validator, yaml); + assertThat(errors).as("%s mode must report the expression:\n%s", mode, yaml).isNotEmpty(); + for (String expected : expectedInMessage) { + assertThat(errors).as("%s mode must say '%s'", mode, expected) + .anyMatch(e -> e.getMessage().contains(expected)); + } + } + } + + private void assertNoHint(String yaml) { + for (YamlValidator validator : List.of(classic, canonical)) { + assertThat(validate(validator, yaml)) + .as("%s mode must accept:\n%s", validator.isCanonical() ? "canonical" : "classic", yaml) + .noneMatch(e -> e.getMessage().contains("write toD:")); + } + } + + private List<Error> validate(YamlValidator validator, String yaml) { + try { + return validator.validate(yaml); + } catch (Exception e) { + throw new AssertionError("Failed to validate:\n" + yaml, e); + } + } +}
