You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kylin.apache.org by ya...@apache.org on 2021/11/03 02:20:49 UTC

[kylin] branch kylin3 updated: fix bug

This is an automated email from the ASF dual-hosted git repository.

yaqian pushed a commit to branch kylin3
in repository https://gitbox.apache.org/repos/asf/kylin.git


The following commit(s) were added to refs/heads/kylin3 by this push:
     new c559967  fix bug
c559967 is described below

commit c5599678f56e629b2243a9f48ad7617afb06e270
Author: yaqian.zhang <59...@qq.com>
AuthorDate: Thu Aug 26 17:12:40 2021 +0800

    fix bug
---
 .../apache/kylin/common/JDBCConnectionUtils.java   | 142 +++++++++++++++++++++
 .../org/apache/kylin/common/KylinConfigBase.java   |  18 ++-
 .../kylin/common/JDBCConnectionUtilsTest.java      | 142 +++++++++++++++++++++
 .../sdk/datasource/adaptor/AdaptorConfig.java      |   6 +-
 .../sdk/datasource/adaptor/AdaptorConfigTest.java  |  16 ++-
 .../org/apache/kylin/source/hive/DBConnConf.java   |   2 +
 6 files changed, 319 insertions(+), 7 deletions(-)

diff --git a/core-common/src/main/java/org/apache/kylin/common/JDBCConnectionUtils.java b/core-common/src/main/java/org/apache/kylin/common/JDBCConnectionUtils.java
new file mode 100644
index 0000000..feed7cd
--- /dev/null
+++ b/core-common/src/main/java/org/apache/kylin/common/JDBCConnectionUtils.java
@@ -0,0 +1,142 @@
+/*
+ * 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;
+
+import org.apache.kylin.common.util.StringUtil;
+import org.apache.kylin.shaded.com.google.common.base.Preconditions;
+
+public class JDBCConnectionUtils {
+    public static final String APPEND_PARAMS = "allowLoadLocalInfile=false&autoDeserialize=false&allowLocalInfile=false&allowUrlInLocalInfile=false";
+    public static final String HOST_NAME_WHITE_LIST = "[^-._a-zA-Z0-9]";
+    public static final String PORT_WHITE_LIST = "[^0-9]";
+    public static final String DATABASE_WHITE_LIST = "[^-._a-zA-Z0-9]";
+    public static final String PROPERTIES_VALUE_WHITE_LIST = "[^a-zA-Z0-9]";
+    public static final String ALLOWED_PROPERTIES = KylinConfig.getInstanceFromEnv().getJdbcUrlAllowedProperties();
+    public static final String MYSQL_PREFIX = "jdbc:mysql";
+    public static final String SQLSERVER_SCHEMA = KylinConfig.getInstanceFromEnv().getJdbcAllowedSqlServerSchema();
+    public static final String ALLOWED_SCHEMA = KylinConfig.getInstanceFromEnv().getJdbcAllowedSchema();
+    public static final String H2_MEM_PREFIX = "jdbc:h2:mem";
+    public static final String SERVER_INFO_FLAG = "//";
+    public static final String DATABASE_FLAG = "/";
+    public static final String PROPERTIES_FLAG = "?";
+    public static final String PROPERTIES_SEPARATOR = "&";
+    public static final String SQLSERVER_SEPARATOR = ";";
+
+    /*
+    Mysql jdbc connection url: jdbc:mysql://host:port/database
+    Postgresql jdbc connection url: jdbc:postgresql://host:port/database
+    Presto jdbc connection url: jdbc:presto://host:port/database
+    SQLServer jdbc connection url: jdbc:sqlserver://host:port;property=value;property=value
+    H2 jdbc connection url [For test]: jdbc:h2:mem:database
+     */
+    public static String checkUrl(String url) {
+        String[] urlInfo = StringUtil.split(url, ":");
+        String schema = null;
+        if (urlInfo.length == 4) {
+            schema = urlInfo[1];
+        } else if (urlInfo.length == 5) {
+            schema = urlInfo[1] + ":" + urlInfo[2];
+        } else {
+            throw new IllegalArgumentException("JDBC URL format does not conform to the specification, Please check it.");
+        }
+
+        checkJdbcSchema(schema);
+
+        if (url.startsWith(H2_MEM_PREFIX)) {
+            checkIllegalCharacter(urlInfo[3], DATABASE_WHITE_LIST);
+            return url;
+        }
+
+        String properties = null;
+        int databaseSeparatorIndex = url.lastIndexOf(DATABASE_FLAG);
+        if (SQLSERVER_SCHEMA.contains(schema)) {
+            databaseSeparatorIndex = url.indexOf(SQLSERVER_SEPARATOR);
+            properties = url.substring(databaseSeparatorIndex + 1);
+            checkJdbcProperties(properties, SQLSERVER_SEPARATOR);
+        }
+
+        String[] hostInfo = url.substring(url.indexOf(SERVER_INFO_FLAG) + 2, databaseSeparatorIndex).split(":");
+        checkIllegalCharacter(hostInfo[0], HOST_NAME_WHITE_LIST);
+        checkIllegalCharacter(hostInfo[1], PORT_WHITE_LIST);
+
+        String database = url.substring(databaseSeparatorIndex + 1);
+        int propertiesSeparatorIndex = url.indexOf(PROPERTIES_FLAG);
+        if (propertiesSeparatorIndex != -1) {
+            database = url.substring(databaseSeparatorIndex + 1, propertiesSeparatorIndex);
+            properties = url.substring(propertiesSeparatorIndex + 1);
+            checkJdbcProperties(properties, PROPERTIES_SEPARATOR);
+        }
+        if (!SQLSERVER_SCHEMA.contains(schema)) {
+            checkIllegalCharacter(database, DATABASE_WHITE_LIST);
+        }
+        if (url.startsWith(MYSQL_PREFIX)) {
+            if (!url.contains(PROPERTIES_FLAG)) {
+                url = url.concat(PROPERTIES_FLAG);
+            } else {
+                url = url.concat(PROPERTIES_SEPARATOR);
+            }
+            url = url.concat(APPEND_PARAMS);
+        }
+        return url;
+    }
+
+    private static void checkIllegalCharacter(String info, String whiteList) {
+        String repaired = info.replaceAll(whiteList, "");
+        if (repaired.length() != info.length()) {
+            throw new IllegalArgumentException("Detected illegal character in " + info + " by "
+                    + whiteList + ", jdbc url not allowed.");
+        }
+    }
+
+    private static void checkJdbcSchema(String jdbcSchema) {
+        if (!ALLOWED_SCHEMA.contains(jdbcSchema)) {
+            throw new IllegalArgumentException("The data source schema : " + jdbcSchema + " is not allowed. " +
+                    "You can add the schema to the allowed schema list by kylin.jdbc.url.allowed.additional.schema " +
+                    "and separate with commas.");
+        }
+    }
+
+    private static void checkJdbcProperties(String properties, String separator) {
+        if (properties != null) {
+            String[] propertiesInfo = StringUtil.split(properties, separator);
+            for (String property : propertiesInfo) {
+                if (property.isEmpty()) {
+                    continue;
+                }
+                String[] propertyInfo = StringUtil.split(property, "=");
+                if (propertyInfo.length < 2) {
+                    throw new IllegalArgumentException("Illegal jdbc properties: " + property);
+                }
+                if (separator.equals(SQLSERVER_SEPARATOR) && "database".equals(propertyInfo[0])) {
+                    checkIllegalCharacter(propertyInfo[1], DATABASE_WHITE_LIST);
+                    continue;
+                }
+                Preconditions.checkArgument(
+                        ALLOWED_PROPERTIES.contains(propertyInfo[0]),
+                        "The property [%s] is not in the allowed list %s, you can add the property to " +
+                                "the allowed properties list by kylin.jdbc.url.allowed.properties" +
+                                " and separate with commas.",
+                        propertyInfo[0],
+                        ALLOWED_PROPERTIES
+                );
+                checkIllegalCharacter(propertyInfo[1], PROPERTIES_VALUE_WHITE_LIST);
+            }
+        }
+    }
+}
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 767ae8b..4d24fd0 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
@@ -68,6 +68,9 @@ public abstract class KylinConfigBase implements Serializable {
     private static final String FILE_SCHEME = "file:";
     private static final String MAPRFS_SCHEME = "maprfs:";
     private static final Integer DEFAULT_MR_HIVE_GLOBAL_DICT_REDUCE_NUM_PER_COLUMN = 2;
+    private static final String DEFAULT_JDBC_URL_ALLOWED_PROPERTIED = "database,useSSL,requireSSL,ssl,sslmode";
+    private static final String DEFAULT_JDBC_URL_ALLOWED_SCHEMA = "mysql,sqlserver,redshift,postgresql,presto,h2,microsoft:sqlserver,jtds:sqlserver";
+    private static final String DEFAULT_JDBC_URL_ALLOWED_SQLSERVER_SCHEMA = "sqlserver,microsoft:sqlserver,jtds:sqlserver";
 
     /*
      * DON'T DEFINE CONSTANTS FOR PROPERTY KEYS!
@@ -2719,5 +2722,18 @@ public abstract class KylinConfigBase implements Serializable {
 
     public String getKylinDictCacheStrength(){
         return getOptional("kylin.dict.cache.strength", "soft");
-    };
+    }
+
+    public String getJdbcUrlAllowedProperties() {
+        return getOptional("kylin.jdbc.url.allowed.properties", DEFAULT_JDBC_URL_ALLOWED_PROPERTIED);
+    }
+
+    public String getJdbcAllowedSchema() {
+        return DEFAULT_JDBC_URL_ALLOWED_SCHEMA + "," + getOptional("kylin.jdbc.url.allowed.additional.schema", "");
+    }
+
+    public String getJdbcAllowedSqlServerSchema() {
+        return getOptional("kylin.jdbc.url.allowed.sqlserver.schema", DEFAULT_JDBC_URL_ALLOWED_SQLSERVER_SCHEMA);
+    }
+
 }
diff --git a/core-common/src/test/java/org/apache/kylin/common/JDBCConnectionUtilsTest.java b/core-common/src/test/java/org/apache/kylin/common/JDBCConnectionUtilsTest.java
new file mode 100644
index 0000000..2a6fa00
--- /dev/null
+++ b/core-common/src/test/java/org/apache/kylin/common/JDBCConnectionUtilsTest.java
@@ -0,0 +1,142 @@
+/*
+ * 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;
+
+import org.apache.commons.lang.exception.ExceptionUtils;
+import org.apache.kylin.common.util.LocalFileMetadataTestCase;
+import org.junit.Assert;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import java.sql.SQLException;
+
+public class JDBCConnectionUtilsTest extends LocalFileMetadataTestCase {
+    @BeforeClass
+    public static void setupClass() throws SQLException {
+        staticCreateTestMetadata();
+        KylinConfig kylinConfig = KylinConfig.getInstanceFromEnv();
+        kylinConfig.setProperty("kylin.jdbc.url.allowed.properties", "database,useSSL,requireSSL,ssl,sslmode,integratedSecurity");
+    }
+
+    @Test
+    public void testInvalidSchemaJdbcUrl() {
+        String jdbcUrl = "jdbc:mysqld://fakehost:1433/database";
+        try {
+            JDBCConnectionUtils.checkUrl(jdbcUrl);
+        } catch (Exception illegalException) {
+            Assert.assertEquals("IllegalArgumentException: The data source schema : mysqld is not allowed. " +
+                    "You can add the schema to the allowed schema list by kylin.jdbc.url.allowed.additional.schema " +
+                    "and separate with commas.", ExceptionUtils.getRootCauseMessage(illegalException));
+        }
+    }
+
+    @Test
+    public void testInvalidHostMysqlJdbcUrl() {
+        String jdbcUrl = "jdbc:mysql://fakehost&:1433/database";
+        try {
+            JDBCConnectionUtils.checkUrl(jdbcUrl);
+        } catch (Exception illegalException) {
+            Assert.assertEquals("IllegalArgumentException: Detected illegal character in fakehost& by "
+                    + JDBCConnectionUtils.HOST_NAME_WHITE_LIST + ", jdbc url not allowed.", ExceptionUtils.getRootCauseMessage(illegalException));
+        }
+    }
+
+    @Test
+    public void testInvalidPortMysqlJdbcUrl() {
+        String jdbcUrl = "jdbc:mysql://fakehost:1433F/database";
+        try {
+            JDBCConnectionUtils.checkUrl(jdbcUrl);
+        } catch (Exception illegalException) {
+            Assert.assertEquals("IllegalArgumentException: Detected illegal character in 1433F by "
+                    + JDBCConnectionUtils.PORT_WHITE_LIST + ", jdbc url not allowed.", ExceptionUtils.getRootCauseMessage(illegalException));
+        }
+    }
+
+    @Test
+    public void testInvalidPortSqlserverJdbcUrl() {
+        String jdbcUrl = "jdbc:sqlserver://fakehost:1433F;database=database";
+        try {
+            JDBCConnectionUtils.checkUrl(jdbcUrl);
+        } catch (Exception illegalException) {
+            Assert.assertEquals("IllegalArgumentException: Detected illegal character in 1433F by "
+                    + JDBCConnectionUtils.PORT_WHITE_LIST + ", jdbc url not allowed.", ExceptionUtils.getRootCauseMessage(illegalException));
+        }
+    }
+
+    @Test
+    public void testInvalidPropertyMysqlJdbcUrl() {
+        String jdbcUrl = "jdbc:mysql://fakehost:1433/database?allowLoadLocalInfile=true&autoDeserialize=false";
+        try {
+            JDBCConnectionUtils.checkUrl(jdbcUrl);
+        } catch (Exception illegalException) {
+            Assert.assertEquals("IllegalArgumentException: The property [allowLoadLocalInfile]" +
+                    " is not in the allowed list database,useSSL,requireSSL,ssl,sslmode,integratedSecurity" +
+                            ", you can add the property to the allowed properties list by kylin.jdbc.url.allowed.properties and separate with commas.",
+                    ExceptionUtils.getRootCauseMessage(illegalException));
+        }
+    }
+
+    @Test
+    public void testInvalidDatabaseSqlServerJdbcUrl() {
+        String jdbcUrl = "jdbc:sqlserver://fakehost:1433;database=database!;integratedSecurity=true;";
+        try {
+            JDBCConnectionUtils.checkUrl(jdbcUrl);
+        } catch (Exception illegalException) {
+            Assert.assertEquals("IllegalArgumentException: Detected illegal character in database! by "
+                    + JDBCConnectionUtils.DATABASE_WHITE_LIST + ", jdbc url not allowed.", ExceptionUtils.getRootCauseMessage(illegalException));
+        }
+    }
+
+    @Test
+    public void testInvalidPropertySqlserverJdbcUrl() {
+        String jdbcUrl = "jdbc:sqlserver://fakehost:1433;database=database;autoDeserialize=true";
+        try {
+            JDBCConnectionUtils.checkUrl(jdbcUrl);
+        } catch (Exception illegalException) {
+            Assert.assertEquals("IllegalArgumentException: The property [autoDeserialize]" +
+                    " is not in the allowed list database,useSSL,requireSSL,ssl,sslmode,integratedSecurity" +
+                    ", you can add the property to the allowed properties list by kylin.jdbc.url.allowed.properties and separate with commas.",
+                    ExceptionUtils.getRootCauseMessage(illegalException));
+        }
+    }
+
+    @Test
+    public void testValidMysqlJdbcUrl() {
+        String jdbcUrl = "jdbc:mysql://fakehost:1433/database_test?useSSL=true&requireSSL=true";
+        Assert.assertEquals(jdbcUrl + "&" + JDBCConnectionUtils.APPEND_PARAMS, JDBCConnectionUtils.checkUrl(jdbcUrl));
+    }
+
+    @Test
+    public void testValidSqlServerJdbcUrl() {
+        String jdbcUrl = "jdbc:sqlserver://fakehost:1433;database=database_test;integratedSecurity=true;";
+        Assert.assertEquals(jdbcUrl, JDBCConnectionUtils.checkUrl(jdbcUrl));
+    }
+
+    @Test
+    public void testValidMsSqlServerJdbcUrl() {
+        String jdbcUrl = "jdbc:microsoft:sqlserver://fakehost:1433;database=database_test;integratedSecurity=true;";
+        Assert.assertEquals(jdbcUrl, JDBCConnectionUtils.checkUrl(jdbcUrl));
+    }
+
+    @Test
+    public void testValidJtdsSqlServerJdbcUrl() {
+        String jdbcUrl = "jdbc:jtds:sqlserver://fakehost:1433;database=database_test;integratedSecurity=true;";
+        Assert.assertEquals(jdbcUrl, JDBCConnectionUtils.checkUrl(jdbcUrl));
+    }
+}
diff --git a/datasource-sdk/src/main/java/org/apache/kylin/sdk/datasource/adaptor/AdaptorConfig.java b/datasource-sdk/src/main/java/org/apache/kylin/sdk/datasource/adaptor/AdaptorConfig.java
index 49c4d53..e6ce26f 100644
--- a/datasource-sdk/src/main/java/org/apache/kylin/sdk/datasource/adaptor/AdaptorConfig.java
+++ b/datasource-sdk/src/main/java/org/apache/kylin/sdk/datasource/adaptor/AdaptorConfig.java
@@ -17,6 +17,8 @@
  */
 package org.apache.kylin.sdk.datasource.adaptor;
 
+import org.apache.kylin.common.JDBCConnectionUtils;
+
 public class AdaptorConfig {
     public final String url;
     public final String driver;
@@ -29,10 +31,10 @@ public class AdaptorConfig {
     public int poolMinIdle = 0;
 
     public AdaptorConfig(String url, String driver, String username, String password) {
-        this.url = url;
-        this.driver = driver;
+        this.url = JDBCConnectionUtils.checkUrl(url);
         this.username = username;
         this.password = password;
+        this.driver = driver;
     }
 
     @Override
diff --git a/datasource-sdk/src/test/java/org/apache/kylin/sdk/datasource/adaptor/AdaptorConfigTest.java b/datasource-sdk/src/test/java/org/apache/kylin/sdk/datasource/adaptor/AdaptorConfigTest.java
index 3ef826c..9db2c43 100644
--- a/datasource-sdk/src/test/java/org/apache/kylin/sdk/datasource/adaptor/AdaptorConfigTest.java
+++ b/datasource-sdk/src/test/java/org/apache/kylin/sdk/datasource/adaptor/AdaptorConfigTest.java
@@ -17,15 +17,23 @@
  */
 package org.apache.kylin.sdk.datasource.adaptor;
 
+import org.apache.kylin.common.util.LocalFileMetadataTestCase;
 import org.junit.Assert;
+import org.junit.BeforeClass;
 import org.junit.Test;
 
-public class AdaptorConfigTest {
+import java.sql.SQLException;
+
+public class AdaptorConfigTest extends LocalFileMetadataTestCase {
+    @BeforeClass
+    public static void setupClass() throws SQLException {
+        staticCreateTestMetadata();
+    }
     @Test
     public void testEquals() {
-        AdaptorConfig conf1 = new AdaptorConfig("a", "b", "c", "d");
-        AdaptorConfig conf2 = new AdaptorConfig("a", "b", "c", "d");
-        AdaptorConfig conf3 = new AdaptorConfig("a1", "b1", "c1", "d1");
+        AdaptorConfig conf1 = new AdaptorConfig("jdbc:mysql://fakehost:1433/database", "b", "c", "d");
+        AdaptorConfig conf2 = new AdaptorConfig("jdbc:mysql://fakehost:1433/database", "b", "c", "d");
+        AdaptorConfig conf3 = new AdaptorConfig("jdbc:mysql://fakehost:1433/database", "b1", "c1", "d1");
 
         Assert.assertEquals(conf1, conf2);
         Assert.assertEquals(conf1.hashCode(), conf2.hashCode());
diff --git a/source-hive/src/main/java/org/apache/kylin/source/hive/DBConnConf.java b/source-hive/src/main/java/org/apache/kylin/source/hive/DBConnConf.java
index 3460d5c..f79fb9d 100644
--- a/source-hive/src/main/java/org/apache/kylin/source/hive/DBConnConf.java
+++ b/source-hive/src/main/java/org/apache/kylin/source/hive/DBConnConf.java
@@ -21,6 +21,7 @@ package org.apache.kylin.source.hive;
 import java.util.Locale;
 
 import org.apache.commons.configuration.PropertiesConfiguration;
+import org.apache.kylin.common.JDBCConnectionUtils;
 
 public class DBConnConf {
     public static final String KEY_DRIVER = "driver";
@@ -63,6 +64,7 @@ public class DBConnConf {
     }
 
     public String getUrl() {
+        JDBCConnectionUtils.checkUrl(url);
         return url;
     }