Type safety in Groovy: introducing type declarations in scripts, some…

… sandboxing tests (failing)
Evolveum · Mar 20, 2019 · f5a06f0 · f5a06f0
1 parent 3e440e6
commit f5a06f0
Show file tree

Hide file tree

Showing 19 changed files with 475 additions and 56 deletions.
diff --git a/infra/prism-api/src/main/java/com/evolveum/midpoint/prism/Definition.java b/infra/prism-api/src/main/java/com/evolveum/midpoint/prism/Definition.java
@@ -174,7 +174,10 @@ default SchemaRegistry getSchemaRegistry() {
 	// TODO fix this!
 	Class getTypeClassIfKnown();
 
-	// todo suspicious, please investigate and document
+	/**
+	 * Returns a compile-time class that is used to represent items.
+	 * E.g. returns String, Integer, sublcasses of Objectable and Containerable and so on. 
+	 */
 	Class getTypeClass();
 
 	/**

diff --git a/infra/prism-api/src/main/java/com/evolveum/midpoint/prism/PrismContainerDefinition.java b/infra/prism-api/src/main/java/com/evolveum/midpoint/prism/PrismContainerDefinition.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010-2018 Evolveum
+ * Copyright (c) 2010-2019 Evolveum
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -62,4 +62,7 @@ public interface PrismContainerDefinition<C extends Containerable> extends ItemD
 
 	@Override
 	MutablePrismContainerDefinition<C> toMutable();
+
+	@Override
+	Class<C> getTypeClass();
 }
diff --git a/...ism-impl/src/main/java/com/evolveum/midpoint/prism/impl/PrismContainerDefinitionImpl.java b/...ism-impl/src/main/java/com/evolveum/midpoint/prism/impl/PrismContainerDefinitionImpl.java
@@ -112,6 +112,11 @@ public Class<C> getCompileTimeClass() {
 	public void setCompileTimeClass(Class<C> compileTimeClass) {
 		this.compileTimeClass = compileTimeClass;
 	}
+
+	@Override
+	public Class<C> getTypeClass() {
+		return compileTimeClass;
+	}
 
     protected String getSchemaNamespace() {
         return getName().getNamespaceURI();

diff --git a/...ism-impl/src/main/java/com/evolveum/midpoint/prism/impl/PrismReferenceDefinitionImpl.java b/...ism-impl/src/main/java/com/evolveum/midpoint/prism/impl/PrismReferenceDefinitionImpl.java
@@ -24,6 +24,8 @@
 import com.evolveum.midpoint.prism.path.ItemPath;
 import com.evolveum.midpoint.prism.util.DefinitionUtil;
 import com.evolveum.midpoint.util.QNameUtil;
+import com.evolveum.prism.xml.ns._public.types_3.ObjectReferenceType;
+
 import org.jetbrains.annotations.NotNull;
 
 /**
@@ -149,6 +151,11 @@ public boolean canBeDefinitionOf(PrismValue pvalue) {
 			return true;
 		}
 	}
+
+	@Override
+	public Class getTypeClass() {
+		return ObjectReferenceType.class;
+	}
 
 	@Override
 	public MutablePrismReferenceDefinition toMutable() {

diff --git a/infra/schema/src/main/java/com/evolveum/midpoint/schema/expression/TypedValue.java b/infra/schema/src/main/java/com/evolveum/midpoint/schema/expression/TypedValue.java
@@ -17,14 +17,27 @@
 
 import com.evolveum.midpoint.prism.Item;
 import com.evolveum.midpoint.prism.ItemDefinition;
+import com.evolveum.midpoint.prism.PrismReferenceDefinition;
 import com.evolveum.midpoint.util.ShortDumpable;
+import com.evolveum.midpoint.util.exception.SchemaException;
+import com.evolveum.midpoint.xml.ns._public.common.common_3.ObjectReferenceType;
 
 /**
  * Value and definition pair. E.g. used in expression variable maps.
  * We need to have explicit type here. It may happen that there will be
  * variables without any value. But we need to know the type of the
  * variable to compile the scripts properly.
  * 
+ * The definition, typeClass and the T parameter of this class refer to the
+ * type of the value as it should appear in the expression. The actual
+ * value may be different when TypedValue is created. The value may
+ * get converted as it is passing down the stack.
+ * 
+ * E.g. if we want script variable to contain a user, the type should be
+ * declared as UserType, not as object reference - even though the value
+ * is object reference when the TypedValue is created. But the reference
+ * is resolved down the way to place user in the value. 
+ * 
  * @author Radovan Semancik
  */
 public class TypedValue<T> implements ShortDumpable {
@@ -97,6 +110,31 @@ public void setTypeClass(Class<T> typeClass) {
 		this.typeClass = typeClass;
 	}
 
+
+	public Class<T> determineClass() throws SchemaException {
+		if (definition == null) {
+			if (typeClass == null) {
+				throw new SchemaException("Cannot determine class for variable, neither definition nor class specified");
+			} else {
+				return typeClass;
+			}
+		} else {
+			Class determinedClass;
+			if (definition instanceof PrismReferenceDefinition) {
+				// Stock prism reference would return ObjectReferenceType from prism schema.
+				// But we have exteded type for this.
+				// TODO: how to make this more elegant?
+				determinedClass = ObjectReferenceType.class;
+			} else {
+				determinedClass = definition.getTypeClass();
+			}
+			if (determinedClass == null) {
+				throw new SchemaException("Cannot determine class from definition "+definition);
+			}
+			return determinedClass;
+		}
+	}
+
 	@Override
 	public int hashCode() {
 		final int prime = 31;

diff --git a/.../com/evolveum/midpoint/model/common/expression/script/AbstractCachingScriptEvaluator.java b/.../com/evolveum/midpoint/model/common/expression/script/AbstractCachingScriptEvaluator.java
@@ -22,25 +22,23 @@
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.function.Function;
 
-import javax.script.Bindings;
-import javax.script.Compilable;
-import javax.script.CompiledScript;
-import javax.script.ScriptEngine;
-import javax.script.ScriptEngineManager;
-import javax.script.ScriptException;
 import javax.xml.namespace.QName;
 
 import com.evolveum.midpoint.common.LocalizationService;
 import com.evolveum.midpoint.model.common.expression.functions.FunctionLibrary;
-import com.evolveum.midpoint.model.common.expression.script.ScriptEvaluator;
-import com.evolveum.midpoint.prism.*;
+import com.evolveum.midpoint.prism.ItemDefinition;
+import com.evolveum.midpoint.prism.PrismContainerDefinition;
+import com.evolveum.midpoint.prism.PrismContainerValue;
+import com.evolveum.midpoint.prism.PrismContext;
+import com.evolveum.midpoint.prism.PrismProperty;
+import com.evolveum.midpoint.prism.PrismValue;
+import com.evolveum.midpoint.prism.PrismValueCollectionsUtil;
 import com.evolveum.midpoint.prism.crypto.Protector;
 import com.evolveum.midpoint.prism.xml.XsdTypeMapper;
 import com.evolveum.midpoint.repo.common.ObjectResolver;
 import com.evolveum.midpoint.repo.common.expression.ExpressionSyntaxException;
 import com.evolveum.midpoint.repo.common.expression.ExpressionUtil;
 import com.evolveum.midpoint.repo.common.expression.ExpressionVariables;
-import com.evolveum.midpoint.schema.constants.MidPointConstants;
 import com.evolveum.midpoint.schema.internals.InternalCounters;
 import com.evolveum.midpoint.schema.internals.InternalMonitor;
 import com.evolveum.midpoint.schema.result.OperationResult;
@@ -51,7 +49,6 @@
 import com.evolveum.midpoint.util.exception.ExpressionEvaluationException;
 import com.evolveum.midpoint.util.exception.ObjectNotFoundException;
 import com.evolveum.midpoint.util.exception.SecurityViolationException;
-import com.evolveum.midpoint.util.exception.SystemException;
 import com.evolveum.midpoint.util.logging.Trace;
 import com.evolveum.midpoint.util.logging.TraceManager;
 import com.evolveum.midpoint.xml.ns._public.common.common_3.ScriptExpressionEvaluatorType;
@@ -99,12 +96,16 @@ public <T, V extends PrismValue> List<V> evaluate(ScriptExpressionEvaluatorType
 			allowEmptyValues = expressionType.isAllowEmptyValues();
 		}
 
-		C compiledScript = getCompiledScript(codeString, contextDescription);
+		C compiledScript = getCompiledScript(codeString, variables, functions, contextDescription);
 
 		Object evalRawResult;
 		try {
+			beforeEvaluation(compiledScript, variables, functions, contextDescription, task, result);
 			InternalMonitor.recordCount(InternalCounters.SCRIPT_EXECUTION_COUNT);
+
 			evalRawResult = evaluateScript(compiledScript, variables, objectResolver, functions, contextDescription, task, result);
+
+			afterEvaluation(evalRawResult, compiledScript, variables, functions, contextDescription, task, result);
 		} catch (Throwable e) {
 			throw getLocalizationService().translate(
 					new ExpressionEvaluationException(e.getMessage() + " in " + contextDescription,
@@ -160,22 +161,26 @@ public <T, V extends PrismValue> List<V> evaluate(ScriptExpressionEvaluatorType
 		return pvals;
 	}
 
-	protected C getCompiledScript(String codeString, String contextDescription) throws ExpressionEvaluationException {
+	protected C getCompiledScript(String codeString, ExpressionVariables variables, Collection<FunctionLibrary> functions, String contextDescription) throws ExpressionEvaluationException {
 		C compiledScript = scriptCache.get(codeString);
 		if (compiledScript != null) {
 			return compiledScript;
 		}
 		InternalMonitor.recordCount(InternalCounters.SCRIPT_COMPILE_COUNT);
 		try {
-			compiledScript = compileScript(codeString, contextDescription);
+			beforeCompileScript(codeString, variables, functions, contextDescription);
+
+			compiledScript = compileScript(codeString, variables, contextDescription);
+
+			afterCompileScript(compiledScript, codeString, variables, functions, contextDescription);
 		} catch (Exception e) {
 			throw new ExpressionEvaluationException(e.getMessage() + " while compiling " + contextDescription, e);
 		}
 		scriptCache.put(codeString, compiledScript);
 		return compiledScript;
 	}
-	
-	protected abstract C compileScript(String codeString, String contextDescription) throws Exception;
+
+	protected abstract C compileScript(String codeString, ExpressionVariables variables, String contextDescription) throws Exception;
 
 	protected abstract Object evaluateScript(C compiledScript, ExpressionVariables variables,
 			ObjectResolver objectResolver, Collection<FunctionLibrary> functions, String contextDescription,
@@ -196,4 +201,22 @@ private <T> T convertScalarResult(Class<T> expectedType, Function<Object, Object
 		}
 	}
 
+	// HOOKS
+
+	protected void beforeCompileScript(String codeString, ExpressionVariables variables, Collection<FunctionLibrary> functions, String contextDescription) {
+
+	}
+
+	protected void afterCompileScript(C compiledScript, String codeString, ExpressionVariables variables, Collection<FunctionLibrary> functions, String contextDescription) {
+
+	}
+
+	protected void beforeEvaluation(C compiledScriptClass, ExpressionVariables variables, Collection<FunctionLibrary> functions, String contextDescription, Task task, OperationResult result) {
+
+	}
+
+	protected void afterEvaluation(Object resultObject, C compiledScriptClass, ExpressionVariables variables, Collection<FunctionLibrary> functions, String contextDescription, Task task, OperationResult result) {
+
+	}
+
 }
diff --git a/...main/java/com/evolveum/midpoint/model/common/expression/script/groovy/CompileOptions.java b/...main/java/com/evolveum/midpoint/model/common/expression/script/groovy/CompileOptions.java
@@ -15,6 +15,9 @@
  */
 package com.evolveum.midpoint.model.common.expression.script.groovy;
 
+import java.util.Collection;
+
+import com.evolveum.midpoint.model.common.expression.functions.FunctionLibrary;
 import com.evolveum.midpoint.repo.common.expression.ExpressionVariables;
 
 import groovy.lang.Binding;
@@ -26,6 +29,8 @@
 public class CompileOptions {
 
 	private ExpressionVariables variables;
+	private Collection<FunctionLibrary> functions;
+	private String contextDescription;
 
 	public ExpressionVariables getVariables() {
 		return variables;
@@ -34,5 +39,21 @@ public ExpressionVariables getVariables() {
 	public void setVariables(ExpressionVariables variables) {
 		this.variables = variables;
 	}
+
+	public Collection<FunctionLibrary> getFunctions() {
+		return functions;
+	}
+
+	public void setFunctions(Collection<FunctionLibrary> functions) {
+		this.functions = functions;
+	}
+
+	public String getContextDescription() {
+		return contextDescription;
+	}
+
+	public void setContextDescription(String contextDescription) {
+		this.contextDescription = contextDescription;
+	}
 
 }
diff --git a/...va/com/evolveum/midpoint/model/common/expression/script/groovy/GroovyScriptEvaluator.java b/...va/com/evolveum/midpoint/model/common/expression/script/groovy/GroovyScriptEvaluator.java
@@ -85,7 +85,7 @@ public String getLanguageUrl() {
 
 
 	@Override
-	protected Class compileScript(String codeString, String contextDescription)
+	protected Class compileScript(String codeString, ExpressionVariables variables, String contextDescription)
 			throws ExpressionEvaluationException {
 		return groovyLoader.parseClass(codeString, contextDescription);
 	}
@@ -102,23 +102,12 @@ protected Object evaluateScript(Class compiledScriptClass, ExpressionVariables v
 
 		Binding binding = new Binding(getVariableValuesMap(variables, objectResolver, functions, contextDescription, task, result));
 
-		beforeEvaluation(compiledScriptClass, variables, binding, contextDescription, task, result);
-
 		Script scriptResultObject = InvokerHelper.createScript(compiledScriptClass, binding);
 
 		Object resultObject = scriptResultObject.run();
 
-		afterEvaluation(resultObject, compiledScriptClass, variables, binding, contextDescription, task, result);
-
 		return resultObject;
 	}
-
-	protected void beforeEvaluation(Class compiledScriptClass, ExpressionVariables variables, Binding binding, String contextDescription, Task task, OperationResult result) {
-
-	}
 
-	protected void afterEvaluation(Object resultObject, Class compiledScriptClass, ExpressionVariables variables, Binding binding, String contextDescription, Task task, OperationResult result) {
-
-	}
 
 }
diff --git a/...evolveum/midpoint/model/common/expression/script/groovy/SandboxTypeCheckingExtension.java b/...evolveum/midpoint/model/common/expression/script/groovy/SandboxTypeCheckingExtension.java
@@ -15,6 +15,8 @@
  */
 package com.evolveum.midpoint.model.common.expression.script.groovy;
 
+import java.util.Collection;
+
 import org.codehaus.groovy.ast.ClassHelper;
 import org.codehaus.groovy.ast.ClassNode;
 import org.codehaus.groovy.ast.MethodNode;
@@ -23,7 +25,10 @@
 import org.codehaus.groovy.transform.stc.AbstractTypeCheckingExtension;
 import org.codehaus.groovy.transform.stc.StaticTypeCheckingVisitor;
 
+import com.evolveum.midpoint.model.common.expression.functions.FunctionLibrary;
 import com.evolveum.midpoint.repo.common.expression.ExpressionVariables;
+import com.evolveum.midpoint.schema.expression.TypedValue;
+import com.evolveum.midpoint.util.exception.SchemaException;
 import com.evolveum.midpoint.util.logging.Trace;
 import com.evolveum.midpoint.util.logging.TraceManager;
 
@@ -41,7 +46,11 @@ public SandboxTypeCheckingExtension(StaticTypeCheckingVisitor typeCheckingVisito
 	}
 
 	private CompileOptions getCompileOptions() {
-		return SandboxedGroovyScriptEvaluator.COMPILE_OPTIONS.get();
+		CompileOptions compileOptions = SandboxedGroovyScriptEvaluator.COMPILE_OPTIONS.get();
+		if (compileOptions == null) {
+			throw new AssertionError("No compile option in thread-local variable during script compilation");
+		}
+		return compileOptions;
 	}
 
 	@Override
@@ -54,16 +63,47 @@ public void onMethodSelection(final Expression expression, final MethodNode targ
 	public boolean handleUnresolvedVariableExpression(VariableExpression vexp) {
 		String variableName = vexp.getName();
 		LOGGER.info("GROOVY:handleUnresolvedVariableExpression: variableName={}", variableName);
+		CompileOptions compileOptions = getCompileOptions();
+		String contextDescription = compileOptions.getContextDescription();
+
 		if (!isDynamic(vexp)) {
+			LOGGER.error("Unresolved script variable {} because it is not dynamic, in {}", contextDescription);
 			return false;
 		}
-		ExpressionVariables variables = getCompileOptions().getVariables();
-		if (!variables.containsKey(variableName)) {
-			return false;
+
+		ExpressionVariables variables = compileOptions.getVariables();
+		if (variables != null) {
+			TypedValue variableTypedValue = variables.get(variableName);
+			if (variableTypedValue != null) {
+				Class variableClass;
+				try {
+					variableClass = variableTypedValue.determineClass();
+				} catch (SchemaException e) {
+					String msg = "Cannot determine class for "+variableTypedValue+" in "+contextDescription+": "+e.getMessage();
+					LOGGER.error("{}", msg);
+					throw new IllegalStateException(msg, e);
+				}
+				LOGGER.trace("Determine script variable {} as expression variable, class {} in {}", variableName, variableClass, contextDescription);
+				storeType(vexp, ClassHelper.make(variableClass));
+				setHandled(true);
+				return true;
+			}
+		}
+
+		Collection<FunctionLibrary> functions = compileOptions.getFunctions();
+		if (functions != null) {
+			for (FunctionLibrary function : functions) {
+				if (function.getVariableName().equals(variableName)) {
+					Class functionClass = function.getGenericFunctions().getClass();
+					LOGGER.trace("Determine script variable {} as function library, class {} in {}", variableName, functionClass, contextDescription);
+					storeType(vexp, ClassHelper.make(functionClass));
+					setHandled(true);
+					return true;
+				}
+			}
 		}
-//		Object variableValue = variables.getVariable(variableName);
-//		ClassHelper.make(c)
-//		makeDynamic(vexp, returnType);
+
+		LOGGER.error("Unresolved script variable {} because no declaration for it cannot be found in {}", contextDescription);
 		return false;
     }
 }