fix(engine): fix set variables batch operation class-loading bug

If a class is only present in a process application, the object variable cannot be deserialized in the job execution handler. related to CAM-13435, closes camunda#1426
tmetzke · May 3, 2021 · c1a130e · c1a130e
1 parent e33732e
commit c1a130e
Show file tree

Hide file tree

Showing 14 changed files with 298 additions and 77 deletions.
diff --git a/...e/src/main/java/org/camunda/bpm/engine/impl/batch/variables/BatchSetVariablesHandler.java b/...e/src/main/java/org/camunda/bpm/engine/impl/batch/variables/BatchSetVariablesHandler.java
@@ -22,17 +22,17 @@
 import org.camunda.bpm.engine.impl.batch.BatchJobConfiguration;
 import org.camunda.bpm.engine.impl.batch.BatchJobContext;
 import org.camunda.bpm.engine.impl.batch.BatchJobDeclaration;
+import org.camunda.bpm.engine.impl.cmd.SetExecutionVariablesCmd;
+import org.camunda.bpm.engine.impl.core.variable.VariableUtil;
 import org.camunda.bpm.engine.impl.interceptor.CommandContext;
 import org.camunda.bpm.engine.impl.jobexecutor.JobDeclaration;
 import org.camunda.bpm.engine.impl.json.JsonObjectConverter;
 import org.camunda.bpm.engine.impl.persistence.entity.ByteArrayEntity;
 import org.camunda.bpm.engine.impl.persistence.entity.ExecutionEntity;
 import org.camunda.bpm.engine.impl.persistence.entity.MessageEntity;
-import org.camunda.bpm.engine.impl.persistence.entity.VariableInstanceEntity;
 
 import java.util.List;
 import java.util.Map;
