Skip to content

Commit

Permalink
Merge pull request #331 from alfasoftware/fix/validate-Postgres-conne…
Browse files Browse the repository at this point in the history
…ction-getStandardConformingStrings

Adding validation preventing standard_conforming_strings=off
  • Loading branch information
gilleain authored Nov 18, 2024
2 parents ee269c0 + b76749c commit 4c8c4f0
Show file tree
Hide file tree
Showing 2 changed files with 128 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package org.alfasoftware.morf.jdbc;


import java.lang.reflect.InvocationTargetException;
import java.sql.Array;
import java.sql.Blob;
import java.sql.CallableStatement;
Expand Down Expand Up @@ -44,6 +45,7 @@
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;

/**
Expand Down Expand Up @@ -293,10 +295,17 @@ public Connection getConnection(String username, String password) throws SQLExce
log.info("Opening new database connection to [" + AbstractConnectionResources.this.getJdbcUrl() + "] with username [" + username + "] for schema [" + AbstractConnectionResources.this.getSchemaName() + "]");
loadJdbcDriver();
Connection connection = openConnection(username, password);
validateConnection(connection);
return log.isDebugEnabled() ? new LoggingConnection(connection) : connection;
}


private void validateConnection(Connection connection) {
final String databaseType = getDatabaseType();
validatePgConnectionStandardConformingStrings(databaseType, connection);
}


private void loadJdbcDriver() {
String driverClassName = findDatabaseType().driverClassName();
try {
Expand Down Expand Up @@ -327,6 +336,35 @@ private Connection openConnection(String username, String password) throws SQLEx
}


@VisibleForTesting
static void validatePgConnectionStandardConformingStrings(String databaseType, Connection connection) {
final String getStandardConformingStringsMethodName = "getStandardConformingStrings";

if ("PGSQL".equals(databaseType)) {
try {
// see org.postgresql.core.BaseConnection.getStandardConformingStrings()
// see org.postgresql.jdbc.PgConnection.getStandardConformingStrings()
boolean standardConformingStrings = connection.getClass()
.getMethod(getStandardConformingStringsMethodName)
.invoke(connection)
.equals(Boolean.TRUE);

if (log.isDebugEnabled()) log.debug(getStandardConformingStringsMethodName + "(): " + standardConformingStrings);

if (!standardConformingStrings) {
throw new IllegalStateException("Postgres: standard_conforming_strings=off config value has been detected, which is not supported");
}
}
catch (NoSuchMethodException e) {
throw new IllegalStateException(getStandardConformingStringsMethodName + "(): no such method on " + connection, e);
}
catch (IllegalAccessException | IllegalArgumentException | InvocationTargetException | SecurityException e) {
throw new IllegalStateException(getStandardConformingStringsMethodName + "(): invoking such method on " + connection, e);
}
}
}


private static final class LoggingConnection implements Connection {

final int id = new Random().nextInt(0xFFFF);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
package org.alfasoftware.morf.jdbc;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.assertThrows;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;

import java.sql.Connection;

import org.hamcrest.Matchers;
import org.junit.Test;
import org.mockito.Mockito;

public class TestAbstractConnectionResources {

/**
* Test happy path for non-Postgres connections.
*/
@Test
public void testValidatePgConnectionStandardConformingStringsOther() {
AbstractConnectionResources.validatePgConnectionStandardConformingStrings("OTHER", mock(Connection.class));
}


/**
* Test happy path for Postgres connections, where standard_conforming_strings is on, which is good.
*/
@Test
public void testValidatePgConnectionStandardConformingStringsTrue() {
MockPgConnection mockPgConnection = mock(MockPgConnection.class);
when(mockPgConnection.getStandardConformingStrings()).thenReturn(true);

AbstractConnectionResources.validatePgConnectionStandardConformingStrings("PGSQL", mockPgConnection);
}


/**
* Test unhappy path for Postgres connections, where standard_conforming_strings is off, and therefore wrong.
*/
@Test
public void testValidatePgConnectionStandardConformingStringsFalse() {
MockPgConnection mockPgConnection = mock(MockPgConnection.class);
when(mockPgConnection.getStandardConformingStrings()).thenReturn(false);

IllegalStateException exception = assertThrows(IllegalStateException.class,
() -> AbstractConnectionResources.validatePgConnectionStandardConformingStrings("PGSQL", mockPgConnection));

assertThat(exception.getMessage(), Matchers.containsString("standard_conforming_strings=off"));
}


/**
* Test unhappy path for Postgres connections, where we cannot find the expected method.
*/
@Test
public void testValidatePgConnectionStandardConformingStringsPrivateMethod() {
IllegalStateException exception = assertThrows(IllegalStateException.class,
() -> AbstractConnectionResources.validatePgConnectionStandardConformingStrings("PGSQL", mock(PrivatePgConnection.class, Mockito.CALLS_REAL_METHODS)));

assertThat(exception.getMessage(), Matchers.containsString("invoking such method"));
}


/**
* Test unhappy path for Postgres connections, where we cannot invoke the expected method.
*/
@Test
public void testValidatePgConnectionStandardConformingStringsMissingMethod() {
IllegalStateException exception = assertThrows(IllegalStateException.class,
() -> AbstractConnectionResources.validatePgConnectionStandardConformingStrings("PGSQL", mock(Connection.class)));

assertThat(exception.getMessage(), Matchers.containsString("no such method"));
}


/** Helper interface for testing */
private interface MockPgConnection extends Connection {
// see org.postgresql.core.BaseConnection.getStandardConformingStrings()
boolean getStandardConformingStrings();
}


/** Helper class for testing */
abstract class PrivatePgConnection implements Connection {
// see org.postgresql.core.BaseConnection.getStandardConformingStrings()
public boolean getStandardConformingStrings() throws java.lang.reflect.InvocationTargetException {
throw new java.lang.reflect.InvocationTargetException(null); // emulating reflection issues
}
}
}

0 comments on commit 4c8c4f0

Please sign in to comment.