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 <598593...@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; }