feat(core): enhance template marker detection and URL evaluation in Target and CallInterceptor

predic8 · rrayst · commit 54c7e3ba72a2 · 2026-02-20T09:20:04.000+01:00
- Replaced `evaluateExpressions` with `urlIsTemplate` for better semantics and clarity.
- Improved `Target` and `CallInterceptor` to skip URL evaluation when no template markers are present.
- Introduced stricter validation for configurations involving template markers and illegal characters.
- Updated relevant tests to validate changes and added new test cases for URL template evaluation.
diff --git a/core/src/main/java/com/predic8/membrane/core/interceptor/flow/CallInterceptor.java b/core/src/main/java/com/predic8/membrane/core/interceptor/flow/CallInterceptor.java
@@ -20,6 +20,7 @@
 import com.predic8.membrane.core.interceptor.*;
 import com.predic8.membrane.core.interceptor.lang.*;
 import com.predic8.membrane.core.transport.http.*;
+import com.predic8.membrane.core.util.*;
 import org.slf4j.*;
 
 import java.io.*;
@@ -32,6 +33,7 @@
 import static com.predic8.membrane.core.interceptor.Interceptor.Flow.*;
 import static com.predic8.membrane.core.interceptor.Outcome.*;
 import static com.predic8.membrane.core.interceptor.Outcome.ABORT;
+import static com.predic8.membrane.core.util.TemplateUtil.*;
 import static java.util.Collections.*;
 
 /**
@@ -43,6 +45,11 @@ public class CallInterceptor extends AbstractExchangeExpressionInterceptor {
 
     private static final Logger log = LoggerFactory.getLogger(CallInterceptor.class);
 
+    /**
+     * If url contains template marker ${}, if not expression evaluation is skipped
+     */
+    private boolean urlIsTemplate = false;
+
     /**
      * These headers are filtered out from the response of a called resource
      * and are not added to the current message.
@@ -56,6 +63,21 @@ public class CallInterceptor extends AbstractExchangeExpressionInterceptor {
     @Override
     public void init() {
         super.init();
+
+        if (router.getConfiguration().getUriFactory().isAllowIllegalCharacters()) {
+             throw new ConfigurationException("""
+                    URL Templating and Illegal URL Characters
+                    
+                    Url templating expressions and enablement of illegal characters in URLs are mutually exclusive. Either disable
+                    illegal characters in the configuration (configuration/uriFactory/allowIllegalCharacters) or remove the
+                    templating expression %s from the URL in the call URL.
+                    """.formatted(exchangeExpression.getExpression()));
+        }
+
+        // If there is no template marker ${ than do not try to evaluate url as expression
+        if (containsTemplateMarker(exchangeExpression.getExpression())) {
+            urlIsTemplate = true;
+        }
     }
 
     @Override
@@ -69,7 +91,7 @@ public Outcome handleResponse(Exchange exc) {
     }
 
     private Outcome handleInternal(Exchange exc) {
-        final String dest = exchangeExpression.evaluate(exc, REQUEST, String.class);
+        var dest = computeDestinationUrl(exc);
         log.debug("Calling {}", dest);
 
         final Exchange newExc = createNewExchange(dest, getNewRequest(exc));
@@ -107,6 +129,13 @@ private Outcome handleInternal(Exchange exc) {
         }
     }
 
+    private String computeDestinationUrl(Exchange exc) {
+        if (urlIsTemplate) {
+            return exchangeExpression.evaluate(exc, REQUEST, String.class);
+        }
+        return exchangeExpression.getExpression();
+    }
+
     private ProblemDetails createProblemDetails(String dest) {
         return internal(router.getConfiguration().isProduction(), "call")
                 .title("Error performing callout.")
diff --git a/core/src/main/java/com/predic8/membrane/core/proxies/Target.java b/core/src/main/java/com/predic8/membrane/core/proxies/Target.java
@@ -24,7 +24,6 @@
 import com.predic8.membrane.core.router.*;
 import com.predic8.membrane.core.util.*;
 import com.predic8.membrane.core.util.text.*;
-import com.predic8.membrane.core.util.uri.*;
 import com.predic8.membrane.core.util.uri.EscapingUtil.*;
 import org.slf4j.*;
 
@@ -64,9 +63,9 @@ public class Target implements XMLSupport {
     private Function<String, String> escapingFunction;
 
     /**
-     * If exchangeExpressions should be evaluated.
+     * If url contains template marker ${}, if not expression evaluation is skipped
      */
