diff --git a/pgjdbc/src/main/java/org/postgresql/PGProperty.java b/pgjdbc/src/main/java/org/postgresql/PGProperty.java index b342b44c9c..9e99dc7a3a 100644 --- a/pgjdbc/src/main/java/org/postgresql/PGProperty.java +++ b/pgjdbc/src/main/java/org/postgresql/PGProperty.java @@ -172,6 +172,12 @@ public enum PGProperty { LOG_UNCLOSED_CONNECTIONS("logUnclosedConnections", "false", "When connections that are not explicitly closed are garbage collected, log the stacktrace from the opening of the connection to trace the leak source"), + /** + * Whether to include full server error detail in exception messages. + */ + LOG_SERVER_ERROR_DETAIL("logServerErrorDetail", "true", + "Include full server error detail in exception messages. If disabled then only the error itself will be included."), + /** * Enable optimization that disables column name sanitiser. */ diff --git a/pgjdbc/src/main/java/org/postgresql/core/QueryExecutorBase.java b/pgjdbc/src/main/java/org/postgresql/core/QueryExecutorBase.java index fbe5c4980b..1a74a7f73b 100644 --- a/pgjdbc/src/main/java/org/postgresql/core/QueryExecutorBase.java +++ b/pgjdbc/src/main/java/org/postgresql/core/QueryExecutorBase.java @@ -45,6 +45,7 @@ public abstract class QueryExecutorBase implements QueryExecutor { private final PreferQueryMode preferQueryMode; private AutoSave autoSave; private boolean flushCacheOnDeallocate = true; + protected final boolean logServerErrorDetail; // default value for server versions that don't report standard_conforming_strings private boolean standardConformingStrings = false; @@ -70,6 +71,7 @@ protected QueryExecutorBase(PGStream pgStream, String user, String preferMode = PGProperty.PREFER_QUERY_MODE.get(info); this.preferQueryMode = PreferQueryMode.of(preferMode); this.autoSave = AutoSave.of(PGProperty.AUTOSAVE.get(info)); + this.logServerErrorDetail = PGProperty.LOG_SERVER_ERROR_DETAIL.getBoolean(info); this.cachedQueryCreateAction = new CachedQueryCreateAction(this); statementCache = new LruCache( Math.max(0, PGProperty.PREPARED_STATEMENT_CACHE_QUERIES.getInt(info)), diff --git a/pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java b/pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java index 662cc263c8..1efcd52798 100644 --- a/pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java +++ b/pgjdbc/src/main/java/org/postgresql/core/v3/ConnectionFactoryImpl.java @@ -517,7 +517,7 @@ private void doAuthentication(PGStream pgStream, String host, String user, Prope ServerErrorMessage errorMsg = new ServerErrorMessage(pgStream.receiveErrorString(elen - 4)); LOGGER.log(Level.FINEST, " <=BE ErrorMessage({0})", errorMsg); - throw new PSQLException(errorMsg); + throw new PSQLException(errorMsg, PGProperty.LOG_SERVER_ERROR_DETAIL.getBoolean(info)); case 'R': // Authentication request. @@ -647,7 +647,8 @@ private void doAuthentication(PGStream pgStream, String host, String user, Prope org.postgresql.gss.MakeGSS.authenticate(pgStream, host, user, password, PGProperty.JAAS_APPLICATION_NAME.get(info), PGProperty.KERBEROS_SERVER_NAME.get(info), usespnego, - PGProperty.JAAS_LOGIN.getBoolean(info)); + PGProperty.JAAS_LOGIN.getBoolean(info), + PGProperty.LOG_SERVER_ERROR_DETAIL.getBoolean(info)); } break; diff --git a/pgjdbc/src/main/java/org/postgresql/core/v3/QueryExecutorImpl.java b/pgjdbc/src/main/java/org/postgresql/core/v3/QueryExecutorImpl.java index 0b889022e0..48ca673bf9 100644 --- a/pgjdbc/src/main/java/org/postgresql/core/v3/QueryExecutorImpl.java +++ b/pgjdbc/src/main/java/org/postgresql/core/v3/QueryExecutorImpl.java @@ -2500,7 +2500,7 @@ private SQLException receiveErrorResponse() throws IOException { LOGGER.log(Level.FINEST, " <=BE ErrorMessage({0})", errorMsg.toString()); } - PSQLException error = new PSQLException(errorMsg); + PSQLException error = new PSQLException(errorMsg, this.logServerErrorDetail); if (transactionFailCause == null) { transactionFailCause = error; } else { diff --git a/pgjdbc/src/main/java/org/postgresql/ds/common/BaseDataSource.java b/pgjdbc/src/main/java/org/postgresql/ds/common/BaseDataSource.java index 64e9f04b0e..f4be819749 100644 --- a/pgjdbc/src/main/java/org/postgresql/ds/common/BaseDataSource.java +++ b/pgjdbc/src/main/java/org/postgresql/ds/common/BaseDataSource.java @@ -928,6 +928,22 @@ public void setLogUnclosedConnections(boolean enabled) { PGProperty.LOG_UNCLOSED_CONNECTIONS.set(properties, enabled); } + /** + * @return true if driver should log include detail in server error messages + * @see PGProperty#LOG_SERVER_ERROR_DETAIL + */ + public boolean getLogServerErrorDetail() { + return PGProperty.LOG_SERVER_ERROR_DETAIL.getBoolean(properties); + } + + /** + * @param enabled true if driver should include detail in server error messages + * @see PGProperty#LOG_SERVER_ERROR_DETAIL + */ + public void setLogServerErrorDetail(boolean enabled) { + PGProperty.LOG_SERVER_ERROR_DETAIL.set(properties, enabled); + } + /** * @return assumed minimal server version * @see PGProperty#ASSUME_MIN_SERVER_VERSION diff --git a/pgjdbc/src/main/java/org/postgresql/gss/GssAction.java b/pgjdbc/src/main/java/org/postgresql/gss/GssAction.java index fa27e30db1..f747fd4aaa 100644 --- a/pgjdbc/src/main/java/org/postgresql/gss/GssAction.java +++ b/pgjdbc/src/main/java/org/postgresql/gss/GssAction.java @@ -32,15 +32,17 @@ class GssAction implements PrivilegedAction { private final String kerberosServerName; private final boolean useSpnego; private final GSSCredential clientCredentials; + private final boolean logServerErrorDetail; GssAction(PGStream pgStream, GSSCredential clientCredentials, String host, String user, - String kerberosServerName, boolean useSpnego) { + String kerberosServerName, boolean useSpnego, boolean logServerErrorDetail) { this.pgStream = pgStream; this.clientCredentials = clientCredentials; this.host = host; this.user = user; this.kerberosServerName = kerberosServerName; this.useSpnego = useSpnego; + this.logServerErrorDetail = logServerErrorDetail; } private static boolean hasSpnegoSupport(GSSManager manager) throws GSSException { @@ -111,7 +113,7 @@ public Exception run() { LOGGER.log(Level.FINEST, " <=BE ErrorMessage({0})", errorMsg); - return new PSQLException(errorMsg); + return new PSQLException(errorMsg, logServerErrorDetail); case 'R': LOGGER.log(Level.FINEST, " <=BE AuthenticationGSSContinue"); int len = pgStream.receiveInteger4(); diff --git a/pgjdbc/src/main/java/org/postgresql/gss/MakeGSS.java b/pgjdbc/src/main/java/org/postgresql/gss/MakeGSS.java index 84c2f35fee..b3b856b490 100644 --- a/pgjdbc/src/main/java/org/postgresql/gss/MakeGSS.java +++ b/pgjdbc/src/main/java/org/postgresql/gss/MakeGSS.java @@ -28,7 +28,8 @@ public class MakeGSS { private static final Logger LOGGER = Logger.getLogger(MakeGSS.class.getName()); public static void authenticate(PGStream pgStream, String host, String user, String password, - String jaasApplicationName, String kerberosServerName, boolean useSpnego, boolean jaasLogin) + String jaasApplicationName, String kerberosServerName, boolean useSpnego, boolean jaasLogin, + boolean logServerErrorDetail) throws IOException, SQLException { LOGGER.log(Level.FINEST, " <=BE AuthenticationReqGSS"); @@ -58,7 +59,7 @@ public static void authenticate(PGStream pgStream, String host, String user, Str sub = lc.getSubject(); } PrivilegedAction action = new GssAction(pgStream, gssCredential, host, user, - kerberosServerName, useSpnego); + kerberosServerName, useSpnego, logServerErrorDetail); result = Subject.doAs(sub, action); } catch (Exception e) { diff --git a/pgjdbc/src/main/java/org/postgresql/util/PSQLException.java b/pgjdbc/src/main/java/org/postgresql/util/PSQLException.java index 370ce8a9f2..0e3cd47677 100644 --- a/pgjdbc/src/main/java/org/postgresql/util/PSQLException.java +++ b/pgjdbc/src/main/java/org/postgresql/util/PSQLException.java @@ -20,7 +20,11 @@ public PSQLException(String msg, PSQLState state) { } public PSQLException(ServerErrorMessage serverError) { - super(serverError.toString(), serverError.getSQLState()); + this(serverError, true); + } + + public PSQLException(ServerErrorMessage serverError, boolean detail) { + super(detail ? serverError.toString() : serverError.getNonSensitiveErrorMessage(), serverError.getSQLState()); this.serverError = serverError; } diff --git a/pgjdbc/src/main/java/org/postgresql/util/ServerErrorMessage.java b/pgjdbc/src/main/java/org/postgresql/util/ServerErrorMessage.java index d44b464de0..7b3f62c208 100644 --- a/pgjdbc/src/main/java/org/postgresql/util/ServerErrorMessage.java +++ b/pgjdbc/src/main/java/org/postgresql/util/ServerErrorMessage.java @@ -143,6 +143,19 @@ private int getIntegerPart(Character c) { return Integer.parseInt(s); } + String getNonSensitiveErrorMessage() { + StringBuilder totalMessage = new StringBuilder(); + String message = mesgParts.get(SEVERITY); + if (message != null) { + totalMessage.append(message).append(": "); + } + message = mesgParts.get(MESSAGE); + if (message != null) { + totalMessage.append(message); + } + return totalMessage.toString(); + } + public String toString() { // Now construct the message from what the server sent // The general format is: diff --git a/pgjdbc/src/test/java/org/postgresql/test/core/LogServerMessagePropertyTest.java b/pgjdbc/src/test/java/org/postgresql/test/core/LogServerMessagePropertyTest.java new file mode 100644 index 0000000000..daab69354b --- /dev/null +++ b/pgjdbc/src/test/java/org/postgresql/test/core/LogServerMessagePropertyTest.java @@ -0,0 +1,100 @@ +/* + * Copyright (c) 2019, PostgreSQL Global Development Group + * See the LICENSE file in the project root for more information. + */ + +package org.postgresql.test.core; + +import org.postgresql.PGProperty; +import org.postgresql.core.ServerVersion; +import org.postgresql.test.TestUtil; + +import org.junit.Assert; +import org.junit.Assume; +import org.junit.Test; + +import java.sql.Connection; +import java.sql.SQLException; +import java.util.Properties; + +public class LogServerMessagePropertyTest { + private static final String PRIMARY_KEY_NAME = "lms_test_pk"; + private static final String CREATE_TABLE_SQL = + "CREATE TABLE pg_temp.lms_test (" + + " id text, " + + " CONSTRAINT " + PRIMARY_KEY_NAME + " PRIMARY KEY (id)" + + ")"; + private static final String SECRET_VALUE = "some_secret_value"; + private static final String INSERT_SQL = + "INSERT INTO pg_temp.lms_test (id) VALUES ('" + SECRET_VALUE + "')"; + private static final String UNIQUE_VIOLATION_SQL_STATE = "23505"; + + /** + * Creates a connection with the additional properties, use it to + * create a temp table with a primary key, run two inserts to generate + * a duplicate key error, and finally return the exception message. + */ + private static String testViolatePrimaryKey(Properties props) throws SQLException { + Connection conn = TestUtil.openDB(props); + Assume.assumeTrue(TestUtil.haveMinimumServerVersion(conn, ServerVersion.v9_1)); + try { + TestUtil.execute(CREATE_TABLE_SQL, conn); + // First insert should work + TestUtil.execute(INSERT_SQL, conn); + // Second insert should throw a duplicate key error + TestUtil.execute(INSERT_SQL, conn); + } catch (SQLException e) { + Assert.assertEquals("SQL state must be for a unique violation", UNIQUE_VIOLATION_SQL_STATE, e.getSQLState()); + return e.getMessage(); + } finally { + conn.close(); + } + // Should never get here: + Assert.fail("A duplicate key exception should have occurred"); + return null; + } + + private static void assertMessageContains(String message, String text) { + if (message.toLowerCase().indexOf(text.toLowerCase()) < 0) { + Assert.fail(String.format("Message must contain text '%s': %s", text, message)); + } + } + + private static void assertMessageDoesNotContain(String message, String text) { + if (message.toLowerCase().indexOf(text.toLowerCase()) >= 0) { + Assert.fail(String.format("Message must not contain text '%s': %s", text, message)); + } + } + + @Test + public void testWithDefaults() throws SQLException { + Properties props = new Properties(); + String message = testViolatePrimaryKey(props); + assertMessageContains(message, PRIMARY_KEY_NAME); + assertMessageContains(message, "Detail:"); + assertMessageContains(message, SECRET_VALUE); + } + + /** + * NOTE: This should be the same as the default case as "true" is the default. + */ + @Test + public void testWithExplicitlyEnabled() throws SQLException { + Properties props = new Properties(); + props.setProperty(PGProperty.LOG_SERVER_ERROR_DETAIL.getName(), "true"); + String message = testViolatePrimaryKey(props); + assertMessageContains(message, PRIMARY_KEY_NAME); + assertMessageContains(message, "Detail:"); + assertMessageContains(message, SECRET_VALUE); + } + + @Test + public void testWithLogServerErrorDetailDisabled() throws SQLException { + Properties props = new Properties(); + props.setProperty(PGProperty.LOG_SERVER_ERROR_DETAIL.getName(), "false"); + String message = testViolatePrimaryKey(props); + assertMessageContains(message, PRIMARY_KEY_NAME); + assertMessageDoesNotContain(message, "Detail:"); + assertMessageDoesNotContain(message, SECRET_VALUE); + } +} diff --git a/pgjdbc/src/test/java/org/postgresql/test/jdbc2/Jdbc2TestSuite.java b/pgjdbc/src/test/java/org/postgresql/test/jdbc2/Jdbc2TestSuite.java index 01d9fbf93f..7dc50232d0 100644 --- a/pgjdbc/src/test/java/org/postgresql/test/jdbc2/Jdbc2TestSuite.java +++ b/pgjdbc/src/test/java/org/postgresql/test/jdbc2/Jdbc2TestSuite.java @@ -15,6 +15,7 @@ import org.postgresql.jdbc.DeepBatchedInsertStatementTest; import org.postgresql.jdbc.PrimitiveArraySupportTest; import org.postgresql.test.core.JavaVersionTest; +import org.postgresql.test.core.LogServerMessagePropertyTest; import org.postgresql.test.core.NativeQueryBindLengthTest; import org.postgresql.test.core.OptionsPropertyTest; import org.postgresql.test.util.ExpressionPropertiesTest; @@ -67,6 +68,7 @@ JavaVersionTest.class, JBuilderTest.class, LoginTimeoutTest.class, + LogServerMessagePropertyTest.class, LruCacheTest.class, MiscTest.class, NativeQueryBindLengthTest.class,