This is an automated email from the ASF dual-hosted git repository.
jmclean pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 5355bd075d [#7960] Improvement (core,common): correct parameter order
in SqlSessionFactoryHelper and add unit tests for
JdbcUrlUtils.validateJdbcConfig (#8000)
5355bd075d is described below
commit 5355bd075d54cad1323606723e72cc32ca564f24
Author: Joseph C. <[email protected]>
AuthorDate: Sun Aug 10 23:36:00 2025 -0400
[#7960] Improvement (core,common): correct parameter order in
SqlSessionFactoryHelper and add unit tests for JdbcUrlUtils.validateJdbcConfig
(#8000)
### What changes were proposed in this pull request?
Fixes the parameter order when calling JdbcUrlUtils.validateJdbcConfig
in SqlSessionFactoryHelper.java. The method now receives driverClass as
the first parameter and jdbcUrl as the second, as expected.
Adds comprehensive unit tests in TestJdbcUrlUtils.java covering valid
and invalid JDBC configurations.
### Why are the changes needed?
JdbcUrlUtils.validateJdbcConfig validates the JDBC configuration by
checking the driver class name and URL for unsafe parameters. Incorrect
parameter order caused validation to run against the wrong values,
leading to incorrect results. This fix ensures validation logic works as
intended and improves test coverage.
Fix: #7960
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Added unit tests for both successful and failure cases in
TestJdbcUrlUtils.java. All tests pass.
---
.../apache/gravitino/utils/TestJdbcUrlUtils.java | 128 +++++++++++++++++++++
.../session/SqlSessionFactoryHelper.java | 2 +-
2 files changed, 129 insertions(+), 1 deletion(-)
diff --git
a/common/src/test/java/org/apache/gravitino/utils/TestJdbcUrlUtils.java
b/common/src/test/java/org/apache/gravitino/utils/TestJdbcUrlUtils.java
new file mode 100644
index 0000000000..bcd4810595
--- /dev/null
+++ b/common/src/test/java/org/apache/gravitino/utils/TestJdbcUrlUtils.java
@@ -0,0 +1,128 @@
+/*
+ * 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.util.Map;
+import org.apache.gravitino.exceptions.GravitinoRuntimeException;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+public class TestJdbcUrlUtils {
+
+ @Test
+ public void whenMalformedUrlGiven_ShouldThrowGravitinoRuntimeException() {
+ GravitinoRuntimeException gre =
+ Assertions.assertThrows(
+ GravitinoRuntimeException.class,
+ () ->
+ JdbcUrlUtils.validateJdbcConfig(
+ "testDriver", "malformed%ZZurl", Map.of("test", "test")));
+ Assertions.assertEquals("Unable to decode JDBC URL", gre.getMessage());
+ }
+
+ @Test
+ public void testValidateJdbcConfigWhenDriverClassNameIsNull() {
+ Assertions.assertDoesNotThrow(
+ () ->
+ JdbcUrlUtils.validateJdbcConfig(
+ null, "jdbc:mysql://localhost:0000/test", Map.of("test",
"test")));
+ }
+
+ @Test
+ public void testValidateJdbcConfigForMySQL() {
+ Assertions.assertDoesNotThrow(
+ () ->
+ JdbcUrlUtils.validateJdbcConfig(
+ "testDriver", "jdbc:mysql://localhost:0000/test",
Map.of("test", "test")));
+ }
+
+ @Test
+ public void
whenUnsafeParameterGivenForMySQL_ShouldThrowGravitinoRuntimeException() {
+
+ GravitinoRuntimeException gre =
+ Assertions.assertThrows(
+ GravitinoRuntimeException.class,
+ () ->
+ JdbcUrlUtils.validateJdbcConfig(
+ "testDriver",
+
"jdbc:mysql://localhost:0000/test?allowloadlocalinfile=test",
+ Map.of("test", "test")));
+ Assertions.assertEquals(
+ "Unsafe MySQL parameter 'allowloadlocalinfile' detected in JDBC URL",
gre.getMessage());
+ }
+
+ @Test
+ public void
whenConfigPropertiesMapContainsUnsafeParam_ShouldThrowGravitinoRuntimeException()
{
+ GravitinoRuntimeException gre =
+ Assertions.assertThrows(
+ GravitinoRuntimeException.class,
+ () ->
+ JdbcUrlUtils.validateJdbcConfig(
+ "testDriver",
+ "jdbc:mysql://localhost:0000/test",
+ Map.of("maxAllowedPacket", "maxAllowedPacket")));
+ Assertions.assertEquals(
+ "Unsafe MySQL parameter 'maxAllowedPacket' detected in JDBC URL",
gre.getMessage());
+ }
+
+ @Test
+ public void testValidateJdbcConfigForMariaDB() {
+ Assertions.assertDoesNotThrow(
+ () ->
+ JdbcUrlUtils.validateJdbcConfig(
+ "testDriver", "jdbc:mariadb://localhost:0000/test",
Map.of("test", "test")));
+ }
+
+ @Test
+ public void
whenUnsafeParameterGivenForMariaDB_ShouldThrowGravitintoRuntimeException() {
+ GravitinoRuntimeException gre =
+ Assertions.assertThrows(
+ GravitinoRuntimeException.class,
+ () ->
+ JdbcUrlUtils.validateJdbcConfig(
+ "testDriver",
+
"jdbc:mariaDB://localhost:0000/test?allowloadlocalinfile=test",
+ Map.of("test", "test")));
+ Assertions.assertEquals(
+ "Unsafe MariaDB parameter 'allowloadlocalinfile' detected in JDBC
URL", gre.getMessage());
+ }
+
+ @Test
+ public void testValidateJdbcConfigForPostgreSQL() {
+ Assertions.assertDoesNotThrow(
+ () ->
+ JdbcUrlUtils.validateJdbcConfig(
+ "testDriver", "jdbc:postgresql://localhost:0000/test",
Map.of("test", "test")));
+ }
+
+ @Test
+ public void
whenUnsafeParameterGivenForPostgreSQL_ShouldThrowGravitinoRuntimeException() {
+ GravitinoRuntimeException gre =
+ Assertions.assertThrows(
+ GravitinoRuntimeException.class,
+ () ->
+ JdbcUrlUtils.validateJdbcConfig(
+ "testDriver",
+ "jdbc:postgresql://localhost:0000/test?socketFactory=test",
+ Map.of("test", "test")));
+ Assertions.assertEquals(
+ "Unsafe PostgreSQL parameter 'socketFactory' detected in JDBC URL",
gre.getMessage());
+ }
+}
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 0aca8f6388..a649a454b0 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
@@ -66,7 +66,7 @@ public class SqlSessionFactoryHelper {
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());
+ JdbcUrlUtils.validateJdbcConfig(driverClass, jdbcUrl,
config.getAllConfig());
JDBCBackendType jdbcType = JDBCBackendType.fromURI(jdbcUrl);
dataSource.setUrl(jdbcUrl);