-    private boolean evaluateExpressions = false;
+    private boolean urlIsTemplate = false;
 
     private boolean adjustHostHeader = true;
 
@@ -75,7 +74,8 @@ public class Target implements XMLSupport {
 
     private InterceptorAdapter adapter;
 
-    public Target() {}
+    public Target() {
+    }
 
     public Target(String host) {
         setHost(host);
@@ -91,21 +91,24 @@ public void init(Router router) {
         if (!containsTemplateMarker(url))
             return;
 
-         adapter = new InterceptorAdapter(router, xmlConfig);
+        adapter = new InterceptorAdapter(router, xmlConfig);
 
         if (router.getConfiguration().getUriFactory().isAllowIllegalCharacters()) {
             log.warn("{}Url templates are disabled for security.{} Disable configuration/uriFactory/allowIllegalCharacters to enable them. Illegal characters in templates may lead to injection attacks.", TerminalColors.BRIGHT_RED(), RESET());
             throw new ConfigurationException("""
-                URL Templating and Illegal URL Characters
-                
-                Url templating expressions and enablement of illegal characters in URLs are mutually exclusive. Either disable
-                illegal characters in the configuration (configuration/uriFactory/allowIllegalCharacters) or remove the
-                templating expression %s from the target URL.
-                """.formatted(url));
-        } else {
-            evaluateExpressions = true;
-            escapingFunction = getEscapingFunction(escaping);
+                    URL Templating and Illegal URL Characters
+                    
+                    Url templating expressions and enablement of illegal characters in URLs are mutually exclusive. Either disable
+                    illegal characters in the configuration (configuration/uriFactory/allowIllegalCharacters) or remove the
+                    templating expression %s from the target URL.
+                    """.formatted(url));
+        }
+
+        // If there is no template marker ${ than do not try to evaluate url as expression
+        if(containsTemplateMarker(url)) {
+            urlIsTemplate = true;
         }
+        escapingFunction = getEscapingFunction(escaping);
     }
 
     public void applyModifications(Exchange exc, Router router) {
@@ -124,14 +127,9 @@ private List<String> computeDestinationExpressions(Exchange exc, Router router)
 
     private String evaluateTemplate(Exchange exc, Router router, String url, InterceptorAdapter adapter) {
         // Only evaluate if the target url contains a template marker ${}
-        if (!evaluateExpressions)
+        if (!urlIsTemplate)
             return url;
 
-        // If the url does not contain ${ we do not have to evaluate the expression
-        if (!containsTemplateMarker(url)) {
-            return url;
-        }
-
         // Without caching 1_000_000 => 37s with ConcurrentHashMap as Cache => 34s
         // Cache is probably not worth the effort and complexity
         return TemplateExchangeExpression.newInstance(adapter,
@@ -145,8 +143,8 @@ public String getHost() {
         return host;
     }
 
-    public boolean isEvaluateExpressions() {
-        return evaluateExpressions;
+    public boolean isUrlIsTemplate() {
+        return urlIsTemplate;
     }
 
     /**
diff --git a/core/src/test/java/com/predic8/membrane/core/interceptor/HTTPClientInterceptorTest.java b/core/src/test/java/com/predic8/membrane/core/interceptor/HTTPClientInterceptorTest.java
@@ -165,7 +165,7 @@ void illegalCharacterWithoutTemplate() {
                 fail();
                 return;
             }
-            assertFalse(apiProxy.getTarget().isEvaluateExpressions());
+            assertFalse(apiProxy.getTarget().isUrlIsTemplate());
             assertEquals(1, exc.getDestinations().size());
 
             // The template should not be evaluated, cause illegal characters are allowed!
diff --git a/core/src/test/java/com/predic8/membrane/core/interceptor/flow/CallInterceptorTest.java b/core/src/test/java/com/predic8/membrane/core/interceptor/flow/CallInterceptorTest.java
@@ -13,26 +13,38 @@
    limitations under the License. */
 package com.predic8.membrane.core.interceptor.flow;
 
-import com.predic8.membrane.core.exchange.Exchange;
-import com.predic8.membrane.core.http.Request;
-import com.predic8.membrane.core.http.Response;
-import org.junit.jupiter.api.BeforeAll;
-import org.junit.jupiter.api.Test;
+import com.predic8.membrane.core.exchange.*;
+import com.predic8.membrane.core.http.*;
+import com.predic8.membrane.core.interceptor.*;
+import com.predic8.membrane.core.interceptor.templating.*;
+import com.predic8.membrane.core.openapi.serviceproxy.*;
+import com.predic8.membrane.core.router.*;
+import com.predic8.membrane.core.util.*;
+import org.junit.jupiter.api.*;
 
-import java.net.URISyntaxException;
+import java.io.*;
+import java.net.*;
 
 import static com.predic8.membrane.core.http.Header.*;
-import static com.predic8.membrane.core.interceptor.flow.CallInterceptor.copyHeadersFromResponseToRequest;
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertNull;
+import static com.predic8.membrane.core.http.Request.*;
+import static com.predic8.membrane.core.interceptor.flow.CallInterceptor.*;
+import static org.junit.jupiter.api.Assertions.*;
 
 class CallInterceptorTest {
 
-    static Exchange exc;
+    private Exchange exc;
+    private Router router;
 
-    @BeforeAll
-    static void beforeAll() throws URISyntaxException {
-        exc = Request.get("/foo").buildExchange();
+    @BeforeEach
+    void setup() throws URISyntaxException {
+        exc = get("/foo").buildExchange();
+        exc.setProperty("a", "b");
+        router = new DefaultRouter();
+    }
+
+    @AfterEach
+    void teardown() {
+        router.stop();
     }
 
     @Test
@@ -46,12 +58,47 @@ void filterHeaders() {
         copyHeadersFromResponseToRequest(exc, exc);
 
         // preserve
-        assertEquals("42",exc.getRequest().getHeader().getFirstValue("X-FOO"));
+        var header = exc.getRequest().getHeader();
+        assertEquals("42", header.getFirstValue("X-FOO"));
 
         // take out
-        assertNull(exc.getRequest().getHeader().getFirstValue(SERVER));
-        assertNull(exc.getRequest().getHeader().getFirstValue(TRANSFER_ENCODING));
-        assertNull(exc.getRequest().getHeader().getFirstValue(CONTENT_ENCODING));
+        assertNull(header.getFirstValue(SERVER));
+        assertNull(header.getFirstValue(TRANSFER_ENCODING));
+        assertNull(header.getFirstValue(CONTENT_ENCODING));
+    }
+
+    @Test
+    void evaluateUrlTemplate() throws IOException {
+        extracted("Path: /b");
     }
 
+    @Test
+    void urlTemplateAndAllowIllegalCharactersInURL() {
+        router.getConfiguration().getUriFactory().setAllowIllegalCharacters(true);
+        assertThrows(ConfigurationException.class, () -> extracted("dummy"));
+    }
+
+    private void extracted(String expected) throws IOException {
+        var api = new APIProxy();
+        api.setKey(new APIProxyKey(2000));
+        api.getFlow().add(new AbstractInterceptor() {
+            @Override
+            public Outcome handleRequest(Exchange exc) {
+                System.out.println(exc);
+                return super.handleRequest(exc);
+            }
+        });
+        api.getFlow().add(new TemplateInterceptor() {{
+            setSrc("Path: ${path}");
+        }});
+        api.getFlow().add(new ReturnInterceptor());
+        router.add(api);
+        router.start();
+
+        var ci = new CallInterceptor();
+        ci.setUrl("http://localhost:2000/${property.a}");
+        ci.init(router);
+        ci.handleRequest(exc);
+        assertEquals(expected, exc.getRequest().getBodyAsStringDecoded());
+    }
 }
diff --git a/distribution/examples/configuration/apis.yaml b/distribution/examples/configuration/apis.yaml
@@ -22,7 +22,6 @@ configuration:
   uriFactory:
     # Allow non-RFC-compliant characters like {, } in URIs
     # Attention: This may lead to security vulnerabilities! Use with care!
-    # - ${expression} in the URI will be evaluated
     allowIllegalCharacters: true
     autoEscapeBackslashes: true # Escape backslashes in incoming URIs (\\ -> %5C)
 ---

Original file line number	Diff line number	Diff line change
`@@ -165,7 +165,7 @@ void illegalCharacterWithoutTemplate() {`
`165`	`165`	`fail();`
`166`	`166`	`return;`
`167`	`167`	`}`
`168`		`- assertFalse(apiProxy.getTarget().isEvaluateExpressions());`
	`168`	`+ assertFalse(apiProxy.getTarget().isUrlIsTemplate());`
`169`	`169`	`assertEquals(1, exc.getDestinations().size());`
`170`	`170`
`171`	`171`	`// The template should not be evaluated, cause illegal characters are allowed!`