automatically escape JSON values, if renderend in a template for a JSON content-type; removed 'fn.' prefix from Groovy function calls

Author: rrayst

README.md

@@ -56,9 +56,10 @@ api:
           htpasswdPath: .htpasswd
     - request:
         - template:
+            contentType: application/json
             src: |
               {
-                "sub": ${fn.toJSON(fn.user())}
+                "sub": ${user()}
               }
         - jwtSign:
             jwk:
@@ -716,7 +717,7 @@ api:
             contentType: application/json
             src: |
               {
-                "destination": ${fn.toJSON(json.city)}
+                "destination": ${json.city}
               }
     - return:
         status: 200
@@ -750,7 +751,7 @@ api:
     - request:
         - template:
             src: |
-              Buenas noches, ${fn.xpath('/person/@firstname')}
+              Buenas noches, ${xpath('/person/@firstname')}
         - return:
             status: 200
 ```

core/src/main/java/com/predic8/membrane/core/interceptor/templating/TemplateInterceptor.java

@@ -17,7 +17,9 @@
 import com.predic8.membrane.annot.*;
 import com.predic8.membrane.core.exceptions.*;
 import com.predic8.membrane.core.exchange.*;
+import com.predic8.membrane.core.http.MimeType;
 import com.predic8.membrane.core.interceptor.*;
+import com.predic8.membrane.core.lang.groovy.adapted.StreamingTemplateEngine;
 import com.predic8.membrane.core.util.*;
 import groovy.lang.*;
 import groovy.text.*;
@@ -40,7 +42,12 @@
  * header, query parameters, etc. If the extension of a referenced template file is <i>.xml</i> it will use
  * <a href="https://docs.groovy-lang.org/docs/next/html/documentation/template-engines.html#_xmltemplateengine">XMLTemplateEngine</a>
  * otherwise <a href="https://docs.groovy-lang.org/docs/next/html/documentation/template-engines.html#_streamingtemplateengine">StreamingTemplateEngine</a>.
- * Have a look at the samples in <a href="https://github.com/membrane/api-gateway/tree/master/distribution/examples">examples/template</a>.
+ * Have a look at the samples in <a href="https://github.com/membrane/api-gateway/tree/master/distribution/examples/templating">examples/templating</a>.
+ *
+ * When the <code>contentType</code> is a JSON variant (e.g., <code>application/json</code>), the engine automatically escapes all inserted values. For example, in the
+ * <a href="https://github.com/membrane/api-gateway/tree/master/distribution/examples/templating/json">JSON templating example</a>, executing
+ * <code>curl "localhost:2000/?answer=20"</code> returns <code>{ "answer" : "20" }</code>. The quotes surrounding the value 20 are added by the auto-escaping mechanism
+ * to ensure the output remains a valid string. This feature significantly mitigates security risks by preventing inadvertent JSON injection attacks.
  * @topic 2. Enterprise Integration Patterns
  */
 @MCElement(name = "template", mixed = true)
@@ -121,13 +128,17 @@ protected byte[] getContent(Exchange exchange, Flow flow) {
     }
 
     private @NotNull Map<String, Object> getVariableBinding(Exchange exc, Flow flow) {
-        return createParameterBindings(router, exc, flow, scriptAccessesJson && isJsonMessage(exc, flow));
+        return createParameterBindings(router, exc, flow, scriptAccessesJson && isJsonMessage(exc, flow), producesJSON());
     }
 
     private static boolean isJsonMessage(Exchange exc, Flow flow) {
         return exc.getMessage(flow).isJSON();
     }
 
+    private boolean producesJSON() {
+        return contentType != null && MimeType.isJson(contentType);
+    }
+
     private Template createTemplate() {
         if (src == null)
             throw new ConfigurationException("No template content provided via 'location' or inline text (%s).".formatted(getTemplateLocation()));

core/src/main/java/com/predic8/membrane/core/lang/AbstractScriptInterceptor.java

@@ -207,7 +207,7 @@ protected void handleScriptExecutionException(Exchange exc, Exception e) {
     }
 
     private Map<String, Object> getParameterBindings(Exchange exc, Flow flow, Message msg) {
-        var binding = createParameterBindings(router, exc, flow, scriptAccessesJson && msg.isJSON());
+        var binding = createParameterBindings(router, exc, flow, scriptAccessesJson && msg.isJSON(), false);
         addOutcomeObjects(binding);
         return binding;
     }

core/src/main/java/com/predic8/membrane/core/lang/ScriptingUtils.java

@@ -42,7 +42,9 @@ public class ScriptingUtils {
 
     private static final ObjectMapper om = new ObjectMapper();
 
-    public static Map<String, Object> createParameterBindings(Router router, Exchange exc, Flow flow, boolean includeJsonObject) {
+    public static final String BINDING = "fn";
+
+    public static Map<String, Object> createParameterBindings(Router router, Exchange exc, Flow flow, boolean includeJsonObject, boolean isJSON) {
 
         var msg = exc.getMessage(flow);
 
@@ -112,8 +114,16 @@ public static Map<String, Object> createParameterBindings(Router router, Exchang
 
         params.put("pathParam", new PathParametersMap(exc));
 
-        // Allow invoking functions in Groovy with ${fn.functionname()}
-        params.put("fn", new GroovyBuiltInFunctions(exc, flow, router));
+        // "fn" is the special key used to set the Groovy Binding.
+        // The Groovy Binding is allows function invocation in Groovy scripts with ${functionname()} .
+        params.put(BINDING, new GroovyBuiltInFunctions(exc, flow, router, params) {
+            @Override
+            public Object escape(Object o) {
+                if (isJSON)
+                    return toJSON(o);
+                return o;
+            }
+        });
 
         return params;
     }

core/src/main/java/com/predic8/membrane/core/lang/groovy/GroovyBuiltInFunctions.java

@@ -19,24 +19,24 @@
 import com.predic8.membrane.core.interceptor.Interceptor.Flow;
 import com.predic8.membrane.core.lang.CommonBuiltInFunctions;
 import com.predic8.membrane.core.router.*;
-import groovy.lang.GroovyObjectSupport;
+import groovy.lang.Binding;
 
 import java.util.List;
+import java.util.Map;
 
 /**
  * Helper class for built-in functions that delegates to the implementation CommonBuiltInFunctions.
- *  Difference to SpEL:
- *  The functions are called with ${fn.functionname()} instead of ${functionname()} in the template interceptor
  */
 @SuppressWarnings("unused")
-public class GroovyBuiltInFunctions extends GroovyObjectSupport {
+public class GroovyBuiltInFunctions extends Binding {
 
     private final Exchange exchange;
     private final Flow flow;
     private final Router router;
     private XmlConfig xmlConfig;
 
-    public GroovyBuiltInFunctions(Exchange exchange, Flow flow, Router router) {
+    public GroovyBuiltInFunctions(Exchange exchange, Flow flow, Router router, Map<String, Object> params) {
+        super(params);
         this.exchange = exchange;
         this.flow = flow;
         this.router = router;
@@ -118,4 +118,13 @@ public long getDefaultSessionLifetime(String beanName) {
     public String env(String s) {
         return CommonBuiltInFunctions.env(s);
     }
+
+    /**
+     * Post-Process values before they are written to the output.
+     *
+     * This gives subclasses the option to perform escaping (e.g. JSON/XML).
+     */
+    public Object escape(Object o) {
+        return o;
+    }
 }

core/src/main/java/com/predic8/membrane/core/lang/groovy/GroovyExchangeExpression.java

@@ -47,7 +47,7 @@ public GroovyExchangeExpression(String source, Router router) {
     public <T> T evaluate(Exchange exchange, Interceptor.Flow flow, Class<T> type) {
         Object o = null;
         try {
-            o = script.apply(createParameterBindings(router, exchange, flow, false));
+            o = script.apply(createParameterBindings(router, exchange, flow, false, false));
         } catch (MissingPropertyException mpe) {
             log.info("Expression {} tries to access non existing property {}",expression,mpe.getMessage());
             if (type.getName().equals(Object.class.getName())) {

core/src/main/java/com/predic8/membrane/core/lang/groovy/adapted/StreamingTemplateEngine.java

@@ -21,6 +21,7 @@
 
 package com.predic8.membrane.core.lang.groovy.adapted;
 
+import com.predic8.membrane.core.lang.ScriptingUtils;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import groovy.lang.*;
 import groovy.text.Template;
@@ -46,6 +47,8 @@
 import java.util.Map;
 import java.util.concurrent.atomic.AtomicInteger;
 
+import static com.predic8.membrane.core.lang.ScriptingUtils.BINDING;
+
 /**
  * Adapted from <a href="https://github.com/apache/groovy/blob/master/subprojects/groovy-templates/src/main/groovy/groovy/text/StreamingTemplateEngine.java">Apache Groovy</a> .
  *
@@ -55,7 +58,7 @@
  * methods.
  *
  * We therefore decided to copy the code of the inner class here (see first commit) and then made
- * some adjustments (the following commits).
+ * some adjustments (the following commits, see "adjustment" comments).
  */
 public class StreamingTemplateEngine extends groovy.text.StreamingTemplateEngine {
     @Override
@@ -86,7 +89,11 @@ private static class StreamingTemplate implements Template {
                 +   "return { _p, _s, _b, out -> "
                 +     "int _i = 0;"
                 +     "try {"
-                +       "delegate = new Binding(_b);";
+                // adjustment: using the GroovyBuiltInFunctions binding implementation
+                // provided from the outside (via the 'fn' map entry) allows the script
+                // to directly call Membrane functions. This enables templating syntax like
+                // ${xpath('/root/element')} .
+                +       "delegate = _b.get('" + BINDING + "');";
 
         /**
          * The 'footer' we use for the resulting groovy script source
@@ -514,7 +521,7 @@ private int parseDollarIdentifier(int c,
                                           final StringBuilder target,
                                           final Position sourcePosition,
                                           final Position targetPosition) throws IOException, FinishedReadingException {
-            append(target, targetPosition, "out<<");
+            append(target, targetPosition, "out<<escape("); // adjustment: escape the output
             append(target, targetPosition, (char) c);
 
             while (true) {
@@ -525,7 +532,7 @@ private int parseDollarIdentifier(int c,
                 append(target, targetPosition, (char) c);
             }
 
-            append(target, targetPosition, ";");
+            append(target, targetPosition, ");");
 
             return c;
         }
@@ -562,10 +569,13 @@ private void parseDollarCurlyIdentifier(final Reader reader,
                                                 final StringBuilder target,
                                                 final Position sourcePosition,
                                                 final Position targetPosition) throws IOException, FinishedReadingException {
-            append(target, targetPosition, "out<<\"\"\"${");
+            append(target, targetPosition, "out<<\"\"\"${escape("); // adjustment: escape the output
 
             while (true) {
                 int c = read(reader, sourcePosition);
+                // adjustment: escape the output
+                if (c == '}')
+                    append(target, targetPosition, ')');
                 append(target, targetPosition, (char) c);
                 if (c == '}') break;
             }
@@ -612,11 +622,11 @@ private void parseExpression(final Reader reader,
                                      final StringBuilder target,
                                      final Position sourcePosition,
                                      final Position targetPosition) throws IOException, FinishedReadingException {
-            append(target, targetPosition, "out<<\"\"\"${");
+            append(target, targetPosition, "out<<\"\"\"${escape("); // adjustment: escape the output
 
             readAndAppend(reader, target, sourcePosition, targetPosition);
 
-            append(target, targetPosition, "}\"\"\";");
+            append(target, targetPosition, ")}\"\"\";");
         }
 
         @Override

core/src/test/java/com/predic8/membrane/core/interceptor/templating/GroovyTemplateEscapingTest.java

@@ -0,0 +1,49 @@
+package com.predic8.membrane.core.interceptor.templating;
+
+import com.predic8.membrane.core.exchange.Exchange;
+import com.predic8.membrane.core.http.Request;
+import com.predic8.membrane.core.lang.groovy.adapted.StreamingTemplateEngine;
+import com.predic8.membrane.core.router.TestRouter;
+import groovy.text.Template;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+import java.io.IOException;
+import java.io.StringReader;
+import java.net.URISyntaxException;
+
+import static com.predic8.membrane.core.interceptor.Interceptor.Flow.REQUEST;
+import static com.predic8.membrane.core.lang.ScriptingUtils.createParameterBindings;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+public class GroovyTemplateEscapingTest {
+    private TestRouter r;
+    private Exchange e;
+
+    @BeforeEach
+    public void setUp() throws URISyntaxException {
+        r = new TestRouter();
+        e = Request.get("http://localhost/foo").buildExchange();
+    }
+
+    @Test
+    void noJson() throws IOException, ClassNotFoundException, URISyntaxException {
+        Template t = new StreamingTemplateEngine().createTemplate(new StringReader(
+                "${flow} $flow <%= flow %> ${hasScope('foo')}"));
+
+        assertEquals("REQUEST REQUEST REQUEST false",
+                t.make(createParameterBindings(r, e, REQUEST, false, false))
+                        .toString());
+    }
+
+    @Test
+    void json() throws IOException, ClassNotFoundException, URISyntaxException {
+        Template t = new StreamingTemplateEngine().createTemplate(new StringReader(
+                "${flow} $flow <%= flow %> ${hasScope('foo')}"));
+
+        assertEquals("\"REQUEST\" \"REQUEST\" \"REQUEST\" false",
+                t.make(createParameterBindings(r, e, REQUEST, false, true))
+                        .toString());
+    }
+
+}

core/src/test/java/com/predic8/membrane/core/interceptor/templating/TemplateInterceptorTest.java

@@ -108,10 +108,10 @@ void accessBindings() throws Exception {
                 <% for(h in header.allHeaderFields) { %>
                    <%= h.headerName %> : <%= h.value %>
                 <% } %>
-                Exchange: <%= exc %>
+                Exchange: <% out<<exc %>
                 Flow: <%= flow %>
                 Message.version: <%= message.version %>
-                Body: <%= message.body %>
+                Body: <% out<<message.body %>
                 Properties: <%= property.baz %>
                 <% for(p in property) { %>
                    Key: <%= p.key %> : <%= p.value %>
@@ -128,11 +128,11 @@ void accessBindings() throws Exception {
 
         String body = exchange.getRequest().getBodyAsStringDecoded();
         assertTrue(body.contains("/foo"));
-        assertTrue(body.contains("Flow: REQUEST"));
+        assertTrue(body.contains("Flow: \"REQUEST\""));
         assertTrue(body.contains("Body: vlinder"));
         assertTrue(body.contains("New: 7"));
-        assertTrue(body.contains("A: 1"));
-        assertTrue(body.contains("B: 2"));
+        assertTrue(body.contains("A: \"1\""));
+        assertTrue(body.contains("B: \"2\""));
     }
 
     @Test
@@ -248,7 +248,7 @@ void builtInFunctions() {
         exc.setProperty(SECURITY_SCHEMES, List.of(new BasicHttpSecurityScheme().username("alice")));
         ti.setContentType(APPLICATION_JSON);
         ti.setSrc("""
-        { "foo": "${fn.user()}" }
+        { "foo": ${user()} }
         """);
         ti.init(router);
         ti.handleRequest(exc);

core/src/test/java/com/predic8/membrane/core/lang/groovy/GroovyBuiltInFunctionsTest.java

@@ -34,7 +34,7 @@ class GroovyBuiltInFunctionsTest {
 
     @BeforeEach
     void setUp() throws URISyntaxException {
-        functions = new GroovyBuiltInFunctions(post("/foo").xml("<person name='Fritz'/>").buildExchange(), REQUEST, new DummyTestRouter());
+        functions = new GroovyBuiltInFunctions(post("/foo").xml("<person name='Fritz'/>").buildExchange(), REQUEST, new DummyTestRouter(), new HashMap<>());
     }
 
     @Test