-import java.util.stream.Collectors;
 
 public class BatchSetVariablesHandler extends AbstractBatchJobHandler<BatchConfiguration> {
 
@@ -52,12 +52,7 @@ public void execute(BatchJobConfiguration configuration,
     BatchConfiguration batchConfiguration = readConfiguration(configurationByteArray);
 
     String batchId = batchConfiguration.getBatchId();
-    List<VariableInstanceEntity> variableInstances = commandContext.getVariableInstanceManager()
-        .findVariableInstancesByBatchId(batchId);
-
-    Map<String, Object> variables = variableInstances.stream()
-        .collect(Collectors.toMap(VariableInstanceEntity::getName,
-            VariableInstanceEntity::getTypedValueWithImplicitUpdatesSkipped));
+    Map<String, ?> variables = VariableUtil.findBatchVariablesSerialized(batchId, commandContext);
 
     List<String> processInstanceIds = batchConfiguration.getIds();
 
@@ -69,9 +64,8 @@ public void execute(BatchJobConfiguration configuration,
 
     try {
       for (String processInstanceId : processInstanceIds) {
-          commandContext.getProcessEngineConfiguration()
-              .getRuntimeService()
-              .setVariables(processInstanceId, variables);
+        new SetExecutionVariablesCmd(processInstanceId, variables, false, true)
+            .execute(commandContext);
       }
 
     } finally {

diff --git a/engine/src/main/java/org/camunda/bpm/engine/impl/cmd/AbstractSetVariableCmd.java b/engine/src/main/java/org/camunda/bpm/engine/impl/cmd/AbstractSetVariableCmd.java
@@ -30,16 +30,26 @@ public abstract class AbstractSetVariableCmd extends AbstractVariableCmd {
 
   protected Map<String, ? extends Object> variables;
 
+  protected boolean skipJavaSerializationFormatCheck;
+
   public AbstractSetVariableCmd(String entityId, Map<String, ? extends Object> variables, boolean isLocal) {
     super(entityId, isLocal);
     this.variables = variables;
   }
 
+  public AbstractSetVariableCmd(String entityId,
+                                Map<String, ? extends Object> variables,
+                                boolean isLocal,
+                                boolean skipJavaSerializationFormatCheck) {
+    this(entityId, variables, isLocal);
+    this.skipJavaSerializationFormatCheck = skipJavaSerializationFormatCheck;
+  }
+
   protected void executeOperation(AbstractVariableScope scope) {
     if (isLocal) {
-      scope.setVariablesLocal(variables);
+      scope.setVariablesLocal(variables, skipJavaSerializationFormatCheck);
     } else {
-      scope.setVariables(variables);
+      scope.setVariables(variables, skipJavaSerializationFormatCheck);
     }
   }
 

diff --git a/engine/src/main/java/org/camunda/bpm/engine/impl/cmd/SetExecutionVariablesCmd.java b/engine/src/main/java/org/camunda/bpm/engine/impl/cmd/SetExecutionVariablesCmd.java
@@ -34,8 +34,15 @@ public class SetExecutionVariablesCmd extends AbstractSetVariableCmd {
 
   private static final long serialVersionUID = 1L;
 
+  public SetExecutionVariablesCmd(String executionId,
+                                  Map<String, ?> variables,
+                                  boolean isLocal,
+                                  boolean skipJavaSerializationFormatCheck) {
+    super(executionId, variables, isLocal, skipJavaSerializationFormatCheck);
+  }
+
   public SetExecutionVariablesCmd(String executionId, Map<String, ? extends Object> variables, boolean isLocal) {
-    super(executionId, variables, isLocal);
+    super(executionId, variables, isLocal, false);
   }
 
   protected ExecutionEntity getEntity() {

diff --git a/engine/src/main/java/org/camunda/bpm/engine/impl/core/CoreLogger.java b/engine/src/main/java/org/camunda/bpm/engine/impl/core/CoreLogger.java
@@ -21,6 +21,7 @@
 import org.camunda.bpm.engine.impl.core.instance.CoreExecution;
 import org.camunda.bpm.engine.impl.core.operation.CoreAtomicOperation;
 import org.camunda.bpm.engine.impl.core.variable.CoreVariableInstance;
+import org.camunda.bpm.engine.impl.core.variable.VariableUtil;
 import org.camunda.bpm.engine.impl.core.variable.scope.AbstractVariableScope;
 
 /**
@@ -73,12 +74,8 @@ public ProcessEngineException transientVariableException(String variableName) {
       ));
   }
 
-  public ProcessEngineException javaSerializationProhibitedException(String variableName) {
-    return new ProcessEngineException(exceptionMessage(
-        "007",
-        "Cannot set variable with name {}. Java serialization format is prohibited",
-        variableName
-      ));
+  public ProcessEngineException javaSerializationProhibitedException(String message) {
+    return new ProcessEngineException(exceptionMessage("007", message));
   }
 
 }
diff --git a/engine/src/main/java/org/camunda/bpm/engine/impl/core/variable/VariableUtil.java b/engine/src/main/java/org/camunda/bpm/engine/impl/core/variable/VariableUtil.java
@@ -20,6 +20,8 @@
 import org.camunda.bpm.engine.impl.cfg.ProcessEngineConfigurationImpl;
 import org.camunda.bpm.engine.impl.context.Context;
 import org.camunda.bpm.engine.impl.core.CoreLogger;
+import org.camunda.bpm.engine.impl.interceptor.CommandContext;
+import org.camunda.bpm.engine.impl.persistence.entity.VariableInstanceEntity;
 import org.camunda.bpm.engine.impl.persistence.entity.util.TypedValueField;
 import org.camunda.bpm.engine.impl.variable.serializer.TypedValueSerializer;
 import org.camunda.bpm.engine.impl.variable.serializer.VariableSerializerFactory;
@@ -28,18 +30,23 @@
 import org.camunda.bpm.engine.variable.value.SerializableValue;
 import org.camunda.bpm.engine.variable.value.TypedValue;
 
+import java.text.MessageFormat;
+import java.util.List;
 import java.util.Map;
+import java.util.stream.Collector;
+import java.util.stream.Collectors;
 
 public class VariableUtil {
 
-  public static CoreLogger CORE_LOGGER = ProcessEngineLogger.CORE_LOGGER;
+  public static final CoreLogger CORE_LOGGER = ProcessEngineLogger.CORE_LOGGER;
+
+  public static final String ERROR_MSG = "Cannot set variable with name {0}. Java serialization format is prohibited";
 
   /**
    * Checks, if Java serialization will be used and if it is allowed to be used.
-   * @param variableName
    * @param value
    */
-  public static void checkJavaSerialization(String variableName, TypedValue value) {
+  public static boolean isJavaSerializationProhibited(TypedValue value) {
     ProcessEngineConfigurationImpl processEngineConfiguration =
         Context.getProcessEngineConfiguration();
 
@@ -50,8 +57,6 @@ public static void checkJavaSerialization(String variableName, TypedValue value)
 
       // if Java serialization is prohibited
       if (!serializableValue.isDeserialized()) {
-
-        String javaSerializationDataFormat = Variables.SerializationDataFormats.JAVA.getName();
         String requestedDataFormat = serializableValue.getSerializationDataFormat();
 
         if (requestedDataFormat == null) {
@@ -66,11 +71,17 @@ public static void checkJavaSerialization(String variableName, TypedValue value)
           }
         }
 
-        if (javaSerializationDataFormat.equals(requestedDataFormat)) {
-          throw CORE_LOGGER.javaSerializationProhibitedException(variableName);
-        }
+        return Variables.SerializationDataFormats.JAVA.getName().equals(requestedDataFormat);
       }
     }
+    return false;
+  }
+
+  public static void checkJavaSerialization(String variableName, TypedValue value) {
+    if (isJavaSerializationProhibited(value)) {
+      String message = MessageFormat.format(ERROR_MSG, variableName);
+      throw CORE_LOGGER.javaSerializationProhibitedException(message);
+    }
   }
 
   public static void setVariables(Map<String, ?> variables,
@@ -91,6 +102,20 @@ public static void setVariables(Map<String, ?> variables,
     }
   }
 
+  public static Map<String, ?> findBatchVariablesSerialized(String batchId, CommandContext commandContext) {
+    List<VariableInstanceEntity> variableInstances = commandContext.getVariableInstanceManager()
+        .findVariableInstancesByBatchId(batchId);
+    return variableInstances.stream().collect(variablesCollector());
+  }
+
+  protected static Collector<VariableInstanceEntity, ?, Map<String, TypedValue>> variablesCollector() {
+    return Collectors.toMap(VariableInstanceEntity::getName, VariableUtil::getSerializedValue);
+  }
+
+  protected static TypedValue getSerializedValue(VariableInstanceEntity variableInstanceEntity) {
+    return variableInstanceEntity.getTypedValue(false);
+  }
+
   @FunctionalInterface
   public interface SetVariableFunction {
     void apply(String variableName, Object variableValue);

diff --git a/.../src/main/java/org/camunda/bpm/engine/impl/core/variable/scope/AbstractVariableScope.java b/.../src/main/java/org/camunda/bpm/engine/impl/core/variable/scope/AbstractVariableScope.java
@@ -240,12 +240,22 @@ public Set<String> getVariableNamesLocal() {
     return getVariableStore().getKeys();
   }
 
+  public void setVariables(Map<String, ?> variables, boolean skipJavaSerializationFormatCheck) {
+    VariableUtil.setVariables(variables,
+        (name, value) -> setVariable(name, value, skipJavaSerializationFormatCheck));
+  }
+
   public void setVariables(Map<String, ?> variables) {
-    VariableUtil.setVariables(variables, this::setVariable);
+    setVariables(variables, false);
+  }
+
+  public void setVariablesLocal(Map<String, ?> variables, boolean skipJavaSerializationFormatCheck) {
+    VariableUtil.setVariables(variables,
+        (name, value) -> setVariableLocal(name, value, skipJavaSerializationFormatCheck));
   }
 
   public void setVariablesLocal(Map<String, ?> variables) {
-    VariableUtil.setVariables(variables, this::setVariableLocal);
+    setVariablesLocal(variables, false);
   }
 
   public void removeVariables() {
@@ -279,33 +289,50 @@ public void removeVariablesLocal(Collection<String> variableNames) {
     }
   }
 
-  public void setVariable(String variableName, Object value) {
+  public void setVariable(String variableName, Object value, boolean skipJavaSerializationFormatCheck) {
     TypedValue typedValue = Variables.untypedValue(value);
-    setVariable(variableName, typedValue, getSourceActivityVariableScope());
+    setVariable(variableName, typedValue, getSourceActivityVariableScope(), skipJavaSerializationFormatCheck);
+  }
 
+  public void setVariable(String variableName, Object value) {
+    setVariable(variableName, value, false);
   }
 
-  protected void setVariable(String variableName, TypedValue value, AbstractVariableScope sourceActivityVariableScope) {
+  protected void setVariable(String variableName,
+                             TypedValue value,
+                             AbstractVariableScope sourceActivityVariableScope,
+                             boolean skipJavaSerializationFormatCheck) {
     if (hasVariableLocal(variableName)) {
-      setVariableLocal(variableName, value, sourceActivityVariableScope);
+      setVariableLocal(variableName, value, sourceActivityVariableScope, skipJavaSerializationFormatCheck);
       return;
     }
     AbstractVariableScope parentVariableScope = getParentVariableScope();
     if (parentVariableScope!=null) {
       if (sourceActivityVariableScope==null) {
-        parentVariableScope.setVariable(variableName, value);
+        parentVariableScope.setVariable(variableName, value, skipJavaSerializationFormatCheck);
       } else {
-        parentVariableScope.setVariable(variableName, value, sourceActivityVariableScope);
+        parentVariableScope.setVariable(variableName, value, sourceActivityVariableScope, skipJavaSerializationFormatCheck);
       }
       return;
     }
 
-    setVariableLocal(variableName, value, sourceActivityVariableScope);
+    setVariableLocal(variableName, value, sourceActivityVariableScope, skipJavaSerializationFormatCheck);
   }
 
-  public void setVariableLocal(String variableName, TypedValue value, AbstractVariableScope sourceActivityExecution) {
+  protected void setVariable(String variableName,
+                             TypedValue value,
+                             AbstractVariableScope sourceActivityVariableScope) {
+    setVariable(variableName, value, sourceActivityVariableScope, false);
+  }
+
+  public void setVariableLocal(String variableName,
+                               TypedValue value,
+                               AbstractVariableScope sourceActivityExecution,
+                               boolean skipJavaSerializationFormatCheck) {
 
-    VariableUtil.checkJavaSerialization(variableName, value);
+    if (!skipJavaSerializationFormatCheck) {
+      VariableUtil.checkJavaSerialization(variableName, value);
+    }
 
     VariableStore<CoreVariableInstance> variableStore = getVariableStore();
 
@@ -372,10 +399,13 @@ protected void invokeVariableLifecycleListenersUpdate(CoreVariableInstance varia
     }
   }
 
-  public void setVariableLocal(String variableName, Object value) {
+  public void setVariableLocal(String variableName, Object value, boolean skipJavaSerializationFormatCheck) {
     TypedValue typedValue = Variables.untypedValue(value);
-    setVariableLocal(variableName, typedValue, getSourceActivityVariableScope());
+    setVariableLocal(variableName, typedValue, getSourceActivityVariableScope(), skipJavaSerializationFormatCheck);
+  }
 
+  public void setVariableLocal(String variableName, Object value) {
+    setVariableLocal(variableName, value, false);
   }
 
   public void removeVariable(String variableName) {

diff --git a/engine/src/main/java/org/camunda/bpm/engine/impl/persistence/entity/TaskEntity.java b/engine/src/main/java/org/camunda/bpm/engine/impl/persistence/entity/TaskEntity.java
@@ -1658,8 +1658,8 @@ public void addIdentityLinkChanges(String type, String oldProperty, String newPr
   }
 
   @Override
-  public void setVariablesLocal(Map<String, ?> variables) {
-    super.setVariablesLocal(variables);
+  public void setVariablesLocal(Map<String, ?> variables, boolean skipJavaSerializationFormatCheck) {
+    super.setVariablesLocal(variables, skipJavaSerializationFormatCheck);
     Context.getCommandContext().getDbEntityManager().forceUpdate(this);
   }
 

diff --git a/.../src/main/java/org/camunda/bpm/engine/impl/persistence/entity/VariableInstanceEntity.java b/.../src/main/java/org/camunda/bpm/engine/impl/persistence/entity/VariableInstanceEntity.java
@@ -274,12 +274,8 @@ public TypedValue getTypedValue() {
     return typedValueField.getTypedValue(isTransient);
   }
 
-  public TypedValue getTypedValueWithImplicitUpdatesSkipped() {
-    return typedValueField.getTypedValueWithImplicitUpdatesSkipped(isTransient);
-  }
-
   public TypedValue getTypedValue(boolean deserializeValue) {
-    return typedValueField.getTypedValue(deserializeValue, isTransient, false);
+    return typedValueField.getTypedValue(deserializeValue, isTransient);
   }
 
   public void setValue(TypedValue value) {

diff --git a/...ne/src/main/java/org/camunda/bpm/engine/impl/persistence/entity/util/TypedValueField.java b/...ne/src/main/java/org/camunda/bpm/engine/impl/persistence/entity/util/TypedValueField.java
@@ -79,20 +79,10 @@ public Object getValue() {
   }
 
   public TypedValue getTypedValue(boolean asTransientValue) {
-    return getTypedValue(true, asTransientValue, false);
-  }
-
-  public TypedValue getTypedValueWithImplicitUpdatesSkipped(boolean asTransientValue) {
-    return getTypedValue(true, asTransientValue, true);
+    return getTypedValue(true, asTransientValue);
   }
 
   public TypedValue getTypedValue(boolean deserializeValue, boolean asTransientValue) {
-    return getTypedValue(deserializeValue, asTransientValue, false);
-  }
-
-  public TypedValue getTypedValue(boolean deserializeValue,
-                                  boolean asTransientValue,
-                                  boolean skipImplicitUpdates) {
     if (Context.getCommandContext() != null) {
       // in some circumstances we must invalidate the cached value instead of returning it
 
@@ -114,7 +104,7 @@ public TypedValue getTypedValue(boolean deserializeValue,
       try {
         cachedValue = getSerializer().readValue(valueFields, deserializeValue, asTransientValue);
 
-        if (!skipImplicitUpdates && notifyOnImplicitUpdates && isMutableValue(cachedValue)) {
+        if (notifyOnImplicitUpdates && isMutableValue(cachedValue)) {
           Context.getCommandContext().registerCommandContextListener(this);
         }
 

diff --git a/...org/camunda/bpm/integrationtest/functional/migration/MigrationContextSwitchBeansTest.java b/...org/camunda/bpm/integrationtest/functional/migration/MigrationContextSwitchBeansTest.java
@@ -85,14 +85,6 @@ public static WebArchive clientDeployment() {
 
   }
 
-  protected static Asset modelAsAsset(BpmnModelInstance modelInstance) {
-    ByteArrayOutputStream byteStream = new ByteArrayOutputStream();
-    Bpmn.writeModelToStream(byteStream, modelInstance);
-
-    byte[] bytes = byteStream.toByteArray();
-    return new ByteArrayAsset(bytes);
-  }
-
   @Test
   @OperateOnDeployment("clientDeployment")
   public void testCreateBoundaryTimer() {
@@ -124,5 +116,12 @@ public void testCreateBoundaryTimer() {
     Assert.assertNotNull(timerJob.getDuedate());
   }
 
+  protected static Asset modelAsAsset(BpmnModelInstance modelInstance) {
+    ByteArrayOutputStream byteStream = new ByteArrayOutputStream();
+    Bpmn.writeModelToStream(byteStream, modelInstance);
+
+    byte[] bytes = byteStream.toByteArray();
+    return new ByteArrayAsset(bytes);
+  }
 
 }