This is an automated email from the ASF dual-hosted git repository. davsclaus pushed a commit to branch fix/CAMEL-24986-warn in repository https://gitbox.apache.org/repos/asf/camel.git
commit 218efe19c8acf622d1676810f271ae88fa611ee6 Author: Claus Ibsen <[email protected]> AuthorDate: Thu Sep 24 12:44:19 2026 +0200 CAMEL-24986: a REST producer says which path parameter has no value The request goes out with the placeholder still in the path, and the service answers 404 for a path that holds a {name}, with nothing saying why: HTTP operation failed invoking http://localhost:8080/api/stock/%7Bsku%7D/reserve with statusCode: 404 resolvePlaceholders reads a header and falls back to an exchange variable, and leaves the placeholder as it is when neither has a value. It now says so: The path parameter {sku} of /api/stock/{sku}/reserve has no value: set the header sku, or an exchange variable of that name, before the call. The request is sent with {sku} in the path, which the service is unlikely to answer. This is logged once per parameter. It warns rather than fails, because both shapes are deliberate: a partly resolved template keeps the rest of its placeholders ("Backward compatibility: if one of the params is resolved") and a template where nothing resolved is not an error either. The first attempt at this threw instead, broke RestProducerPathTest in camel-core and was reverted in 7aec6a1a2b9b; the new test asserts the request is still sent, so that cannot happen again unnoticed. Once per parameter, not per message: a route that is missing a value is missing it for every message, and the run this came from produced 2580 of these failures in a night. Only a name in the braces counts, so a uri that holds braces for another reason is left alone. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> Claude-Session: https://claude.ai/code/session_01Bp3538HRBPMQkb5ta9xRaj --- .../apache/camel/component/rest/RestProducer.java | 40 +++++++ .../rest/RestProducerUnresolvedPathWarnTest.java | 117 +++++++++++++++++++++ 2 files changed, 157 insertions(+) diff --git a/components/camel-rest/src/main/java/org/apache/camel/component/rest/RestProducer.java b/components/camel-rest/src/main/java/org/apache/camel/component/rest/RestProducer.java index 8b8abd47c963..f668f3de9fcf 100644 --- a/components/camel-rest/src/main/java/org/apache/camel/component/rest/RestProducer.java +++ b/components/camel-rest/src/main/java/org/apache/camel/component/rest/RestProducer.java @@ -23,7 +23,9 @@ import java.util.HashMap; import java.util.LinkedHashMap; import java.util.Locale; import java.util.Map; +import java.util.Set; import java.util.StringJoiner; +import java.util.concurrent.ConcurrentHashMap; import org.apache.camel.AsyncCallback; import org.apache.camel.AsyncProcessor; @@ -43,6 +45,8 @@ import org.apache.camel.support.service.ServiceHelper; import org.apache.camel.util.FileUtil; import org.apache.camel.util.ObjectHelper; import org.apache.camel.util.URISupport; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import static org.apache.camel.util.ObjectHelper.isEmpty; import static org.apache.camel.util.ObjectHelper.isNotEmpty; @@ -52,6 +56,11 @@ import static org.apache.camel.util.ObjectHelper.isNotEmpty; */ public class RestProducer extends DefaultAsyncProducer { + private static final Logger LOG = LoggerFactory.getLogger(RestProducer.class); + + /** The path parameters already warned about, so a misconfigured route says it once and not per message. */ + private final Set<String> warnedParameters = ConcurrentHashMap.newKeySet(); + private final CamelContext camelContext; private final RestConfiguration configuration; private boolean prepareUriTemplate = true; @@ -167,6 +176,16 @@ public class RestProducer extends DefaultAsyncProducer { } } resolvedUriTemplate = uriTemplateBuilder.toString(); + + // the request is sent with the placeholder still in the path, and the service answers 404 for a + // path that holds a {name}, so say which parameter had no value (CAMEL-24986) + String unresolved = firstPlaceholder(resolvedUriTemplate); + if (unresolved != null && warnedParameters.add(unresolved)) { + LOG.warn("The path parameter {{}} of {} has no value: set the header {}, or an exchange variable" + + " of that name, before the call. The request is sent with {{}} in the path, which the" + + " service is unlikely to answer. This is logged once per parameter.", + unresolved, resolvedUriTemplate, unresolved, unresolved); + } } } @@ -221,6 +240,27 @@ public class RestProducer extends DefaultAsyncProducer { } } + /** + * The name of the first {@code {name}} left in the template, or null when every one of them was resolved. Only a + * name counts, so a uri that holds braces for another reason is left alone (CAMEL-24986). + */ + private static String firstPlaceholder(String uriTemplate) { + int start = uriTemplate.indexOf('{'); + while (start >= 0) { + int end = uriTemplate.indexOf('}', start); + if (end < 0) { + return null; + } + String name = uriTemplate.substring(start + 1, end); + if (!name.isEmpty() && name.chars().allMatch( + c -> Character.isLetterOrDigit(c) || c == '_' || c == '-' || c == '.')) { + return name; + } + start = uriTemplate.indexOf('{', end); + } + return null; + } + /** * Replaces placeholders "{}" with message header or exchange variable values. * diff --git a/core/camel-core/src/test/java/org/apache/camel/component/rest/RestProducerUnresolvedPathWarnTest.java b/core/camel-core/src/test/java/org/apache/camel/component/rest/RestProducerUnresolvedPathWarnTest.java new file mode 100644 index 000000000000..934552a16aa6 --- /dev/null +++ b/core/camel-core/src/test/java/org/apache/camel/component/rest/RestProducerUnresolvedPathWarnTest.java @@ -0,0 +1,117 @@ +/* + * 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.rest; + +import java.util.HashMap; +import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; + +import org.apache.camel.Exchange; +import org.apache.camel.Message; +import org.apache.camel.component.log.ConsumingAppender; +import org.apache.camel.impl.DefaultCamelContext; +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.core.Appender; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * A path parameter with no value leaves its {name} in the uri and the service answers 404 for it, so the producer says + * which parameter it was. The request is still sent, as it was before (CAMEL-24986). + */ +public class RestProducerUnresolvedPathWarnTest { + + private final List<String> warnings = new CopyOnWriteArrayList<>(); + private final RestComponent restComponent; + private Appender appender; + + public RestProducerUnresolvedPathWarnTest() { + DefaultCamelContext context = new DefaultCamelContext(); + context.addComponent("mock-rest", new RestEndpointTest.MockRest()); + restComponent = new RestComponent(); + restComponent.setCamelContext(context); + } + + @BeforeEach + public void before() { + appender = ConsumingAppender.newAppender( + RestProducer.class.getName(), "UnresolvedPath", Level.WARN, + event -> warnings.add(event.getMessage().getFormattedMessage())); + } + + @AfterEach + public void after() { + if (appender != null) { + appender.stop(); + } + } + + private RestProducer createProducer(String uri) throws Exception { + final RestEndpoint restEndpoint = (RestEndpoint) restComponent.createEndpoint(uri); + restEndpoint.setConsumerComponentName("mock-rest"); + restEndpoint.setParameters(new HashMap<>()); + restEndpoint.setHost("http://localhost";); + restEndpoint.setBindingMode("json"); + return (RestProducer) restEndpoint.createProducer(); + } + + @Test + public void testSaysWhichParameterHasNoValue() throws Exception { + RestProducer producer = createProducer("rest:get:list/{id}/{val}"); + Exchange exchange = producer.createExchange(); + Message message = exchange.getIn(); + message.setHeader("id", 1); + + producer.process(exchange); + + // the request is still sent, with the placeholder in it, as before + assertEquals("http://localhost/list/1/{val}";, message.getHeader(Exchange.REST_HTTP_URI)); + assertEquals(1, warnings.size(), warnings.toString()); + assertTrue(warnings.get(0).contains("{val}"), warnings.get(0)); + assertTrue(warnings.get(0).contains("set the header val"), warnings.get(0)); + } + + @Test + public void testSaysItOncePerParameter() throws Exception { + RestProducer producer = createProducer("rest:get:list/{id}"); + + for (int i = 0; i < 3; i++) { + Exchange exchange = producer.createExchange(); + producer.process(exchange); + } + + // a route that is wrong is wrong for every message, so it is said once + assertEquals(1, warnings.size(), warnings.toString()); + assertTrue(warnings.get(0).contains("{id}"), warnings.get(0)); + } + + @Test + public void testSaysNothingWhenEveryParameterHasAValue() throws Exception { + RestProducer producer = createProducer("rest:get:list/{id}"); + Exchange exchange = producer.createExchange(); + exchange.getIn().setHeader("id", 1); + + producer.process(exchange); + + assertEquals("http://localhost/list/1";, exchange.getIn().getHeader(Exchange.REST_HTTP_URI)); + assertTrue(warnings.isEmpty(), warnings.toString()); + } +}
