From 4bee484972dac853bb8ab827574bd2e5cb6af60e Mon Sep 17 00:00:00 2001 From: Francisco Javier Tirado Sarti Date: Wed, 27 Sep 2023 17:48:46 +0200 Subject: [PATCH] [KOGITO-9844] Enhace jq validation to detect invalid functions --- .../org/kie/kogito/expr/jq/JqExpression.java | 84 +++++++++++++------ .../expr/jq/JqExpressionHandlerTest.java | 6 ++ 2 files changed, 63 insertions(+), 27 deletions(-) diff --git a/kogito-serverless-workflow/kogito-jq-expression/src/main/java/org/kie/kogito/expr/jq/JqExpression.java b/kogito-serverless-workflow/kogito-jq-expression/src/main/java/org/kie/kogito/expr/jq/JqExpression.java index 3930ababfca..d947c4ab16a 100644 --- a/kogito-serverless-workflow/kogito-jq-expression/src/main/java/org/kie/kogito/expr/jq/JqExpression.java +++ b/kogito-serverless-workflow/kogito-jq-expression/src/main/java/org/kie/kogito/expr/jq/JqExpression.java @@ -18,10 +18,13 @@ */ package org.kie.kogito.expr.jq; +import java.lang.reflect.Field; import java.util.ArrayList; import java.util.Collection; +import java.util.HashSet; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import java.util.function.Supplier; -import java.util.regex.Pattern; import org.kie.kogito.internal.process.runtime.KogitoProcessContext; import org.kie.kogito.jackson.utils.FunctionJsonNode; @@ -36,27 +39,36 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.node.ArrayNode; -import net.thisptr.jackson.jq.JsonQuery; import net.thisptr.jackson.jq.Output; import net.thisptr.jackson.jq.Scope; import net.thisptr.jackson.jq.Version; import net.thisptr.jackson.jq.exception.JsonQueryException; +import net.thisptr.jackson.jq.internal.javacc.ExpressionParser; +import net.thisptr.jackson.jq.internal.tree.FunctionCall; public class JqExpression implements Expression { static final String LANG = "jq"; private static final Logger logger = LoggerFactory.getLogger(JqExpression.class); + private final Map, Collection> declaredFieldsMap = new ConcurrentHashMap<>(); + private final Map, Collection> allFieldsMap = new ConcurrentHashMap<>(); + private final Supplier scope; private final String expr; - private final Version version; - private JsonQuery query; + + private net.thisptr.jackson.jq.Expression internalExpr; private JsonQueryException validationError; public JqExpression(Supplier scope, String expr, Version version) { this.expr = expr; this.scope = scope; - this.version = version; + try { + this.internalExpr = ExpressionParser.compile(expr, version); + checkFunctionCall(internalExpr); + } catch (JsonQueryException ex) { + validationError = ex; + } } private interface TypedOutput extends Output { @@ -160,40 +172,58 @@ private Scope getScope(KogitoProcessContext processInfo) { private T eval(JsonNode context, Class returnClass, KogitoProcessContext processInfo) { try (JsonNodeContext jsonNode = JsonNodeContext.from(context, processInfo)) { TypedOutput output = output(returnClass); - compile(); - query.apply(getScope(processInfo), jsonNode.getNode(), output); + if (validationError != null) { + throw validationError; + } + internalExpr.apply(getScope(processInfo), jsonNode.getNode(), output); return JsonObjectUtils.convertValue(output.getResult(), returnClass); } catch (JsonQueryException e) { throw new IllegalArgumentException("Unable to evaluate content " + context + " using expr " + expr, e); } } - private void compile() throws JsonQueryException { - if (this.query == null) { - try { - this.query = JsonQuery.compile(expr, version); - } catch (JsonQueryException ex) { - validationError = ex; - throw ex; - } + @Override + public boolean isValid() { + return validationError == null; + } + + private void checkFunctionCall(net.thisptr.jackson.jq.Expression toCheck) throws JsonQueryException { + if (toCheck instanceof FunctionCall) { + toCheck.apply(scope.get(), ObjectMapperFactory.get().createObjectNode(), out -> { + }); + } else { + for (Field f : getAllExprFields(toCheck)) + try { + checkFunctionCall((net.thisptr.jackson.jq.Expression) f.get(toCheck)); + } catch (ReflectiveOperationException e) { + logger.warn("Ignoring unexpected error {} while accesing field {} for class{} and expression {}", e.getMessage(), f.getName(), toCheck.getClass(), expr); + } } } - private static final Pattern JQ_FUNCTION_NAME = Pattern.compile("[a-zA-Z][a-zA-Z0-9_]*"); + private Collection getAllExprFields(net.thisptr.jackson.jq.Expression toCheck) { + return allFieldsMap.computeIfAbsent(toCheck.getClass(), this::getAllExprFields); + } - @Override - public boolean isValid() { - try { - compile(); - if (JQ_FUNCTION_NAME.matcher(expr).matches()) { - query.apply(scope.get(), ObjectMapperFactory.get().createObjectNode(), out -> { - }); + private Collection getAllExprFields(Class clazz) { + Collection fields = new HashSet<>(); + Class currentClass = clazz; + do { + fields.addAll(declaredFieldsMap.computeIfAbsent(currentClass.asSubclass(net.thisptr.jackson.jq.Expression.class), this::getDeclaredExprFields)); + currentClass = currentClass.getSuperclass(); + } while (net.thisptr.jackson.jq.Expression.class.isAssignableFrom(currentClass)); + return fields; + } + + private Collection getDeclaredExprFields(Class clazz) { + Collection fields = new HashSet<>(); + for (Field f : clazz.getDeclaredFields()) { + if (net.thisptr.jackson.jq.Expression.class.isAssignableFrom(f.getType())) { + f.setAccessible(true); + fields.add(f); } - } catch (JsonQueryException ex) { - logger.debug("Invalid expression {}", ex.getMessage()); - return false; } - return validationError == null; + return fields; } @Override diff --git a/kogito-serverless-workflow/kogito-jq-expression/src/test/java/org/kie/kogito/expr/jq/JqExpressionHandlerTest.java b/kogito-serverless-workflow/kogito-jq-expression/src/test/java/org/kie/kogito/expr/jq/JqExpressionHandlerTest.java index d5c11c6e8a8..b0032b019ec 100644 --- a/kogito-serverless-workflow/kogito-jq-expression/src/test/java/org/kie/kogito/expr/jq/JqExpressionHandlerTest.java +++ b/kogito-serverless-workflow/kogito-jq-expression/src/test/java/org/kie/kogito/expr/jq/JqExpressionHandlerTest.java @@ -275,6 +275,12 @@ void testMagicWordsExpressions(String expression, String expectedResult, KogitoP assertThat(parsedExpression.eval(getObjectNode(), String.class, context)).isEqualTo(expectedResult); } + @Test + void testHardcodedStringIsValidOrNot() { + assertThat(ExpressionHandlerFactory.get("jq", "kserve_payload = to_kserve(image)").isValid()).isFalse(); + assertThat(ExpressionHandlerFactory.get("jq", "length .variable").isValid()).isTrue(); + } + private static Stream provideMagicWordExpressionsToTest() { return Stream.of( Arguments.of("$WORKFLOW.instanceId", "1111-2222-3333", getContext()),