This is an automated email from the ASF dual-hosted git repository.
roryqi pushed a commit to branch branch-0.9
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/branch-0.9 by this push:
new 1c03908c81 [#7614] improvement(core, jdbc-catalog): Validate URLs for
JDBC entity store. (#7684)
1c03908c81 is described below
commit 1c03908c81f4d6499ffa031a18249232523cff08
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Mon Jul 14 15:07:19 2025 +0800
[#7614] improvement(core, jdbc-catalog): Validate URLs for JDBC entity
store. (#7684)
### What changes were proposed in this pull request?
Add validation logic for URLs of JDBC entity store.
### Why are the changes needed?
To fix possible problem caused by improper configurations in the URL
URLs.
Fix: #7614
### Does this PR introduce _any_ user-facing change?
N/A.
### How was this patch tested?
Existing tests.
Co-authored-by: Mini Yu <[email protected]>
---
.../catalog/jdbc/utils/DataSourceUtils.java | 63 +----------
...Utils.java => TestDataSourceUrlValidation.java} | 2 +-
.../org/apache/gravitino/utils/JdbcUrlUtils.java | 125 +++++++++++++++++++++
.../session/SqlSessionFactoryHelper.java | 6 +-
4 files changed, 133 insertions(+), 63 deletions(-)
diff --git
a/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/utils/DataSourceUtils.java
b/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/utils/DataSourceUtils.java
index e533edf516..9af11395b5 100644
---
a/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/utils/DataSourceUtils.java
+++
b/catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/utils/DataSourceUtils.java
@@ -19,8 +19,6 @@
package org.apache.gravitino.catalog.jdbc.utils;
import java.sql.SQLException;
-import java.util.Arrays;
-import java.util.List;
import java.util.Map;
import java.util.Properties;
import javax.sql.DataSource;
@@ -28,6 +26,7 @@ import org.apache.commons.dbcp2.BasicDataSource;
import org.apache.commons.dbcp2.BasicDataSourceFactory;
import org.apache.gravitino.catalog.jdbc.config.JdbcConfig;
import org.apache.gravitino.exceptions.GravitinoRuntimeException;
+import org.apache.gravitino.utils.JdbcUrlUtils;
/**
* Utility class for creating a {@link DataSource} from a {@link JdbcConfig}.
It is mainly
@@ -39,26 +38,6 @@ public class DataSourceUtils {
/** SQL statements for database connection pool testing. */
private static final String POOL_TEST_QUERY = "SELECT 1";
- private static final List<String> unsafeMySQLParameters =
- Arrays.asList(
- "maxAllowedPacket",
- "autoDeserialize",
- "queryInterceptors",
- "statementInterceptors",
- "detectCustomCollations",
- "allowloadlocalinfile",
- "allowUrlInLocalInfile",
- "allowLoadLocalInfileInPath");
-
- private static final List<String> unsafePostgresParameters =
- Arrays.asList(
- "socketFactory",
- "socketFactoryArg",
- "sslfactory",
- "sslhostnameverifier",
- "sslpasswordcallback",
- "authenticationPluginClassName");
-
public static DataSource createDataSource(Map<String, String> properties) {
return createDataSource(new JdbcConfig(properties));
}
@@ -73,7 +52,7 @@ public class DataSourceUtils {
}
private static DataSource createDBCPDataSource(JdbcConfig jdbcConfig) throws
Exception {
- validateJdbcConfig(
+ JdbcUrlUtils.validateJdbcConfig(
jdbcConfig.getJdbcDriver(), jdbcConfig.getJdbcUrl(),
jdbcConfig.getAllConfig());
BasicDataSource basicDataSource =
BasicDataSourceFactory.createDataSource(getProperties(jdbcConfig));
@@ -117,44 +96,6 @@ public class DataSourceUtils {
return decoded;
}
- private static void checkUnsafeParameters(
- String url, Map<String, String> config, List<String> unsafeParams,
String dbType) {
-
- String lowerUrl = url.toLowerCase();
-
- for (String param : unsafeParams) {
- String lowerParam = param.toLowerCase();
- if (lowerUrl.contains(lowerParam) || containsValueIgnoreCase(config,
param)) {
- throw new GravitinoRuntimeException(
- "Unsafe %s parameter '%s' detected in JDBC URL", dbType, param);
- }
- }
- }
-
- public static void validateJdbcConfig(String driver, String url, Map<String,
String> all) {
- String lowerUrl = url.toLowerCase();
- String decodedUrl = recursiveDecode(lowerUrl);
-
- if (driver != null) {
- if (decodedUrl.startsWith("jdbc:mysql")) {
- checkUnsafeParameters(decodedUrl, all, unsafeMySQLParameters, "MySQL");
- } else if (decodedUrl.startsWith("jdbc:mariadb")) {
- checkUnsafeParameters(decodedUrl, all, unsafeMySQLParameters,
"MariaDB");
- } else if (decodedUrl.startsWith("jdbc:postgresql")) {
- checkUnsafeParameters(decodedUrl, all, unsafePostgresParameters,
"PostgreSQL");
- }
- }
- }
-
- private static boolean containsValueIgnoreCase(Map<String, String> map,
String value) {
- for (String keyValue : map.values()) {
- if (keyValue != null && keyValue.equalsIgnoreCase(value)) {
- return true;
- }
- }
- return false;
- }
-
public static void closeDataSource(DataSource dataSource) {
if (null != dataSource) {
try {
diff --git
a/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/utils/TestDataSourceUtils.java
b/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/utils/TestDataSourceUrlValidation.java
similarity index 98%
rename from
catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/utils/TestDataSourceUtils.java
rename to
catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/utils/TestDataSourceUrlValidation.java
index 6afb581ecb..a8a98b5faa 100644
---
a/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/utils/TestDataSourceUtils.java
+++
b/catalogs/catalog-jdbc-common/src/test/java/org/apache/gravitino/catalog/jdbc/utils/TestDataSourceUrlValidation.java
@@ -28,7 +28,7 @@ import
org.apache.gravitino.exceptions.GravitinoRuntimeException;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;
-public class TestDataSourceUtils {
+public class TestDataSourceUrlValidation {
@Test
public void testCreateDataSource() throws SQLException {
diff --git a/common/src/main/java/org/apache/gravitino/utils/JdbcUrlUtils.java
b/common/src/main/java/org/apache/gravitino/utils/JdbcUrlUtils.java
new file mode 100644
index 0000000000..eec8c44ec0
--- /dev/null
+++ b/common/src/main/java/org/apache/gravitino/utils/JdbcUrlUtils.java
@@ -0,0 +1,125 @@
+/*
+ * 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.gravitino.utils;
+
+import java.net.URLDecoder;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Map;
+import org.apache.gravitino.exceptions.GravitinoRuntimeException;
+
+/**
+ * Utility class for validating JDBC URLs and configurations. It checks for
unsafe parameters
+ * specific to MySQL, MariaDB, and PostgreSQL JDBC URLs. Unsafe parameters can
lead to security
+ * vulnerabilities or unexpected behavior, so this utility helps ensure that
JDBC configurations are
+ * safe to use.
+ */
+public class JdbcUrlUtils {
+
+ // Unsafe parameters for MySQL and MariaDB, other parameters like
+ // trustCertificateKeyStoreUrl, serverTimezone, characterEncoding,
+ // useSSL are also risky, please use it with caution.
+ private static final List<String> UNSAFE_MYSQL_PARAMETERS =
+ Arrays.asList(
+ "maxAllowedPacket",
+ "autoDeserialize",
+ "queryInterceptors",
+ "statementInterceptors",
+ "detectCustomCollations",
+ "allowloadlocalinfile",
+ "allowUrlInLocalInfile",
+ "allowLoadLocalInfileInPath");
+
+ private static final List<String> UNSAFE_POSTGRES_PARAMETERS =
+ Arrays.asList(
+ "socketFactory",
+ "socketFactoryArg",
+ "sslfactory",
+ "sslhostnameverifier",
+ "sslpasswordcallback",
+ "authenticationPluginClassName");
+
+ private JdbcUrlUtils() {
+ // Utility class, no instantiation allowed
+ }
+
+ /**
+ * Validates the JDBC configuration by checking the driver and URL for
unsafe parameters.
+ *
+ * @param driver the JDBC driver class name
+ * @param url the JDBC URL
+ * @param all the JDBC configuration properties
+ */
+ public static void validateJdbcConfig(String driver, String url, Map<String,
String> all) {
+ String lowerUrl = url.toLowerCase();
+ String decodedUrl = recursiveDecode(lowerUrl);
+
+ // As H2 is only used for testing, we do not check unsafe parameters for
H2.
+ if (driver != null) {
+ if (decodedUrl.startsWith("jdbc:mysql")) {
+ checkUnsafeParameters(decodedUrl, all, UNSAFE_MYSQL_PARAMETERS,
"MySQL");
+ } else if (decodedUrl.startsWith("jdbc:mariadb")) {
+ checkUnsafeParameters(decodedUrl, all, UNSAFE_MYSQL_PARAMETERS,
"MariaDB");
+ } else if (decodedUrl.startsWith("jdbc:postgresql")) {
+ checkUnsafeParameters(decodedUrl, all, UNSAFE_POSTGRES_PARAMETERS,
"PostgreSQL");
+ }
+ }
+ }
+
+ private static void checkUnsafeParameters(
+ String url, Map<String, String> config, List<String> unsafeParams,
String dbType) {
+
+ String lowerUrl = url.toLowerCase();
+
+ for (String param : unsafeParams) {
+ String lowerParam = param.toLowerCase();
+ if (lowerUrl.contains(lowerParam) || containsValueIgnoreCase(config,
param)) {
+ throw new GravitinoRuntimeException(
+ "Unsafe %s parameter '%s' detected in JDBC URL", dbType, param);
+ }
+ }
+ }
+
+ private static String recursiveDecode(String url) {
+ String prev;
+ String decoded = url;
+ int max = 5;
+
+ do {
+ prev = decoded;
+ try {
+ decoded = URLDecoder.decode(prev, "UTF-8");
+ } catch (Exception e) {
+ throw new GravitinoRuntimeException("Unable to decode JDBC URL");
+ }
+ } while (!prev.equals(decoded) && --max > 0);
+
+ return decoded;
+ }
+
+ private static boolean containsValueIgnoreCase(Map<String, String> map,
String value) {
+ for (String keyValue : map.values()) {
+ if (keyValue != null && keyValue.equalsIgnoreCase(value)) {
+ return true;
+ }
+ }
+ return false;
+ }
+}
diff --git
a/core/src/main/java/org/apache/gravitino/storage/relational/session/SqlSessionFactoryHelper.java
b/core/src/main/java/org/apache/gravitino/storage/relational/session/SqlSessionFactoryHelper.java
index 2040ecba9a..0aca8f6388 100644
---
a/core/src/main/java/org/apache/gravitino/storage/relational/session/SqlSessionFactoryHelper.java
+++
b/core/src/main/java/org/apache/gravitino/storage/relational/session/SqlSessionFactoryHelper.java
@@ -32,6 +32,7 @@ import org.apache.gravitino.metrics.MetricsSystem;
import org.apache.gravitino.metrics.source.RelationDatasourceMetricsSource;
import org.apache.gravitino.storage.relational.JDBCBackend.JDBCBackendType;
import
org.apache.gravitino.storage.relational.mapper.provider.MapperPackageProvider;
+import org.apache.gravitino.utils.JdbcUrlUtils;
import org.apache.ibatis.mapping.Environment;
import org.apache.ibatis.session.Configuration;
import org.apache.ibatis.session.SqlSessionFactory;
@@ -64,9 +65,12 @@ public class SqlSessionFactoryHelper {
// Initialize the data source
BasicDataSource dataSource = new BasicDataSource();
String jdbcUrl = config.get(Configs.ENTITY_RELATIONAL_JDBC_BACKEND_URL);
+ String driverClass =
config.get(Configs.ENTITY_RELATIONAL_JDBC_BACKEND_DRIVER);
+ JdbcUrlUtils.validateJdbcConfig(jdbcUrl, driverClass,
config.getAllConfig());
+
JDBCBackendType jdbcType = JDBCBackendType.fromURI(jdbcUrl);
dataSource.setUrl(jdbcUrl);
-
dataSource.setDriverClassName(config.get(Configs.ENTITY_RELATIONAL_JDBC_BACKEND_DRIVER));
+ dataSource.setDriverClassName(driverClass);
dataSource.setUsername(config.get(Configs.ENTITY_RELATIONAL_JDBC_BACKEND_USER));
dataSource.setPassword(config.get(Configs.ENTITY_RELATIONAL_JDBC_BACKEND_PASSWORD));
// Close the auto commit, so that we can control the transaction manual
commit
