This is an automated email from the ASF dual-hosted git repository. davsclaus pushed a commit to branch fix/CAMEL-24992 in repository https://gitbox.apache.org/repos/asf/camel.git
commit 76c6ce9e9e62c03e5e7a5b79939f1441d7ca1e02 Author: Claus Ibsen <[email protected]> AuthorDate: Sat Oct 10 10:09:08 2026 +0200 CAMEL-24992: camel-jbang - the validator reports a rest-openapi call with no header for a path parameter A rest-openapi call to an operation whose path holds a parameter ({sku}) takes the value from a header (or variable) of that name. With none set, the request goes out with {sku} in the path and the service answers 404; the runtime names the parameter after the route has run (CAMEL-24986). In the local-model benchmark the model kept the sku in an exchange property instead, and the app sent 900 to 3100 requests to a literal /stock/{sku}/reserve in 3 of 7 runs. The validator reads the specification beside the route (OpenApiVerbs), finds the operation's path parameters, and walks the steps before the call for a setHeader, setHeaders, setVariable or setVariables of that name, or an endpoint option. It stays quiet when the route consumes from an endpoint that can carry any header, or a step before the call does something it cannot follow (a bean, a processor, a call to another endpoint), and when the specification or the operation is not known. Over the benchmark's routes it reported none of the 45 reference solutions and 49 of the 414 model versions, none of which set the header. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01STT6whBgK1AqsSsUKrnE8m --- .../jbang/core/commands/ai/OpenApiPathParams.java | 294 +++++++++++++++++++++ .../dsl/jbang/core/commands/ai/OpenApiVerbs.java | 2 +- .../jbang/core/commands/ai/SourceValidator.java | 2 + .../core/commands/ai/OpenApiPathParamsTest.java | 121 +++++++++ 4 files changed, 418 insertions(+), 1 deletion(-) diff --git a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/OpenApiPathParams.java b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/OpenApiPathParams.java new file mode 100644 index 000000000000..f02cd3c5a680 --- /dev/null +++ b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/OpenApiPathParams.java @@ -0,0 +1,294 @@ +/* + * 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.jbang.core.commands.ai; + +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import org.yaml.snakeyaml.LoaderOptions; +import org.yaml.snakeyaml.Yaml; +import org.yaml.snakeyaml.constructor.SafeConstructor; + +/** + * A {@code rest-openapi} call to an operation whose path holds a parameter, with no step before it that sets a header + * (or variable) of that name: the request goes out with {@code {sku}} in the path and the service answers 404 + * (CAMEL-24992). The runtime names the parameter once the route has run (CAMEL-24986); this says it before, while the + * route is written. + * <p/> + * The check stays quiet when it cannot prove the header is missing: the route consumes from an endpoint that can carry + * any header (only timer, scheduler, cron, quartz and file cannot), or a step before the call does something the walk + * cannot follow (a bean, a processor, a script, a call to another endpoint). + */ +final class OpenApiPathParams { + + /** Consumers whose messages carry no headers of the application's choosing. */ + private static final Set<String> PLAIN_CONSUMERS = Set.of("timer", "scheduler", "cron", "quartz", "file"); + /** Steps that set no header but the ones the walk reads (setHeader, setHeaders, setVariable, setVariables). */ + private static final Set<String> SAFE_STEPS = Set.of( + "log", "setBody", "transform", "setProperty", "removeProperty", "removeProperties", "removeHeader", + "removeHeaders", "removeVariable", "convertBodyTo", "marshal", "unmarshal", "split", "filter", "choice", + "loop", "delay", "validate", "stop", "aggregate"); + /** Endpoints a call to sets no header of the application's choosing. */ + private static final Set<String> SAFE_ENDPOINTS = Set.of("log", "mock", "rest-openapi"); + private static final Pattern PATH_PARAMETER = Pattern.compile("\\{([^}/]+)}"); + + private OpenApiPathParams() { + } + + /** The problems, each "Line N: message". */ + static List<String> validate(String content, Path directory) { + List<String> answer = new ArrayList<>(); + if (content == null || directory == null || !content.contains("rest-openapi")) { + return answer; + } + Object doc; + try { + doc = new Yaml(new SafeConstructor(new LoaderOptions())).load(content); + } catch (Exception e) { + return answer; + } + if (!(doc instanceof List<?> items)) { + return answer; + } + try { + walk(items, directory, answer, callLines(content)); + } catch (RuntimeException e) { + // a shape the walk does not know: a check must never fail the validation + } + return answer; + } + + private static void walk(List<?> items, Path directory, List<String> answer, List<Integer> callLines) { + int[] call = { 0 }; + for (Object item : items) { + Map<?, ?> from = null; + if (item instanceof Map<?, ?> m && m.get("route") instanceof Map<?, ?> route + && route.get("from") instanceof Map<?, ?> f) { + from = f; + } else if (item instanceof Map<?, ?> m && m.get("from") instanceof Map<?, ?> f) { + from = f; + } + if (from == null) { + continue; + } + Walk walk = new Walk(directory, answer, callLines, call); + walk.followed = PLAIN_CONSUMERS.contains(scheme(String.valueOf(from.get("uri")))); + walk.steps(from.get("steps")); + } + } + + /** The lines that call rest-openapi, in document order. */ + private static List<Integer> callLines(String content) { + List<Integer> lines = new ArrayList<>(); + String[] all = content.split("\n", -1); + for (int i = 0; i < all.length; i++) { + String line = all[i]; + if (line.contains("rest-openapi") && (line.contains("uri") || line.contains("to:")) + && !line.stripLeading().startsWith("#")) { + lines.add(i + 1); + } + } + return lines; + } + + private static String scheme(String uri) { + int colon = uri.indexOf(':'); + return colon > 0 ? uri.substring(0, colon) : uri; + } + + private static final class Walk { + private final Path directory; + private final List<String> answer; + private final List<Integer> callLines; + private final int[] call; + private final Set<String> set = new LinkedHashSet<>(); + private final Set<String> properties = new LinkedHashSet<>(); + /** Whether every header so far is known: false after a step the walk cannot follow. */ + private boolean followed; + + Walk(Path directory, List<String> answer, List<Integer> callLines, int[] call) { + this.directory = directory; + this.answer = answer; + this.callLines = callLines; + this.call = call; + } + + void steps(Object steps) { + if (!(steps instanceof List<?> list)) { + return; + } + for (Object step : list) { + if (!(step instanceof Map<?, ?> m) || m.size() != 1) { + followed = false; + continue; + } + Map.Entry<?, ?> e = m.entrySet().iterator().next(); + step(String.valueOf(e.getKey()), e.getValue()); + } + } + + private void step(String eip, Object body) { + switch (eip) { + case "setHeader" -> name(body, set); + case "setVariable" -> name(body, set); + case "setProperty" -> name(body, properties); + case "setHeaders" -> names(body, "headers"); + case "setVariables" -> names(body, "variables"); + case "to", "toD" -> to(eip, body); + default -> { + if (!SAFE_STEPS.contains(eip)) { + followed = false; + } + } + } + nested(body); + } + + private static void name(Object body, Set<String> into) { + if (body instanceof Map<?, ?> m && m.get("name") != null) { + into.add(String.valueOf(m.get("name"))); + } + } + + private void names(Object body, String key) { + if (body instanceof Map<?, ?> m && m.get(key) instanceof List<?> list) { + list.forEach(h -> name(h, set)); + } + } + + /** The steps nested in an EIP: its steps, and those of its when, otherwise, doCatch and doFinally. */ + private void nested(Object body) { + if (!(body instanceof Map<?, ?> m)) { + return; + } + for (Map.Entry<?, ?> e : m.entrySet()) { + if ("steps".equals(e.getKey())) { + steps(e.getValue()); + } else if (e.getValue() instanceof List<?> list) { + list.forEach(this::nested); + } else if (e.getValue() instanceof Map<?, ?>) { + nested(e.getValue()); + } + } + } + + private void to(String eip, Object body) { + String uri = body instanceof Map<?, ?> m ? String.valueOf(m.get("uri")) : String.valueOf(body); + Map<?, ?> parameters = body instanceof Map<?, ?> m && m.get("parameters") instanceof Map<?, ?> p ? p : Map.of(); + String scheme = scheme(uri); + if (!"rest-openapi".equals(scheme)) { + if (!SAFE_ENDPOINTS.contains(scheme)) { + followed = false; + } + return; + } + int line = call[0] < callLines.size() ? callLines.get(call[0]) : 0; + call[0]++; + if (!followed || "toD".equals(eip)) { + return; + } + Set<String> options = new LinkedHashSet<>(); + parameters.keySet().forEach(k -> options.add(String.valueOf(k))); + // rest-openapi:spec#operationId, or plain rest-openapi with the specificationUri and operationId options + String path = uri.length() > scheme.length() ? uri.substring(scheme.length() + 1) : ""; + int q = path.indexOf('?'); + if (q >= 0) { + for (String pair : path.substring(q + 1).split("&")) { + options.add(pair.split("=", 2)[0]); + } + path = path.substring(0, q); + } + String spec + = parameters.get("specificationUri") != null ? String.valueOf(parameters.get("specificationUri")) : null; + String operation = parameters.get("operationId") != null ? String.valueOf(parameters.get("operationId")) : null; + int hash = path.indexOf('#'); + if (hash >= 0) { + spec = spec != null ? spec : path.substring(0, hash); + operation = operation != null ? operation : path.substring(hash + 1); + } else if (operation == null && !path.isEmpty()) { + operation = path; + } + String template = operationPath(spec, operation); + if (template == null) { + return; // the specification or the operation is not known: nothing can be said + } + Matcher m = PATH_PARAMETER.matcher(template); + while (m.find()) { + String param = m.group(1); + if (set.contains(param) || options.contains(param)) { + continue; + } + answer.add("Line " + line + ": the operation " + operation + " (" + template + ") needs the path parameter " + + param + ", and no step before this call sets a header " + param + + ": the request goes out with {" + param + "} in the path and the service answers 404" + + (properties.contains(param) + ? " (the exchange property " + param + " is not used for the path)" + : "") + + "; add - setHeader: {name: " + param + ", expression: {simple: ...}} before the call"); + } + } + + /** The path of the operation in the specification beside the route, or null when either is not known. */ + private String operationPath(String spec, String operation) { + if (operation == null || operation.isBlank()) { + return null; + } + String file = spec == null || spec.isBlank() ? "openapi.json" : spec; + for (String prefix : List.of("file:", "classpath:")) { + if (file.startsWith(prefix)) { + file = file.substring(prefix.length()); + } + } + if (file.contains("://") || file.contains("{{")) { + return null; + } + Path p = directory.resolve(file); + if (!Files.isRegularFile(p)) { + p = directory.resolve("src/main/resources").resolve(file); + } + if (!Files.isRegularFile(p)) { + return null; + } + Map<?, ?> paths; + try { + paths = OpenApiVerbs.paths(Files.readString(p)); + } catch (Exception e) { + return null; + } + if (paths == null) { + return null; + } + for (Map.Entry<?, ?> e : paths.entrySet()) { + if (e.getValue() instanceof Map<?, ?> operations) { + for (Object o : operations.values()) { + if (o instanceof Map<?, ?> details && operation.equals(String.valueOf(details.get("operationId")))) { + return String.valueOf(e.getKey()); + } + } + } + } + return null; + } + } +} diff --git a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/OpenApiVerbs.java b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/OpenApiVerbs.java index 382900b4e724..5880bdf6428d 100644 --- a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/OpenApiVerbs.java +++ b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/OpenApiVerbs.java @@ -87,7 +87,7 @@ public final class OpenApiVerbs { return answer; } - private static Map<?, ?> paths(String text) { + static Map<?, ?> paths(String text) { try { Object document; if (text.stripLeading().startsWith("{")) { diff --git a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/SourceValidator.java b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/SourceValidator.java index 98e3b451555a..a652bcbd290d 100644 --- a/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/SourceValidator.java +++ b/dsl/camel-jbang/camel-jbang-core/src/main/java/org/apache/camel/dsl/jbang/core/commands/ai/SourceValidator.java @@ -139,6 +139,8 @@ public final class SourceValidator { msgs.addAll(validateYamlBeanRefs(content, declarations, catalog)); msgs.addAll(validateResourceRefs(content, directory)); msgs.addAll(GroovyImportChecks.validateYamlGroovyImports(content, null, declarations.javaClasses())); + // a rest-openapi call with no header for a path parameter of its operation (CAMEL-24992) + msgs.addAll(OpenApiPathParams.validate(content, directory)); if (checkConsumers) { // a direct: or seda: endpoint no route of the application consumes (CAMEL-24955) msgs.addAll(EndpointConsumerChecks.validateYamlConsumers(content, directory, fileName, catalog)); diff --git a/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/OpenApiPathParamsTest.java b/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/OpenApiPathParamsTest.java new file mode 100644 index 000000000000..febd25464f51 --- /dev/null +++ b/dsl/camel-jbang/camel-jbang-core/src/test/java/org/apache/camel/dsl/jbang/core/commands/ai/OpenApiPathParamsTest.java @@ -0,0 +1,121 @@ +/* + * 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.jbang.core.commands.ai; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.List; + +import org.apache.camel.catalog.DefaultCamelCatalog; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import static org.assertj.core.api.Assertions.assertThat; + +/** + * CAMEL-24992: a rest-openapi call with no header for a path parameter of its operation is reported before the route + * runs, and the check stays quiet where it cannot prove the header is missing. + */ +class OpenApiPathParamsTest { + + private static final String SPEC = """ + {"openapi": "3.0.3", "info": {"title": "Stock", "version": "1"}, + "paths": {"/stock/{sku}/reserve": {"post": {"operationId": "reserveStock", + "parameters": [{"name": "sku", "in": "path", "required": true, "schema": {"type": "string"}}]}}, + "/stock": {"get": {"operationId": "listStock"}}}} + """; + + @TempDir + Path dir; + + @BeforeEach + void spec() throws IOException { + Files.writeString(dir.resolve("stock-api.json"), SPEC); + } + + private static String route(String from, String beforeCall, String call) { + return "- route:\n id: reserve-order-lines\n from:\n uri: " + from + "\n steps:\n" + + " - split:\n expression:\n jsonpath: '$.lines'\n steps:\n" + + beforeCall + + " - to:\n uri: " + call + "\n"; + } + + private static final String SET_PROPERTY = " - setProperty:\n name: sku\n" + + " expression:\n simple: ${body.sku}\n"; + private static final String SET_HEADER = " - setHeader:\n name: sku\n" + + " expression:\n simple: ${body.sku}\n"; + private static final String CALL = "rest-openapi:stock-api.json#reserveStock"; + + @Test + void aPathParameterWithNoHeaderIsReportedAtTheCall() { + // the benchmark's model kept the sku in an exchange property, not a header (r7, run q8) + List<String> msgs = OpenApiPathParams.validate(route("file:orders", SET_PROPERTY, CALL), dir); + + assertThat(msgs).singleElement().asString() + .startsWith("Line 15: the operation reserveStock (/stock/{sku}/reserve) needs the path parameter sku,") + .contains("the request goes out with {sku} in the path") + .contains("(the exchange property sku is not used for the path)") + .contains("setHeader"); + } + + @Test + void theOperationGivenAsOptionsIsCheckedToo() { + String route + = "- route:\n from:\n uri: timer:tick\n steps:\n - to:\n uri: rest-openapi\n" + + " parameters:\n specificationUri: stock-api.json\n" + + " operationId: reserveStock\n"; + + assertThat(OpenApiPathParams.validate(route, dir)).singleElement().asString() + .startsWith("Line 6: the operation reserveStock (/stock/{sku}/reserve) needs the path parameter sku"); + } + + @Test + void aHeaderAVariableOrAnOptionOfTheNameIsEnough() { + assertThat(OpenApiPathParams.validate(route("file:orders", SET_HEADER, CALL), dir)).isEmpty(); + assertThat(OpenApiPathParams.validate(route("file:orders", + " - setVariable:\n name: sku\n constant: x\n", CALL), dir)) + .isEmpty(); + assertThat(OpenApiPathParams.validate(route("timer:tick", "", CALL + "?sku=CAMEL-MUG"), dir)).isEmpty(); + // an operation without a path parameter + assertThat(OpenApiPathParams.validate(route("timer:tick", "", "rest-openapi:stock-api.json#listStock"), dir)) + .isEmpty(); + } + + @Test + void quietWhereTheHeaderCouldComeFromSomewhereElse() { + // a caller of a direct route can set the header + assertThat(OpenApiPathParams.validate(route("direct:reserve", SET_PROPERTY, CALL), dir)).isEmpty(); + // a bean the walk cannot follow + assertThat(OpenApiPathParams.validate(route("file:orders", + " - bean:\n ref: skuSetter\n", CALL), dir)).isEmpty(); + // an operation the specification does not have, or no specification beside the route + assertThat(OpenApiPathParams.validate(route("file:orders", "", "rest-openapi:stock-api.json#nope"), dir)) + .isEmpty(); + assertThat(OpenApiPathParams.validate(route("file:orders", "", "rest-openapi:other.json#reserveStock"), dir)) + .isEmpty(); + } + + @Test + void theValidatorOfAWriteReportsIt() { + List<String> msgs = SourceValidator.validate("openapi-client.camel.yaml", route("file:orders", SET_PROPERTY, CALL), + new DefaultCamelCatalog(), null, dir); + + assertThat(msgs).anyMatch(m -> m.contains("needs the path parameter sku")); + } +}
