fix switch condition (#15228)

Co-authored-by: Eric Gao <ericgao.apache@gmail.com>

dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/runner/task/switchtask/SwitchLogicTask.java

@@ -26,7 +26,6 @@ import org.apache.dolphinscheduler.plugin.task.api.enums.TaskExecutionStatus;
 import org.apache.dolphinscheduler.plugin.task.api.model.Property;
 import org.apache.dolphinscheduler.plugin.task.api.model.SwitchResultVo;
 import org.apache.dolphinscheduler.plugin.task.api.parameters.SwitchParameters;
-import org.apache.dolphinscheduler.plugin.task.api.utils.ParameterUtils;
 import org.apache.dolphinscheduler.server.master.cache.ProcessInstanceExecCacheManager;
 import org.apache.dolphinscheduler.server.master.exception.LogicTaskInitializeException;
 import org.apache.dolphinscheduler.server.master.exception.MasterTaskExecuteException;
@@ -39,8 +38,6 @@ import org.apache.commons.lang3.StringUtils;
 
 import java.util.List;
 import java.util.Map;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 
 import lombok.extern.slf4j.Slf4j;
@@ -50,8 +47,6 @@ public class SwitchLogicTask extends BaseSyncLogicTask<SwitchParameters> {
 
     public static final String TASK_TYPE = "SWITCH";
 
-    private static final String rgex = "['\"]*\\$\\{(.*?)\\}['\"]*";
-
     private final ProcessInstance processInstance;
     private final TaskInstance taskInstance;
 
@@ -94,13 +89,24 @@ public class SwitchLogicTask extends BaseSyncLogicTask<SwitchParameters> {
         // todo: refactor these calculate code
         int finalConditionLocation = switchResultVos.size() - 1;
         int i = 0;
+
+        Map<String, Property> globalParams = JSONUtils
+                .toList(processInstance.getGlobalParams(), Property.class)
+                .stream()
+                .collect(Collectors.toMap(Property::getProp, Property -> Property));
+        Map<String, Property> varParams = JSONUtils
+                .toList(taskInstance.getVarPool(), Property.class)
+                .stream()
+                .collect(Collectors.toMap(Property::getProp, Property -> Property));
+
         for (SwitchResultVo info : switchResultVos) {
             log.info("Begin to execute {} condition: {} ", (i + 1), info.getCondition());
             if (StringUtils.isEmpty(info.getCondition())) {
                 finalConditionLocation = i;
                 break;
             }
-            String content = setTaskParams(info.getCondition().replaceAll("'", "\""), rgex);
+            String content =
+                    SwitchTaskUtils.generateContentWithTaskParams(info.getCondition(), globalParams, varParams);
             log.info("Format condition sentence::{} successfully", content);
             Boolean result;
             try {
@@ -131,39 +137,6 @@ public class SwitchLogicTask extends BaseSyncLogicTask<SwitchParameters> {
         return conditionResult;
     }
 
-    public String setTaskParams(String content, String rgex) {
-        Pattern pattern = Pattern.compile(rgex);
-        Matcher m = pattern.matcher(content);
-        Map<String, Property> globalParams = JSONUtils
-                .toList(processInstance.getGlobalParams(), Property.class)
-                .stream()
-                .collect(Collectors.toMap(Property::getProp, Property -> Property));
-        Map<String, Property> varParams = JSONUtils
-                .toList(taskInstance.getVarPool(), Property.class)
-                .stream()
-                .collect(Collectors.toMap(Property::getProp, Property -> Property));
-        if (varParams.size() > 0) {
-            varParams.putAll(globalParams);
-            globalParams = varParams;
-        }
-        while (m.find()) {
-            String paramName = m.group(1);
-            Property property = globalParams.get(paramName);
-            if (property == null) {
-                return "";
-            }
-            String value;
-            if (ParameterUtils.isNumber(property) || ParameterUtils.isBoolean(property)) {
-                value = "" + ParameterUtils.getParameterValue(property);
-            } else {
-                value = "\"" + ParameterUtils.getParameterValue(property) + "\"";
-            }
-            log.info("paramName:{}，paramValue:{}", paramName, value);
-            content = content.replace("${" + paramName + "}", value);
-        }
-        return content;
-    }
-
     private boolean isValidSwitchResult(SwitchResultVo switchResult) {
         if (CollectionUtils.isEmpty(switchResult.getNextNode())) {
             return false;

dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/utils/SwitchTaskUtils.java

@@ -17,14 +17,29 @@
 
 package org.apache.dolphinscheduler.server.master.utils;
 
+import org.apache.dolphinscheduler.plugin.task.api.model.Property;
+import org.apache.dolphinscheduler.plugin.task.api.utils.ParameterUtils;
+
+import org.apache.commons.collections4.MapUtils;
+
+import java.util.Map;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
 import javax.script.ScriptEngine;
 import javax.script.ScriptEngineManager;
 import javax.script.ScriptException;
 
+import lombok.extern.slf4j.Slf4j;
+
+import com.google.common.collect.Maps;
+
+@Slf4j
 public class SwitchTaskUtils {
 
     private static final ScriptEngineManager manager;
     private static final ScriptEngine engine;
+    private static final String rgex = "['\"]*\\$\\{(.*?)\\}['\"]*";
 
     static {
         manager = new ScriptEngineManager();
@@ -36,4 +51,43 @@ public class SwitchTaskUtils {
         return Boolean.TRUE.equals(result);
     }
 
+    public static String generateContentWithTaskParams(String condition, Map<String, Property> globalParams,
+                                                       Map<String, Property> varParams) {
+        String content = condition.replaceAll("'", "\"");
+        if (MapUtils.isEmpty(globalParams) && MapUtils.isEmpty(varParams)) {
+            throw new IllegalArgumentException("globalParams and varParams are both empty, please check it.");
+        }
+        Map<String, Property> params = Maps.newHashMap();
+        if (MapUtils.isNotEmpty(globalParams)) {
+            params.putAll(globalParams);
+        }
+        if (MapUtils.isNotEmpty(varParams)) {
+            params.putAll(varParams);
+        }
+        String originContent = content;
+        Pattern pattern = Pattern.compile(rgex);
+        Matcher m = pattern.matcher(content);
+        while (m.find()) {
+            String paramName = m.group(1);
+            Property property = params.get(paramName);
+            if (property == null) {
+                continue;
+            }
+            String value;
+            if (ParameterUtils.isNumber(property) || ParameterUtils.isBoolean(property)) {
+                value = "" + ParameterUtils.getParameterValue(property);
+            } else {
+                value = "\"" + ParameterUtils.getParameterValue(property) + "\"";
+            }
+            log.info("paramName:{}，paramValue:{}", paramName, value);
+            content = content.replace("${" + paramName + "}", value);
+        }
+
+        // if not replace any params, throw exception to avoid illegal condition
+        if (originContent.equals(content)) {
+            throw new IllegalArgumentException("condition is not valid, please check it. condition: " + condition);
+        }
+        return content;
+    }
+
 }

dolphinscheduler-master/src/test/java/org/apache/dolphinscheduler/server/master/utils/SwitchTaskUtilsTest.java

@@ -0,0 +1,56 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.dolphinscheduler.server.master.utils;
+
+import org.apache.dolphinscheduler.plugin.task.api.enums.DataType;
+import org.apache.dolphinscheduler.plugin.task.api.enums.Direct;
+import org.apache.dolphinscheduler.plugin.task.api.model.Property;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+public class SwitchTaskUtilsTest {
+
+    @Test
+    public void testGenerateContentWithTaskParams() {
+        String content = "${test}==1";
+        Map<String, Property> globalParams = new HashMap<>();
+        Map<String, Property> varParams = new HashMap<>();
+        Assertions.assertThrowsExactly(IllegalArgumentException.class, () -> {
+            SwitchTaskUtils.generateContentWithTaskParams(content, globalParams, varParams);
+        });
+
+        globalParams.put("test", new Property("test", Direct.IN, DataType.INTEGER, "1"));
+        String result = SwitchTaskUtils.generateContentWithTaskParams(content, globalParams, varParams);
+        Assertions.assertEquals("1==1", result);
+    }
+
+    @Test
+    public void testIllegalCondition() {
+        String content = "java.lang.Runtime.getRuntime().exec(\"bash /tmp/shell\")";
+        Map<String, Property> globalParams = new HashMap<>();
+        Map<String, Property> varParams = new HashMap<>();
+        globalParams.put("test", new Property("test", Direct.IN, DataType.INTEGER, "1"));
+        Assertions.assertThrowsExactly(IllegalArgumentException.class, () -> {
+            SwitchTaskUtils.generateContentWithTaskParams(content, globalParams, varParams);
+        });
+    }
+}