This is an automated email from the ASF dual-hosted git repository.
xxyu pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/kylin.git
The following commit(s) were added to refs/heads/main by this push:
new 737f79e fix
737f79e is described below
commit 737f79edc0ef73ab780355f2a54f5898e5809ab3
Author: yaqian.zhang <598593...@qq.com>
AuthorDate: Tue Feb 8 11:27:34 2022 +0800
fix
---
.../org/apache/kylin/common/KylinConfigBase.java | 3 +-
.../kylin/common/util/CliCommandExecutor.java | 46 ------------
.../apache/kylin/common/util/ParameterFilter.java | 81 ++++++++++++++++++++++
...dExecutorTest.java => ParameterFilterTest.java} | 30 ++++++--
.../kylin/engine/spark/job/NSparkExecutable.java | 14 +++-
.../kylin/rest/controller/JobController.java | 6 +-
.../kylin/rest/controller/ProjectController.java | 2 +-
.../org/apache/kylin/rest/service/CubeService.java | 7 +-
8 files changed, 128 insertions(+), 61 deletions(-)
diff --git
a/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java
b/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java
index 10c6733..22e2d88 100644
--- a/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java
+++ b/core-common/src/main/java/org/apache/kylin/common/KylinConfigBase.java
@@ -46,6 +46,7 @@ import org.apache.kylin.common.util.ClassUtil;
import org.apache.kylin.common.util.CliCommandExecutor;
import org.apache.kylin.common.util.FileUtils;
import org.apache.kylin.common.util.HadoopUtil;
+import org.apache.kylin.common.util.ParameterFilter;
import org.apache.kylin.common.util.StringUtil;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -1113,7 +1114,7 @@ public abstract class KylinConfigBase implements
Serializable {
}
public String getHiveDatabaseForIntermediateTable() {
- return
CliCommandExecutor.checkHiveProperty(this.getOptional("kylin.source.hive.database-for-flat-table",
DEFAULT));
+ return
ParameterFilter.checkHiveProperty(this.getOptional("kylin.source.hive.database-for-flat-table",
DEFAULT));
}
public String getFlatTableStorageFormat() {
diff --git
a/core-common/src/main/java/org/apache/kylin/common/util/CliCommandExecutor.java
b/core-common/src/main/java/org/apache/kylin/common/util/CliCommandExecutor.java
index 54bab60..83f30ab 100644
---
a/core-common/src/main/java/org/apache/kylin/common/util/CliCommandExecutor.java
+++
b/core-common/src/main/java/org/apache/kylin/common/util/CliCommandExecutor.java
@@ -175,50 +175,4 @@ public class CliCommandExecutor {
}
}
}
-
- public static final String COMMAND_BLOCK_LIST = "[
&`>|{}()$;\\-#~!+*\\\\]+";
- public static final String COMMAND_WHITE_LIST = "[^\\w%,@/:=?.\"\\[\\]]";
- public static final String HIVE_BLOCK_LIST = "[ <>()$;\\-#!+*\"'/=%@]+";
-
-
- /**
- * <pre>
- * Check parameter for preventing command injection, replace illegal
character into empty character.
- *
- * Note:
- * 1. Whitespace is also refused because parameter is a single word,
should not contains it
- * 2. Some character may be illegal but still be accepted because
commandParameter maybe a URI/path expression,
- * you may check "Character part" in
https://docs.oracle.com/javase/8/docs/api/java/net/URI.html,
- * here is the character which is not banned.
- *
- * 1. dot .
- * 2. slash /
- * 3. colon :
- * 4. equal =
- * 5. ?
- * 6. @
- * 7. bracket []
- * 8. comma ,
- * 9. %
- * </pre>
- */
- public static String checkParameter(String commandParameter) {
- return checkParameter(commandParameter, COMMAND_BLOCK_LIST);
- }
-
- public static String checkParameterWhiteList(String commandParameter) {
- return checkParameter(commandParameter, COMMAND_WHITE_LIST);
- }
-
- public static String checkHiveProperty(String hiveProperty) {
- return checkParameter(hiveProperty, HIVE_BLOCK_LIST);
- }
-
- private static String checkParameter(String commandParameter, String rex) {
- String repaired = commandParameter.replaceAll(rex, "");
- if (repaired.length() != commandParameter.length()) {
- logger.warn("Detected illegal character in command {} by {} ,
replace it to {}.", commandParameter, rex, repaired);
- }
- return repaired;
- }
}
diff --git
a/core-common/src/main/java/org/apache/kylin/common/util/ParameterFilter.java
b/core-common/src/main/java/org/apache/kylin/common/util/ParameterFilter.java
new file mode 100644
index 0000000..2b9e405
--- /dev/null
+++
b/core-common/src/main/java/org/apache/kylin/common/util/ParameterFilter.java
@@ -0,0 +1,81 @@
+/*
+ * 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.kylin.common.util;
+
+import org.slf4j.LoggerFactory;
+import org.slf4j.Logger;
+
+public class ParameterFilter {
+ private static final Logger logger =
LoggerFactory.getLogger(ParameterFilter.class);
+
+ public static final String PARAMETER_REGULAR_EXPRESSION = "[
&`>|{}()$;\\-#~!+*\\\\]+";
+ public static final String URI_REGULAR_EXPRESSION =
"[^\\w%,@/:=?.\"\\[\\]]";
+ public static final String HIVE_PROPERTY_REGULAR_EXPRESSION = "[
<>()$;\\-#!+*\"'/=%@]+";
+ public static final String SPARK_CONF_REGULAR_EXPRESSION = "[`$|&;]+";
+
+ /**
+ * <pre>
+ * Check parameter for preventing command injection, replace illegal
character into empty character.
+ *
+ * Note:
+ * 1. Whitespace is also refused because parameter is a single word,
should not contains it
+ * 2. Some character may be illegal but still be accepted because
commandParameter maybe a URI/path expression,
+ * you may check "Character part" in
https://docs.oracle.com/javase/8/docs/api/java/net/URI.html,
+ * here is the character which is not banned.
+ *
+ * 1. dot .
+ * 2. slash /
+ * 3. colon :
+ * 4. equal =
+ * 5. ?
+ * 6. @
+ * 7. bracket []
+ * 8. comma ,
+ * 9. %
+ * </pre>
+ */
+ public static String checkParameter(String commandParameter) {
+ return checkParameter(commandParameter, PARAMETER_REGULAR_EXPRESSION,
false);
+ }
+
+ public static String checkURI(String commandParameter) {
+ return checkParameter(commandParameter, URI_REGULAR_EXPRESSION, false);
+ }
+
+ public static String checkHiveProperty(String hiveProperty) {
+ return checkParameter(hiveProperty, HIVE_PROPERTY_REGULAR_EXPRESSION,
false);
+ }
+
+ public static String checkSparkConf(String sparkConf) {
+ return checkParameter(sparkConf, SPARK_CONF_REGULAR_EXPRESSION, true);
+ }
+
+ private static String checkParameter(String commandParameter, String rex,
boolean throwException) {
+ String repaired = commandParameter.replaceAll(rex, "");
+ if (repaired.length() != commandParameter.length()) {
+ if (throwException) {
+ throw new IllegalArgumentException("Detected illegal character
in " + commandParameter + " by " + rex);
+ } else {
+ logger.warn("Detected illegal character in command {} by {} ,
replace it to {}.",
+ commandParameter, rex, repaired);
+ }
+ }
+ return repaired;
+ }
+}
diff --git
a/core-common/src/test/java/org/apache/kylin/common/util/CliCommandExecutorTest.java
b/core-common/src/test/java/org/apache/kylin/common/util/ParameterFilterTest.java
similarity index 69%
rename from
core-common/src/test/java/org/apache/kylin/common/util/CliCommandExecutorTest.java
rename to
core-common/src/test/java/org/apache/kylin/common/util/ParameterFilterTest.java
index 18aea77..192f477 100644
---
a/core-common/src/test/java/org/apache/kylin/common/util/CliCommandExecutorTest.java
+++
b/core-common/src/test/java/org/apache/kylin/common/util/ParameterFilterTest.java
@@ -17,11 +17,12 @@
*/
package org.apache.kylin.common.util;
+import org.junit.Assert;
import org.junit.Test;
import static org.junit.Assert.assertEquals;
-public class CliCommandExecutorTest {
+public class ParameterFilterTest {
private String[][] commands = {
{"nslookup unknown.com &", "nslookupunknown.com"},
@@ -41,24 +42,41 @@ public class CliCommandExecutorTest {
{"db and 1=2", "dband12"}
};
+ private String[] sparkConf = {"kylin.engine.spark-conf.'`touch
/tmp/test`'", "'$(touch /tmp/test)'",
+ "'|touch /tmp/test|'", "';touch /tmp/test;'", "'&touch /tmp/test&'",
"'$(|;&touch /tmp/test&;|)'", "default"};
+
@Test
- public void testCmd() {
+ public void testParameter() {
for (String[] pair : commands) {
- assertEquals(pair[1], CliCommandExecutor.checkParameter(pair[0]));
+ assertEquals(pair[1], ParameterFilter.checkParameter(pair[0]));
}
}
@Test
- public void testCmd2() {
+ public void testURI() {
for (String[] pair : commands) {
- assertEquals(pair[1],
CliCommandExecutor.checkParameterWhiteList(pair[0]));
+ assertEquals(pair[1], ParameterFilter.checkURI(pair[0]));
}
}
@Test
public void testHiveProperties() {
for (String[] pair : properties) {
- assertEquals(pair[1],
CliCommandExecutor.checkHiveProperty(pair[0]));
+ assertEquals(pair[1], ParameterFilter.checkHiveProperty(pair[0]));
+ }
+ }
+
+ @Test
+ public void testSparkConf() {
+ int exceptionNum = 0;
+ for(String conf : sparkConf) {
+ try {
+ ParameterFilter.checkSparkConf(conf);
+ } catch (Exception exception) {
+ Assert.assertTrue(exception instanceof
IllegalArgumentException);
+ exceptionNum++;
+ }
}
+ assertEquals(6, exceptionNum);
}
}
diff --git
a/kylin-spark-project/kylin-spark-engine/src/main/java/org/apache/kylin/engine/spark/job/NSparkExecutable.java
b/kylin-spark-project/kylin-spark-engine/src/main/java/org/apache/kylin/engine/spark/job/NSparkExecutable.java
index 86e234a..eef9ca8 100644
---
a/kylin-spark-project/kylin-spark-engine/src/main/java/org/apache/kylin/engine/spark/job/NSparkExecutable.java
+++
b/kylin-spark-project/kylin-spark-engine/src/main/java/org/apache/kylin/engine/spark/job/NSparkExecutable.java
@@ -27,6 +27,7 @@ import java.nio.file.Paths;
import java.util.Collections;
import java.util.EnumSet;
import java.util.HashMap;
+import java.util.LinkedHashMap;
import java.util.List;
import java.util.Locale;
import java.util.Map;
@@ -34,6 +35,7 @@ import java.util.Set;
import java.util.Objects;
import java.util.Map.Entry;
+import org.apache.kylin.common.util.ParameterFilter;
import org.apache.kylin.cube.CubeInstance;
import org.apache.kylin.cube.CubeManager;
import org.apache.kylin.engine.spark.utils.MetaDumpUtil;
@@ -110,10 +112,20 @@ public class NSparkExecutable extends AbstractExecutable {
@Override
protected ExecuteResult doWork(ExecutableContext context) throws
ExecuteException {
- //context.setLogPath(getSparkDriverLogHdfsPath(context.getConfig()));
CubeManager cubeMgr =
CubeManager.getInstance(KylinConfig.getInstanceFromEnv());
CubeInstance cube = cubeMgr.getCube(this.getCubeName());
KylinConfig config = cube.getConfig();
+
+ Map<String, String> overrideKylinProps = new HashMap<>();
+ LinkedHashMap<String, String> cubeConfig =
cube.getDescriptor().getOverrideKylinProps();
+ LinkedHashMap<String, String> projectConfig =
cube.getProjectInstance().getOverrideKylinProps();
+ overrideKylinProps.putAll(projectConfig);
+ overrideKylinProps.putAll(cubeConfig);
+ for (Map.Entry<String, String> configEntry :
overrideKylinProps.entrySet()) {
+ ParameterFilter.checkSparkConf(configEntry.getKey());
+ ParameterFilter.checkSparkConf(configEntry.getValue());
+ }
+
this.setLogPath(getSparkDriverLogHdfsPath(context.getConfig()));
config = wrapConfig(config);
diff --git
a/server-base/src/main/java/org/apache/kylin/rest/controller/JobController.java
b/server-base/src/main/java/org/apache/kylin/rest/controller/JobController.java
index 5c60d38..e713eff 100644
---
a/server-base/src/main/java/org/apache/kylin/rest/controller/JobController.java
+++
b/server-base/src/main/java/org/apache/kylin/rest/controller/JobController.java
@@ -27,8 +27,8 @@ import java.util.List;
import java.util.Map;
import java.util.Locale;
-import org.apache.kylin.common.util.CliCommandExecutor;
import org.apache.kylin.common.KylinConfig;
+import org.apache.kylin.common.util.ParameterFilter;
import org.apache.kylin.job.JobInstance;
import org.apache.kylin.job.constant.JobStatusEnum;
import org.apache.kylin.job.constant.JobTimeFilterEnum;
@@ -195,8 +195,8 @@ public class JobController extends BasicController {
checkRequiredArg("job_id", jobId);
checkRequiredArg("step_id", stepId);
checkRequiredArg("project", project);
- String validatedPrj = CliCommandExecutor.checkParameter(project);
- String validatedStepId = CliCommandExecutor.checkParameter(stepId);
+ String validatedPrj = ParameterFilter.checkParameter(project);
+ String validatedStepId = ParameterFilter.checkParameter(stepId);
String downloadFilename = String.format(Locale.ROOT, "%s_%s.log",
validatedPrj, validatedStepId);
String jobOutput = jobService.getAllJobStepOutput(jobId, stepId);
diff --git
a/server-base/src/main/java/org/apache/kylin/rest/controller/ProjectController.java
b/server-base/src/main/java/org/apache/kylin/rest/controller/ProjectController.java
index 07a70bd..26d9391 100644
---
a/server-base/src/main/java/org/apache/kylin/rest/controller/ProjectController.java
+++
b/server-base/src/main/java/org/apache/kylin/rest/controller/ProjectController.java
@@ -89,7 +89,7 @@ public class ProjectController extends BasicController {
@RequestMapping(value = "", method = { RequestMethod.GET }, produces = {
"application/json" })
@ResponseBody
public List<ProjectInstance> getProjects(@RequestParam(value = "limit",
required = false) Integer limit,
- @RequestParam(value = "offset", required = false) Integer offset) {
+ @RequestParam(value = "offset",
required = false) Integer offset) {
return projectService.listProjects(limit, offset);
}
diff --git
a/server-base/src/main/java/org/apache/kylin/rest/service/CubeService.java
b/server-base/src/main/java/org/apache/kylin/rest/service/CubeService.java
index 4fdbf1f..bfd428f 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/service/CubeService.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/service/CubeService.java
@@ -35,6 +35,7 @@ import org.apache.kylin.common.lock.DistributedLock;
import org.apache.kylin.common.persistence.RootPersistentEntity;
import org.apache.kylin.common.util.CliCommandExecutor;
import org.apache.kylin.common.util.Pair;
+import org.apache.kylin.common.util.ParameterFilter;
import org.apache.kylin.cube.CubeInstance;
import org.apache.kylin.cube.CubeManager;
import org.apache.kylin.cube.CubeSegment;
@@ -1152,10 +1153,10 @@ public class CubeService extends BasicService
implements InitializingBean {
String cmd = String.format(Locale.ROOT,
stringBuilder,
KylinConfig.getKylinHome(),
- CliCommandExecutor.checkParameterWhiteList(srcCfgUri),
- CliCommandExecutor.checkParameterWhiteList(dstCfgUri),
+ ParameterFilter.checkURI(srcCfgUri),
+ ParameterFilter.checkURI(dstCfgUri),
cube.getName(),
- CliCommandExecutor.checkParameterWhiteList(projectName),
+ ParameterFilter.checkParameter(projectName),
config.isAutoMigrateCubeCopyAcl(),
config.isAutoMigrateCubePurge());
