Skip to content

Safe postgres jdbc #134

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
ostinru wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Safe postgres jdbc #134

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e3f39cb
hide .pgpass values (#9)
Artem3213212 Aug 28, 2025
cb2f330
Do not enforce username/passwords for non-postgresql datasources (#17)
ostinru Dec 24, 2025
93cbd30
CLRF -> LF
ostinru Jul 17, 2026
c63dc68
fix test
ostinru Jul 17, 2026
ed5b368
an attempt to fix test
ostinru Jul 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions automation/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,8 @@ sync_jdbc_config:
cp $(TEMPLATES_DIR)/jdbc-site.xml $(PXF_BASE_SERVERS)/database/; \
sed $(SED_OPTS) "s|YOUR_DATABASE_JDBC_DRIVER_CLASS_NAME|org.postgresql.Driver|" $(PXF_BASE_SERVERS)/database/jdbc-site.xml; \
sed $(SED_OPTS) "s|YOUR_DATABASE_JDBC_URL|jdbc:postgresql://$(JDBC_HOST):$(JDBC_PORT)/pxfautomation|" $(PXF_BASE_SERVERS)/database/jdbc-site.xml; \
sed $(SED_OPTS) "s|YOUR_DATABASE_JDBC_USER||" $(PXF_BASE_SERVERS)/database/jdbc-site.xml; \
sed $(SED_OPTS) "s|YOUR_DATABASE_JDBC_PASSWORD||" $(PXF_BASE_SERVERS)/database/jdbc-site.xml; \
sed $(SED_OPTS) "s|YOUR_DATABASE_JDBC_USER|gpadmin|" $(PXF_BASE_SERVERS)/database/jdbc-site.xml; \
sed $(SED_OPTS) "s|YOUR_DATABASE_JDBC_PASSWORD|gpadmin|" $(PXF_BASE_SERVERS)/database/jdbc-site.xml; \
cp $(PXF_BASE_SERVERS)/database/jdbc-site.xml $(PXF_BASE_SERVERS)/database/testuser-user.xml; \
sed $(SED_OPTS) "s|pxfautomation|template1|" $(PXF_BASE_SERVERS)/database/testuser-user.xml; \
fi
Expand All @@ -152,8 +152,8 @@ sync_jdbc_config:
cp $(TEMPLATES_DIR)/jdbc-site.xml $(PXF_BASE_SERVERS)/db-session-params/; \
sed $(SED_OPTS) "s|YOUR_DATABASE_JDBC_DRIVER_CLASS_NAME|org.postgresql.Driver|" $(PXF_BASE_SERVERS)/db-session-params/jdbc-site.xml; \
sed $(SED_OPTS) "s|YOUR_DATABASE_JDBC_URL|jdbc:postgresql://$(JDBC_HOST):$(JDBC_PORT)/pxfautomation|" $(PXF_BASE_SERVERS)/db-session-params/jdbc-site.xml; \
sed $(SED_OPTS) "s|YOUR_DATABASE_JDBC_USER||" $(PXF_BASE_SERVERS)/db-session-params/jdbc-site.xml; \
sed $(SED_OPTS) "s|YOUR_DATABASE_JDBC_PASSWORD||" $(PXF_BASE_SERVERS)/db-session-params/jdbc-site.xml; \
sed $(SED_OPTS) "s|YOUR_DATABASE_JDBC_USER|gpadmin|" $(PXF_BASE_SERVERS)/db-session-params/jdbc-site.xml; \
sed $(SED_OPTS) "s|YOUR_DATABASE_JDBC_PASSWORD|gpadmin|" $(PXF_BASE_SERVERS)/db-session-params/jdbc-site.xml; \
sed $(SED_OPTS) "s|</configuration>|<property><name>jdbc.session.property.client_min_messages</name><value>debug1</value></property></configuration>|" $(PXF_BASE_SERVERS)/db-session-params/jdbc-site.xml; \
sed $(SED_OPTS) "s|</configuration>|<property><name>jdbc.session.property.default_statistics_target</name><value>123</value></property></configuration>|" $(PXF_BASE_SERVERS)/db-session-params/jdbc-site.xml; \
fi
Expand Down
8 changes: 4 additions & 4 deletions ...ation/src/main/java/org/apache/cloudberry/pxf/automation/applications/PXFApplication.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ public void configureJdbcServers() throws IOException, InterruptedException {
"cp ${TEMPLATES_DIR}/jdbc-site.xml ${PXF_BASE_SERVERS}/database/",
"sed -i 's|YOUR_DATABASE_JDBC_DRIVER_CLASS_NAME|org.postgresql.Driver|' ${PXF_BASE_SERVERS}/database/jdbc-site.xml",
"sed -i 's|YOUR_DATABASE_JDBC_URL|jdbc:postgresql://localhost:7000/pxfautomation|' ${PXF_BASE_SERVERS}/database/jdbc-site.xml",
"sed -i 's|YOUR_DATABASE_JDBC_USER||' ${PXF_BASE_SERVERS}/database/jdbc-site.xml",
"sed -i 's|YOUR_DATABASE_JDBC_PASSWORD||' ${PXF_BASE_SERVERS}/database/jdbc-site.xml",
"sed -i 's|YOUR_DATABASE_JDBC_USER|gpadmin|' ${PXF_BASE_SERVERS}/database/jdbc-site.xml",
"sed -i 's|YOUR_DATABASE_JDBC_PASSWORD|gpadmin|' ${PXF_BASE_SERVERS}/database/jdbc-site.xml",
"cp ${PXF_BASE_SERVERS}/database/jdbc-site.xml ${PXF_BASE_SERVERS}/database/testuser-user.xml",
"sed -i 's|pxfautomation|template1|' ${PXF_BASE_SERVERS}/database/testuser-user.xml",
"cp /home/gpadmin/workspace/cloudberry-pxf/automation/src/test/resources/report.sql ${PXF_BASE_SERVERS}/database/",
Expand All @@ -62,8 +62,8 @@ public void configureJdbcServers() throws IOException, InterruptedException {
"cp ${TEMPLATES_DIR}/jdbc-site.xml ${PXF_BASE_SERVERS}/db-session-params/",
"sed -i 's|YOUR_DATABASE_JDBC_DRIVER_CLASS_NAME|org.postgresql.Driver|' ${PXF_BASE_SERVERS}/db-session-params/jdbc-site.xml",
"sed -i 's|YOUR_DATABASE_JDBC_URL|jdbc:postgresql://localhost:7000/pxfautomation|' ${PXF_BASE_SERVERS}/db-session-params/jdbc-site.xml",
"sed -i 's|YOUR_DATABASE_JDBC_USER||' ${PXF_BASE_SERVERS}/db-session-params/jdbc-site.xml",
"sed -i 's|YOUR_DATABASE_JDBC_PASSWORD||' ${PXF_BASE_SERVERS}/db-session-params/jdbc-site.xml",
"sed -i 's|YOUR_DATABASE_JDBC_USER|gpadmin|' ${PXF_BASE_SERVERS}/db-session-params/jdbc-site.xml",
"sed -i 's|YOUR_DATABASE_JDBC_PASSWORD|gpadmin|' ${PXF_BASE_SERVERS}/db-session-params/jdbc-site.xml",
"sed -i 's|</configuration>|<property><name>jdbc.session.property.client_min_messages</name><value>debug1</value></property></configuration>|' ${PXF_BASE_SERVERS}/db-session-params/jdbc-site.xml",
"sed -i 's|</configuration>|<property><name>jdbc.session.property.default_statistics_target</name><value>123</value></property></configuration>|' ${PXF_BASE_SERVERS}/db-session-params/jdbc-site.xml",

Expand Down
45 changes: 40 additions & 5 deletions server/pxf-jdbc/src/main/java/org/apache/cloudberry/pxf/plugins/jdbc/JdbcBasePlugin.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
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;

Expand Down Expand Up @@ -97,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),
Expand Down Expand Up @@ -315,11 +318,9 @@ public void afterPropertiesSet() {
jdbcUser = context.getUser();
}
}
if (jdbcUser != null) {
if (jdbcUser != null && !jdbcUser.isEmpty()) {
LOG.debug("Effective JDBC user {}", jdbcUser);
connectionConfiguration.setProperty("user", jdbcUser);
} else {
LOG.debug("JDBC user has not been set");
}

if (LOG.isDebugEnabled()) {
Expand All @@ -332,14 +333,16 @@ public void afterPropertiesSet() {

// This must be the last parameter parsed, as we output connectionConfiguration earlier
// Optional parameter. By default, corresponding connectionConfiguration property is not set
String jdbcPassword = configuration.get(JDBC_PASSWORD_PROPERTY_NAME);
if (jdbcUser != null) {
String jdbcPassword = configuration.get(JDBC_PASSWORD_PROPERTY_NAME);
if (jdbcPassword != null) {
if (jdbcPassword != null && !jdbcPassword.isEmpty()) {
LOG.debug("Connection password: {}", ConnectionManager.maskPassword(jdbcPassword));
connectionConfiguration.setProperty("password", jdbcPassword);
}
}

assertJDBC(jdbcUrl, jdbcUser, jdbcPassword);

// connection pool is optional, enabled by default
isConnectionPoolUsed = configuration.getBoolean(JDBC_CONNECTION_POOL_ENABLED_PROPERTY_NAME, true);
LOG.debug("Connection pool is {}enabled", isConnectionPoolUsed ? "" : "not ");
Expand Down Expand Up @@ -401,6 +404,38 @@ public Connection getConnection() throws SQLException {
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
*
Expand Down
2 changes: 2 additions & 0 deletions server/pxf-jdbc/src/test/java/org/apache/cloudberry/pxf/plugins/jdbc/JdbcAccessorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@ public void setup() {

accessor = new JdbcAccessor(mockConnectionManager, mockSecureLogin);
configuration = new Configuration();
configuration.set("jdbc.user", "test-user");
configuration.set("jdbc.password", "test-password");
context = new RequestContext();
context.setConfig("default");
context.setDataSource("test-table");
Expand Down
151 changes: 145 additions & 6 deletions server/pxf-jdbc/src/test/java/org/apache/cloudberry/pxf/plugins/jdbc/JdbcBasePluginTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.apache.cloudberry.pxf.api.model.RequestContext;
import org.apache.cloudberry.pxf.api.security.SecureLogin;
import org.apache.cloudberry.pxf.plugins.jdbc.utils.ConnectionManager;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
Expand Down Expand Up @@ -71,9 +72,19 @@ public class JdbcBasePluginTest {
private RequestContext context;
private Properties poolProps;

private Properties getDefaultConnectionProperties() {
Properties properties = new Properties();
properties.setProperty("user", "test-user");
properties.setProperty("password", "test-password");
return properties;
}

@BeforeEach
public void before() {
configuration = new Configuration();
configuration.set("jdbc.user", "test-user");
configuration.set("jdbc.password", "test-password");

context = new RequestContext();
context.setConfig("default");
context.setDataSource("test-table");
Expand Down Expand Up @@ -331,6 +342,134 @@ public void testGetPreparedStatementDoesNotSetQueryTimeoutIfNotSpecified() throw
verify(mockStatement, never()).setQueryTimeout(anyInt());
}

@Test
public void testGetConnectionErrorWithoutPassword1() throws SQLException {
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");

try {
getPlugin(mockConnectionManager, mockSecureLogin, context);
} catch (IllegalArgumentException e) {
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 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");
context.setConfiguration(configuration);

context.setServerName("test-server");

try {
getPlugin(mockConnectionManager, mockSecureLogin, context);
} catch (IllegalArgumentException e) {
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.apache.cloudberry.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.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");
context.setConfiguration(configuration);

when(mockConnectionManager.getConnection(any(), any(), any(), anyBoolean(), any(), any())).thenReturn(mockConnection);
when(mockConnection.getMetaData()).thenReturn(mockMetaData);

JdbcBasePlugin plugin = getPlugin(mockConnectionManager, mockSecureLogin, context);
Connection conn = plugin.getConnection();

assertSame(mockConnection, conn);

Properties properties = new Properties();
properties.setProperty("user", "test-user");

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.postgresql.Driver");
configuration.set("jdbc.url", "jdbc:postgresql://example.com/test-url");
configuration.set("jdbc.user", "");
configuration.set("jdbc.password", "test-password");

context.setServerName("test-server");

try {
getPlugin(mockConnectionManager, mockSecureLogin, context);
} catch (IllegalArgumentException e) {
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.");
}

@Test
public void testGetConnectionErrorWithoutUser2() throws SQLException {
configuration = new Configuration();
configuration.set("jdbc.password", "test-password");
configuration.set("jdbc.driver", "org.postgresql.Driver");
configuration.set("jdbc.url", "jdbc:postgresql://example.com/test-url");
context.setConfiguration(configuration);

context.setServerName("test-server");

try {
getPlugin(mockConnectionManager, mockSecureLogin, context);
} catch (IllegalArgumentException e) {
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.");
}

@Test
public void testGetConnectionWithUserAndPasswordInUrl() throws SQLException {
configuration = new Configuration();
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");

when(mockConnectionManager.getConnection(any(), any(), any(), anyBoolean(), any(), any())).thenReturn(mockConnection);
when(mockConnection.getMetaData()).thenReturn(mockMetaData);

JdbcBasePlugin plugin = getPlugin(mockConnectionManager, mockSecureLogin, context);
Connection conn = plugin.getConnection();

assertSame(mockConnection, conn);

verify(mockConnectionManager).getConnection("test-server", "jdbc:postgresql://example.com/db?user=test-user&password=test-password", new Properties(), true, poolProps, null);
}

@Test
public void testGetConnectionNoConnPropsPoolDisabled() throws SQLException {
context.setServerName("test-server");
Expand All @@ -346,14 +485,14 @@ public void testGetConnectionNoConnPropsPoolDisabled() throws SQLException {

assertSame(mockConnection, conn);

verify(mockConnectionManager).getConnection("test-server", "test-url", new Properties(), false, null, null);
verify(mockConnectionManager).getConnection("test-server", "test-url", getDefaultConnectionProperties(), false, null, null);
}

@Test
public void testGetConnectionConnPropsPoolDisabled() throws SQLException {
context.setServerName("test-server");

Properties connProps = new Properties();
Properties connProps = getDefaultConnectionProperties();
connProps.setProperty("foo", "foo-val");
connProps.setProperty("bar", "bar-val");

Expand Down Expand Up @@ -392,7 +531,7 @@ public void testGetConnectionConnPropsPoolEnabledNoPoolProps() throws SQLExcepti

assertSame(mockConnection, conn);

Properties connProps = new Properties();
Properties connProps = getDefaultConnectionProperties();
connProps.setProperty("foo", "foo-val");
connProps.setProperty("bar", "bar-val");

Expand All @@ -419,7 +558,7 @@ public void testGetConnectionConnPropsPoolEnabledWithQualifier() throws SQLExcep

assertSame(mockConnection, conn);

Properties connProps = new Properties();
Properties connProps = getDefaultConnectionProperties();
connProps.setProperty("foo", "foo-val");
connProps.setProperty("bar", "bar-val");

Expand Down Expand Up @@ -447,7 +586,7 @@ public void testGetConnectionConnPropsPoolEnabledPoolProps() throws SQLException

assertSame(mockConnection, conn);

Properties connProps = new Properties();
Properties connProps = getDefaultConnectionProperties();
connProps.setProperty("foo", "foo-val");
connProps.setProperty("bar", "bar-val");

Expand Down Expand Up @@ -478,7 +617,7 @@ public void testGetConnectionConnPropsPoolDisabledPoolProps() throws SQLExceptio

assertSame(mockConnection, conn);

Properties connProps = new Properties();
Properties connProps = getDefaultConnectionProperties();
connProps.setProperty("foo", "foo-val");
connProps.setProperty("bar", "bar-val");

Expand Down
Loading
Loading