Skip to content
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.

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

Add connection property to limit server error detail in exception exceptions #1579

Merged
merged 3 commits into from Nov 14, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions pgjdbc/src/main/java/org/postgresql/PGProperty.java
Expand Up @@ -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.
*/
Expand Down
Expand Up @@ -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;
Expand All @@ -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<Object, CachedQuery>(
Math.max(0, PGProperty.PREPARED_STATEMENT_CACHE_QUERIES.getInt(info)),
Expand Down
Expand Up @@ -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.
Expand Down Expand Up @@ -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;

Expand Down
Expand Up @@ -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 {
Expand Down
16 changes: 16 additions & 0 deletions pgjdbc/src/main/java/org/postgresql/ds/common/BaseDataSource.java
Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions pgjdbc/src/main/java/org/postgresql/gss/GssAction.java
Expand Up @@ -32,15 +32,17 @@ class GssAction implements PrivilegedAction<Exception> {
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 {
Expand Down Expand Up @@ -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();
Expand Down
5 changes: 3 additions & 2 deletions pgjdbc/src/main/java/org/postgresql/gss/MakeGSS.java
Expand Up @@ -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");

Expand Down Expand Up @@ -58,7 +59,7 @@ public static void authenticate(PGStream pgStream, String host, String user, Str
sub = lc.getSubject();
}
PrivilegedAction<Exception> action = new GssAction(pgStream, gssCredential, host, user,
kerberosServerName, useSpnego);
kerberosServerName, useSpnego, logServerErrorDetail);

result = Subject.doAs(sub, action);
} catch (Exception e) {
Expand Down
6 changes: 5 additions & 1 deletion pgjdbc/src/main/java/org/postgresql/util/PSQLException.java
Expand Up @@ -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;
}

Expand Down
13 changes: 13 additions & 0 deletions pgjdbc/src/main/java/org/postgresql/util/ServerErrorMessage.java
Expand Up @@ -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:
Expand Down
@@ -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);
}
}
Expand Up @@ -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;
Expand Down Expand Up @@ -67,6 +68,7 @@
JavaVersionTest.class,
JBuilderTest.class,
LoginTimeoutTest.class,
LogServerMessagePropertyTest.class,
LruCacheTest.class,
MiscTest.class,
NativeQueryBindLengthTest.class,
Expand Down