fix switch condition (#15228) (#15336)

Author: caishunfeng

Diff for: dolphinscheduler-master/src/main/java/org/apache/dolphinscheduler/server/master/runner/task/SwitchTaskProcessor.java

@@ -37,8 +37,6 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 
 import com.google.auto.service.AutoService;
@@ -140,6 +138,16 @@ private boolean setSwitchResult() {
         switchResultVos.add(switchResultVo);
         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));
+
         conditionResult = DependResult.SUCCESS;
         for (SwitchResultVo info : switchResultVos) {
             logger.info("the {} execution ", (i + 1));
@@ -148,7 +156,8 @@ private boolean setSwitchResult() {
                 finalConditionLocation = i;
                 break;
             }
-            String content = setTaskParams(info.getCondition().replaceAll("'", "\""), rgex);
+            String content =
+                    SwitchTaskUtils.generateContentWithTaskParams(info.getCondition(), globalParams, varParams);
             logger.info("format condition sentence::{}", content);
             Boolean result = null;
             try {
@@ -191,37 +200,6 @@ private void endTaskState() {
         processService.updateTaskInstance(taskInstance);
     }
 
-    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 = property.getValue();
-            if (!org.apache.commons.lang3.math.NumberUtils.isCreatable(value)) {
-                value = "\"" + value + "\"";
-            }
-            logger.info("paramName:{}，paramValue:{}", paramName, value);
-            content = content.replace("${" + paramName + "}", value);
-        }
-        return content;
-    }
-
     /**
      * check whether switch result is valid
      */

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

@@ -17,13 +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 ScriptEngineManager manager;
     private static ScriptEngine engine;
+    private static final String rgex = "['\"]*\\$\\{(.*?)\\}['\"]*";
 
     static {
         manager = new ScriptEngineManager();
@@ -35,4 +51,42 @@ public static boolean evaluate(String expression) throws ScriptException {
         return (Boolean) 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;
+    }
+}

Diff for: 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);
+        });
+    }
+}

Diff for: dolphinscheduler-task-plugin/dolphinscheduler-task-api/src/main/java/org/apache/dolphinscheduler/plugin/task/api/utils/ParameterUtils.java

@@ -19,12 +19,14 @@
 
 import org.apache.dolphinscheduler.common.constants.DateConstants;
 import org.apache.dolphinscheduler.common.utils.DateUtils;
+import org.apache.dolphinscheduler.plugin.task.api.enums.DataType;
 import org.apache.dolphinscheduler.plugin.task.api.model.Property;
 import org.apache.dolphinscheduler.plugin.task.api.parser.PlaceholderUtils;
 import org.apache.dolphinscheduler.plugin.task.api.parser.TimePlaceholderUtils;
 
 import org.apache.commons.lang3.StringUtils;
 
+import java.io.Serializable;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.Iterator;
@@ -130,4 +132,38 @@ private static String dateTemplateParse(String templateStr, Date date) {
         return newValue.toString();
     }
 
+    public static Serializable getParameterValue(Property property) {
+        if (property == null) {
+            return null;
+        }
+        String value = property.getValue();
+        switch (property.getType()) {
+            case LONG:
+                return Long.valueOf(value);
+            case FLOAT:
+                return Float.valueOf(value);
+            case INTEGER:
+                return Integer.valueOf(value);
+            case DOUBLE:
+                return Double.valueOf(value);
+            case BOOLEAN:
+                return Boolean.valueOf(value);
+            // todo: add date type, list type....
+            default:
+                return value;
+        }
+    }
+
+    public static boolean isNumber(Property property) {
+        return property != null &&
+                (DataType.INTEGER.equals(property.getType())
+                        || DataType.LONG.equals(property.getType())
+                        || DataType.FLOAT.equals(property.getType())
+                        || DataType.DOUBLE.equals(property.getType()));
+    }
+
+    public static boolean isBoolean(Property property) {
+        return property != null && DataType.BOOLEAN.equals(property.getType());
+    }
+
 }