You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cloudberry.apache.org by os...@apache.org on 2026/07/17 13:41:01 UTC
(cloudberry-pxf) 02/02: Do not enforce username/passwords for non-postgresql datasources (#17)
This is an automated email from the ASF dual-hosted git repository.
ostinru pushed a commit to branch safe-postgres-jdbc
in repository https://gitbox.apache.org/repos/asf/cloudberry-pxf.git
commit cb2f3307325ed40db62b98385df97a154ba9b920
Author: Nikolay Antonov <os...@yandex-team.ru>
AuthorDate: Wed Dec 24 12:35:15 2025 +0500
Do not enforce username/passwords for non-postgresql datasources (#17)
`java.net.URI` is not proper tool to parse JDBC connections strings:
* it fails to parse schemas that contains ':'. For example, `jdbc:postgres://` and `jdbc:sqlserver://` will fail to parse.
* Also it won't work with mircosoft jdbc that doesn't use `?` as separator between hostname and query parameters[1]. For example, in jdbcUrl `jdbc:sqlserver://localhost:1433;databaseName=db1;user=admin` it will not extract `user` field.
**Solution**: Use `Driver.parseURL` for postgresql and do not check other drivers.
[1] https://learn.microsoft.com/en-us/sql/connect/jdbc/using-the-jdbc-driver?view=sql-server-ver17#make-a-simple-connection-to-a-database
---
.../pxf/plugins/jdbc/JdbcBasePlugin.java | 72 ++++++++++++----------
.../pxf/plugins/jdbc/JdbcBasePluginTest.java | 47 ++++++++------
2 files changed, 67 insertions(+), 52 deletions(-)
diff --git a/server/pxf-jdbc/src/main/java/org/apache/cloudberry/pxf/plugins/jdbc/JdbcBasePlugin.java b/server/pxf-jdbc/src/main/java/org/apache/cloudberry/pxf/plugins/jdbc/JdbcBasePlugin.java
index 1ded13ff..1cda61ed 100644
--- a/server/pxf-jdbc/src/main/java/org/apache/cloudberry/pxf/plugins/jdbc/JdbcBasePlugin.java
+++ b/server/pxf-jdbc/src/main/java/org/apache/cloudberry/pxf/plugins/jdbc/JdbcBasePlugin.java
@@ -30,12 +30,11 @@ import org.apache.cloudberry.pxf.api.utilities.Utilities;
import org.apache.cloudberry.pxf.plugins.jdbc.utils.ConnectionManager;
import org.apache.cloudberry.pxf.plugins.jdbc.utils.DbProduct;
import org.apache.cloudberry.pxf.plugins.jdbc.utils.HiveJdbcUtils;
+import org.postgresql.Driver;
+import org.postgresql.PGProperty;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import java.io.IOException;
-import java.net.URI;
-import java.net.URISyntaxException;
import java.security.PrivilegedExceptionAction;
import java.sql.Connection;
import java.sql.DatabaseMetaData;
@@ -100,6 +99,7 @@ public class JdbcBasePlugin extends BasePlugin {
private static final String HIVE_DEFAULT_DRIVER_CLASS = "org.apache.hive.jdbc.HiveDriver";
private static final String MYSQL_DRIVER_PREFIX = "com.mysql.";
private static final String JDBC_DATE_WIDE_RANGE = "jdbc.date.wideRange";
+ private static final String POSTGRESQL_URL_PREFIX = "jdbc:postgresql://";
private enum TransactionIsolation {
READ_UNCOMMITTED(1),
@@ -303,25 +303,6 @@ public class JdbcBasePlugin extends BasePlugin {
String transactionIsolationString = configuration.get(JDBC_CONNECTION_TRANSACTION_ISOLATION, "NOT_PROVIDED");
transactionIsolation = TransactionIsolation.typeOf(transactionIsolationString);
- boolean hasUserInUrl = false;
- boolean hasPasswordInUrl = false;
- try {
- URI uri = new URI(jdbcUrl);
- String query = uri.getQuery();
- if (query != null && !query.isEmpty()) {
- hasUserInUrl = query.contains("user=");
- hasPasswordInUrl = query.contains("password=");
- }
- String rawUserInfo = uri.getRawUserInfo();
- if (rawUserInfo != null && !rawUserInfo.isEmpty()) {
- hasUserInUrl = true;
- hasPasswordInUrl = rawUserInfo.contains("/"); // https://www.orafaq.com/wiki/JDBC
- }
- } catch (URISyntaxException e) {
- // If the URL is malformed, it's also an invalid argument
- throw new IllegalArgumentException("Invalid JDBC URL format provided: " + jdbcUrl, e);
- }
-
// Set optional user parameter, taking into account impersonation setting for the server.
String jdbcUser = configuration.get(JDBC_USER_PROPERTY_NAME);
boolean impersonationEnabledForServer = configuration.getBoolean(CONFIG_KEY_SERVICE_USER_IMPERSONATION, false);
@@ -340,9 +321,6 @@ public class JdbcBasePlugin extends BasePlugin {
if (jdbcUser != null && !jdbcUser.isEmpty()) {
LOG.debug("Effective JDBC user {}", jdbcUser);
connectionConfiguration.setProperty("user", jdbcUser);
- } else if (!hasUserInUrl) {
- LOG.debug("JDBC user has not been set");
- throw new IllegalArgumentException("JDBC user has not been set");
}
if (LOG.isDebugEnabled()) {
@@ -355,18 +333,15 @@ public class JdbcBasePlugin extends BasePlugin {
// This must be the last parameter parsed, as we output connectionConfiguration earlier
// Optional parameter. By default, corresponding connectionConfiguration property is not set
- boolean passwordSetted = false;
+ String jdbcPassword = configuration.get(JDBC_PASSWORD_PROPERTY_NAME);
if (jdbcUser != null) {
- String jdbcPassword = configuration.get(JDBC_PASSWORD_PROPERTY_NAME);
if (jdbcPassword != null && !jdbcPassword.isEmpty()) {
LOG.debug("Connection password: {}", ConnectionManager.maskPassword(jdbcPassword));
connectionConfiguration.setProperty("password", jdbcPassword);
- passwordSetted = true;
}
}
- if (!passwordSetted && !hasPasswordInUrl) {
- throw new IllegalArgumentException("JDBC password has not been set");
- }
+
+ assertJDBC(jdbcUrl, jdbcUser, jdbcPassword);
// connection pool is optional, enabled by default
isConnectionPoolUsed = configuration.getBoolean(JDBC_CONNECTION_POOL_ENABLED_PROPERTY_NAME, true);
@@ -429,6 +404,38 @@ public class JdbcBasePlugin extends BasePlugin {
return connection;
}
+ /**
+ * Asserts whether all required JDBC URL params/properties set, throws IllegalArgumentException otherwise
+ *
+ * @param jdbcUrl connection string
+ * @param username username from properties (outside of connection string)
+ * @param password password from properties (outside of connection string)
+ */
+ private void assertJDBC(String jdbcUrl, String username, String password) {
+ if (!StringUtils.startsWith(jdbcUrl, POSTGRESQL_URL_PREFIX)) {
+ return;
+ }
+
+ // For PostgreSQL we enforce users to specify username and password
+ Properties urlProperties = Driver.parseURL(jdbcUrl, null);
+ if (urlProperties == null) {
+ throw new IllegalArgumentException("Invalid PostgreSQL JDBC URL format provided: " + jdbcUrl);
+ }
+
+ String urlUsername = urlProperties.getProperty(PGProperty.USER.getName());
+ if (StringUtils.isEmpty(username) && StringUtils.isEmpty(urlUsername)) {
+ LOG.debug("PostgreSQL JDBC user has not been set");
+ throw new IllegalArgumentException("PostgreSQL JDBC user has not been set");
+ }
+
+ String urlPassword = urlProperties.getProperty(PGProperty.PASSWORD.getName());
+ if (StringUtils.isEmpty(password) && StringUtils.isEmpty(urlPassword)) {
+ throw new IllegalArgumentException("PostgreSQL JDBC password has not been set");
+ }
+
+ // Ok. It is fine to work with this postgres datasource
+}
+
/**
* Prepare a JDBC PreparedStatement
*
@@ -624,6 +631,3 @@ public class JdbcBasePlugin extends BasePlugin {
}
}
-
-
-
diff --git a/server/pxf-jdbc/src/test/java/org/apache/cloudberry/pxf/plugins/jdbc/JdbcBasePluginTest.java b/server/pxf-jdbc/src/test/java/org/apache/cloudberry/pxf/plugins/jdbc/JdbcBasePluginTest.java
index db6cf0bc..2bcb781d 100644
--- a/server/pxf-jdbc/src/test/java/org/apache/cloudberry/pxf/plugins/jdbc/JdbcBasePluginTest.java
+++ b/server/pxf-jdbc/src/test/java/org/apache/cloudberry/pxf/plugins/jdbc/JdbcBasePluginTest.java
@@ -344,8 +344,8 @@ public class JdbcBasePluginTest {
@Test
public void testGetConnectionErrorWithoutPassword1() throws SQLException {
- configuration.set("jdbc.driver", "org.greenplum.pxf.plugins.jdbc.FakeJdbcDriver");
- configuration.set("jdbc.url", "test-url");
+ configuration.set("jdbc.driver", "org.postgresql.Driver");
+ configuration.set("jdbc.url", "jdbc:postgresql://example.com/test-url");
configuration.set("jdbc.password", "");
context.setServerName("test-server");
@@ -353,7 +353,7 @@ public class JdbcBasePluginTest {
try {
getPlugin(mockConnectionManager, mockSecureLogin, context);
} catch (IllegalArgumentException e) {
- assertEquals("JDBC password has not been set", e.getMessage());
+ assertEquals("PostgreSQL JDBC password has not been set", e.getMessage());
return;
}
Assertions.fail("Expected an exception to be thrown due to missing password, but no exception was thrown.");
@@ -362,9 +362,9 @@ public class JdbcBasePluginTest {
@Test
public void testGetConnectionErrorWithoutPassword2() throws SQLException {
configuration = new Configuration();
+ configuration.set("jdbc.driver", "org.postgresql.Driver");
+ configuration.set("jdbc.url", "jdbc:postgresql://example.com/test-url");
configuration.set("jdbc.user", "test-user");
- configuration.set("jdbc.driver", "org.greenplum.pxf.plugins.jdbc.FakeJdbcDriver");
- configuration.set("jdbc.url", "test-url");
context.setConfiguration(configuration);
context.setServerName("test-server");
@@ -372,17 +372,28 @@ public class JdbcBasePluginTest {
try {
getPlugin(mockConnectionManager, mockSecureLogin, context);
} catch (IllegalArgumentException e) {
- assertEquals("JDBC password has not been set", e.getMessage());
+ assertEquals("PostgreSQL JDBC password has not been set", e.getMessage());
return;
}
Assertions.fail("Expected an exception to be thrown due to missing password, but no exception was thrown.");
}
+ @Test
+ public void testGetConnectionErrorWithoutPassword3() throws SQLException {
+ configuration.set("jdbc.driver", "org.greenplum.pxf.plugins.jdbc.FakeJdbcDriver");
+ configuration.set("jdbc.url", "jdbc:sqlserver://localhost:1433;databaseName=db1;user=admin");
+
+ context.setServerName("test-server");
+
+ // non-PostgreSQL drivers are not subject to credential validation
+ getPlugin(mockConnectionManager, mockSecureLogin, context);
+ }
+
@Test
public void testGetConnectionWithPasswordInUrl() throws SQLException {
configuration = new Configuration();
- configuration.set("jdbc.driver", "org.greenplum.pxf.plugins.jdbc.FakeJdbcDriver");
- configuration.set("jdbc.url", "test-url?password=test-password");
+ configuration.set("jdbc.driver", "org.postgresql.Driver");
+ configuration.set("jdbc.url", "jdbc:postgresql://example.com/test-url?password=test-password");
configuration.set("jdbc.user", "test-user");
context.setServerName("test-server");
@@ -399,13 +410,13 @@ public class JdbcBasePluginTest {
Properties properties = new Properties();
properties.setProperty("user", "test-user");
- verify(mockConnectionManager).getConnection("test-server", "test-url?password=test-password", properties, true, poolProps, null);
+ verify(mockConnectionManager).getConnection("test-server", "jdbc:postgresql://example.com/test-url?password=test-password", properties, true, poolProps, null);
}
@Test
public void testGetConnectionErrorWithoutUser1() throws SQLException {
- configuration.set("jdbc.driver", "org.greenplum.pxf.plugins.jdbc.FakeJdbcDriver");
- configuration.set("jdbc.url", "test-url");
+ configuration.set("jdbc.driver", "org.postgresql.Driver");
+ configuration.set("jdbc.url", "jdbc:postgresql://example.com/test-url");
configuration.set("jdbc.user", "");
configuration.set("jdbc.password", "test-password");
@@ -414,7 +425,7 @@ public class JdbcBasePluginTest {
try {
getPlugin(mockConnectionManager, mockSecureLogin, context);
} catch (IllegalArgumentException e) {
- assertEquals("JDBC user has not been set", e.getMessage());
+ assertEquals("PostgreSQL JDBC user has not been set", e.getMessage());
return;
}
Assertions.fail("Expected an exception to be thrown due to missing user, but no exception was thrown.");
@@ -424,8 +435,8 @@ public class JdbcBasePluginTest {
public void testGetConnectionErrorWithoutUser2() throws SQLException {
configuration = new Configuration();
configuration.set("jdbc.password", "test-password");
- configuration.set("jdbc.driver", "org.greenplum.pxf.plugins.jdbc.FakeJdbcDriver");
- configuration.set("jdbc.url", "test-url");
+ configuration.set("jdbc.driver", "org.postgresql.Driver");
+ configuration.set("jdbc.url", "jdbc:postgresql://example.com/test-url");
context.setConfiguration(configuration);
context.setServerName("test-server");
@@ -433,7 +444,7 @@ public class JdbcBasePluginTest {
try {
getPlugin(mockConnectionManager, mockSecureLogin, context);
} catch (IllegalArgumentException e) {
- assertEquals("JDBC user has not been set", e.getMessage());
+ assertEquals("PostgreSQL JDBC user has not been set", e.getMessage());
return;
}
Assertions.fail("Expected an exception to be thrown due to missing user, but no exception was thrown.");
@@ -442,8 +453,8 @@ public class JdbcBasePluginTest {
@Test
public void testGetConnectionWithUserAndPasswordInUrl() throws SQLException {
configuration = new Configuration();
- configuration.set("jdbc.driver", "org.greenplum.pxf.plugins.jdbc.FakeJdbcDriver");
- configuration.set("jdbc.url", "test-url?user=test-user&password=test-password");
+ configuration.set("jdbc.driver", "org.postgresql.Driver");
+ configuration.set("jdbc.url", "jdbc:postgresql://example.com/db?user=test-user&password=test-password");
context.setConfiguration(configuration);
context.setServerName("test-server");
@@ -456,7 +467,7 @@ public class JdbcBasePluginTest {
assertSame(mockConnection, conn);
- verify(mockConnectionManager).getConnection("test-server", "test-url?user=test-user&password=test-password", new Properties(), true, poolProps, null);
+ verify(mockConnectionManager).getConnection("test-server", "jdbc:postgresql://example.com/db?user=test-user&password=test-password", new Properties(), true, poolProps, null);
}
@Test
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cloudberry.apache.org
For additional commands, e-mail: commits-help@cloudberry.apache.org