Skip to content

Commit

Permalink
fix: logger should be generally quiet (#1187)
Browse files Browse the repository at this point in the history
The logger used in the driver has some calls to levels WARNING and SEVERE
this lower the level to FINE to make the logger quiet in some cases.

fixes #1009
fixes #1162
closes #1149
  • Loading branch information
jorsol authored and vlsi committed Jul 1, 2018
1 parent 435e2f7 commit 30f06e1
Show file tree
Hide file tree
Showing 8 changed files with 32 additions and 36 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ Notable changes since version 42.0.0, read the complete [History of Changes](htt
The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/). The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).


## [Unreleased] ## [Unreleased]
### Changed
- Avoid the print of highest logger levels when the exception is re-thrown. [PR 1187](https://github.com/pgjdbc/pgjdbc/pull/1187)


## [42.2.2] (2018-03-15) ## [42.2.2] (2018-03-15)
### Added ### Added
Expand Down
23 changes: 15 additions & 8 deletions pgjdbc/src/main/java/org/postgresql/Driver.java
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -122,11 +122,14 @@ private Properties loadDefaultProperties() throws IOException {
// neither case can throw SecurityException. // neither case can throw SecurityException.
ClassLoader cl = getClass().getClassLoader(); ClassLoader cl = getClass().getClassLoader();
if (cl == null) { if (cl == null) {
LOGGER.log(Level.FINE, "Can't find our classloader for the Driver; "
+ "attempt to use the system class loader");
cl = ClassLoader.getSystemClassLoader(); cl = ClassLoader.getSystemClassLoader();
} }


if (cl == null) { if (cl == null) {
LOGGER.log(Level.WARNING, "Can't find a classloader for the Driver; not loading driver configuration"); LOGGER.log(Level.WARNING, "Can't find a classloader for the Driver; not loading driver "
+ "configuration from org/postgresql/driverconfig.properties");
return merged; // Give up on finding defaults. return merged; // Give up on finding defaults.
} }


Expand Down Expand Up @@ -233,7 +236,6 @@ public Connection connect(String url, Properties info) throws SQLException {
} }
// parse URL and add more properties // parse URL and add more properties
if ((props = parseURL(url, props)) == null) { if ((props = parseURL(url, props)) == null) {
LOGGER.log(Level.SEVERE, "Error in url: {0}", url);
return null; return null;
} }
try { try {
Expand Down Expand Up @@ -261,7 +263,7 @@ public Connection connect(String url, Properties info) throws SQLException {
thread.start(); thread.start();
return ct.getResult(timeout); return ct.getResult(timeout);
} catch (PSQLException ex1) { } catch (PSQLException ex1) {
LOGGER.log(Level.SEVERE, "Connection error: ", ex1); LOGGER.log(Level.FINE, "Connection error: ", ex1);
// re-throw the exception, otherwise it will be caught next, and a // re-throw the exception, otherwise it will be caught next, and a
// org.postgresql.unusual error will be returned instead. // org.postgresql.unusual error will be returned instead.
throw ex1; throw ex1;
Expand All @@ -271,7 +273,7 @@ public Connection connect(String url, Properties info) throws SQLException {
"Your security policy has prevented the connection from being attempted. You probably need to grant the connect java.net.SocketPermission to the database server host and port that you wish to connect to."), "Your security policy has prevented the connection from being attempted. You probably need to grant the connect java.net.SocketPermission to the database server host and port that you wish to connect to."),
PSQLState.UNEXPECTED_ERROR, ace); PSQLState.UNEXPECTED_ERROR, ace);
} catch (Exception ex2) { } catch (Exception ex2) {
LOGGER.log(Level.SEVERE, "Unexpected connection error: ", ex2); LOGGER.log(Level.FINE, "Unexpected connection error: ", ex2);
throw new PSQLException( throw new PSQLException(
GT.tr( GT.tr(
"Something unusual has occurred to cause the driver to fail. Please report this exception."), "Something unusual has occurred to cause the driver to fail. Please report this exception."),
Expand Down Expand Up @@ -552,6 +554,7 @@ public static Properties parseURL(String url, Properties defaults) {
} }


if (!l_urlServer.startsWith("jdbc:postgresql:")) { if (!l_urlServer.startsWith("jdbc:postgresql:")) {
LOGGER.log(Level.FINE, "JDBC URL must start with \"jdbc:postgresql:\" but was: {0}", url);
return null; return null;
} }
l_urlServer = l_urlServer.substring("jdbc:postgresql:".length()); l_urlServer = l_urlServer.substring("jdbc:postgresql:".length());
Expand All @@ -560,6 +563,7 @@ public static Properties parseURL(String url, Properties defaults) {
l_urlServer = l_urlServer.substring(2); l_urlServer = l_urlServer.substring(2);
int slash = l_urlServer.indexOf('/'); int slash = l_urlServer.indexOf('/');
if (slash == -1) { if (slash == -1) {
LOGGER.log(Level.WARNING, "JDBC URL must contain a slash at the end of the host or port: {0}", url);
return null; return null;
} }
urlProps.setProperty("PGDBNAME", URLCoder.decode(l_urlServer.substring(slash + 1))); urlProps.setProperty("PGDBNAME", URLCoder.decode(l_urlServer.substring(slash + 1)));
Expand All @@ -572,10 +576,13 @@ public static Properties parseURL(String url, Properties defaults) {
if (portIdx != -1 && address.lastIndexOf(']') < portIdx) { if (portIdx != -1 && address.lastIndexOf(']') < portIdx) {
String portStr = address.substring(portIdx + 1); String portStr = address.substring(portIdx + 1);
try { try {
// squid:S2201 The return value of "parseInt" must be used. int port = Integer.parseInt(portStr);
// The side effect is NumberFormatException, thus ignore sonar error here if (port < 1 || port > 65535) {
Integer.parseInt(portStr); //NOSONAR LOGGER.log(Level.WARNING, "JDBC URL port in invalid range: {0}", portStr);
} catch (NumberFormatException ex) { return null;
}
} catch (NumberFormatException ignore) {
LOGGER.log(Level.WARNING, "JDBC URL invalid port number: {0}", portStr);
return null; return null;
} }
ports.append(portStr); ports.append(portStr);
Expand Down
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -67,12 +67,6 @@ public class ConnectionFactoryImpl extends ConnectionFactory {
private static final int AUTH_REQ_SASL_CONTINUE = 11; private static final int AUTH_REQ_SASL_CONTINUE = 11;
private static final int AUTH_REQ_SASL_FINAL = 12; private static final int AUTH_REQ_SASL_FINAL = 12;


/**
* Marker exception; thrown when we want to fall back to using V2.
*/
private static class UnsupportedProtocolException extends IOException {
}

private ISSPIClient createSSPI(PGStream pgStream, private ISSPIClient createSSPI(PGStream pgStream,
String spnServiceClass, String spnServiceClass,
boolean enableNegotiate) { boolean enableNegotiate) {
Expand Down Expand Up @@ -230,18 +224,13 @@ public QueryExecutor openConnectionImpl(HostSpec[] hostSpecs, String user, Strin


// And we're done. // And we're done.
return queryExecutor; return queryExecutor;
} catch (UnsupportedProtocolException upe) {
// Swallow this and return null so ConnectionFactory tries the next protocol.
LOGGER.log(Level.SEVERE, "Protocol not supported, abandoning connection.", upe);
closeStream(newStream);
return null;
} catch (ConnectException cex) { } catch (ConnectException cex) {
// Added by Peter Mount <peter@retep.org.uk> // Added by Peter Mount <peter@retep.org.uk>
// ConnectException is thrown when the connection cannot be made. // ConnectException is thrown when the connection cannot be made.
// we trap this an return a more meaningful message for the end user // we trap this an return a more meaningful message for the end user
GlobalHostStatusTracker.reportHostStatus(hostSpec, HostStatus.ConnectFail); GlobalHostStatusTracker.reportHostStatus(hostSpec, HostStatus.ConnectFail);
knownStates.put(hostSpec, HostStatus.ConnectFail); knownStates.put(hostSpec, HostStatus.ConnectFail);
log(Level.WARNING, "ConnectException occurred while connecting to {0}", cex, hostSpec); log(Level.FINE, "ConnectException occurred while connecting to {0}", cex, hostSpec);
if (hostIter.hasNext()) { if (hostIter.hasNext()) {
// still more addresses to try // still more addresses to try
continue; continue;
Expand All @@ -253,7 +242,7 @@ public QueryExecutor openConnectionImpl(HostSpec[] hostSpecs, String user, Strin
closeStream(newStream); closeStream(newStream);
GlobalHostStatusTracker.reportHostStatus(hostSpec, HostStatus.ConnectFail); GlobalHostStatusTracker.reportHostStatus(hostSpec, HostStatus.ConnectFail);
knownStates.put(hostSpec, HostStatus.ConnectFail); knownStates.put(hostSpec, HostStatus.ConnectFail);
log(Level.WARNING, "IOException occurred while connecting to {0}", ioe, hostSpec); log(Level.FINE, "IOException occurred while connecting to {0}", ioe, hostSpec);
if (hostIter.hasNext()) { if (hostIter.hasNext()) {
// still more addresses to try // still more addresses to try
continue; continue;
Expand All @@ -262,7 +251,7 @@ public QueryExecutor openConnectionImpl(HostSpec[] hostSpecs, String user, Strin
PSQLState.CONNECTION_UNABLE_TO_CONNECT, ioe); PSQLState.CONNECTION_UNABLE_TO_CONNECT, ioe);
} catch (SQLException se) { } catch (SQLException se) {
closeStream(newStream); closeStream(newStream);
log(Level.WARNING, "SQLException occurred while connecting to {0}", se, hostSpec); log(Level.FINE, "SQLException occurred while connecting to {0}", se, hostSpec);
GlobalHostStatusTracker.reportHostStatus(hostSpec, HostStatus.ConnectFail); GlobalHostStatusTracker.reportHostStatus(hostSpec, HostStatus.ConnectFail);
knownStates.put(hostSpec, HostStatus.ConnectFail); knownStates.put(hostSpec, HostStatus.ConnectFail);
if (hostIter.hasNext()) { if (hostIter.hasNext()) {
Expand Down Expand Up @@ -467,11 +456,6 @@ private void doAuthentication(PGStream pgStream, String host, String user, Prope
// "User authentication failed" // "User authentication failed"
// //
int l_elen = pgStream.receiveInteger4(); int l_elen = pgStream.receiveInteger4();
if (l_elen > 30000) {
// if the error length is > than 30000 we assume this is really a v2 protocol
// server, so trigger fallback.
throw new UnsupportedProtocolException();
}


ServerErrorMessage errorMsg = ServerErrorMessage errorMsg =
new ServerErrorMessage(pgStream.receiveErrorString(l_elen - 4)); new ServerErrorMessage(pgStream.receiveErrorString(l_elen - 4));
Expand Down Expand Up @@ -668,7 +652,7 @@ private void doAuthentication(PGStream pgStream, String host, String user, Prope
try { try {
sspiClient.dispose(); sspiClient.dispose();
} catch (RuntimeException ex) { } catch (RuntimeException ex) {
LOGGER.log(Level.WARNING, "Unexpected error during SSPI context disposal", ex); LOGGER.log(Level.FINE, "Unexpected error during SSPI context disposal", ex);
} }


} }
Expand Down
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -2608,7 +2608,7 @@ public void receiveParameterStatus() throws IOException, SQLException {
if (name.equals("client_encoding")) { if (name.equals("client_encoding")) {
if (allowEncodingChanges) { if (allowEncodingChanges) {
if (!value.equalsIgnoreCase("UTF8") && !value.equalsIgnoreCase("UTF-8")) { if (!value.equalsIgnoreCase("UTF8") && !value.equalsIgnoreCase("UTF-8")) {
LOGGER.log(Level.WARNING, LOGGER.log(Level.FINE,
"pgjdbc expects client_encoding to be UTF8 for proper operation. Actual encoding is {0}", "pgjdbc expects client_encoding to be UTF8 for proper operation. Actual encoding is {0}",
value); value);
} }
Expand Down
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public Connection getConnection(String user, String password) throws SQLExceptio
} }
return con; return con;
} catch (SQLException e) { } catch (SQLException e) {
LOGGER.log(Level.SEVERE, "Failed to create a {0} for {1} at {2}: {3}", LOGGER.log(Level.FINE, "Failed to create a {0} for {1} at {2}: {3}",
new Object[]{getDescription(), user, getUrl(), e}); new Object[]{getDescription(), user, getUrl(), e});
throw e; throw e;
} }
Expand Down
2 changes: 1 addition & 1 deletion pgjdbc/src/main/java/org/postgresql/jdbc/PgConnection.java
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -1386,7 +1386,7 @@ public boolean isValid(int timeout) throws SQLException {
// "current transaction aborted", assume the connection is up and running // "current transaction aborted", assume the connection is up and running
return true; return true;
} }
LOGGER.log(Level.WARNING, GT.tr("Validating connection."), e); LOGGER.log(Level.FINE, GT.tr("Validating connection."), e);
} }
return false; return false;
} }
Expand Down
2 changes: 1 addition & 1 deletion pgjdbc/src/main/java/org/postgresql/sspi/SSPIClient.java
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ public boolean isSSPISupported() {
* won't have JNA and this will throw a NoClassDefFoundError. * won't have JNA and this will throw a NoClassDefFoundError.
*/ */
if (!Platform.isWindows()) { if (!Platform.isWindows()) {
LOGGER.log(Level.WARNING, "SSPI not supported: non-Windows host"); LOGGER.log(Level.FINE, "SSPI not supported: non-Windows host");
return false; return false;
} }
/* Waffle must be on the CLASSPATH */ /* Waffle must be on the CLASSPATH */
Expand Down
11 changes: 7 additions & 4 deletions pgjdbc/src/test/java/org/postgresql/test/jdbc2/DriverTest.java
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -65,10 +65,13 @@ public void testAcceptsURL() throws Exception {
verifyUrl(drv, "jdbc:postgresql://[::1]:5740/db", "[::1]", "5740", "db"); verifyUrl(drv, "jdbc:postgresql://[::1]:5740/db", "[::1]", "5740", "db");


// Badly formatted url's // Badly formatted url's
assertTrue(!drv.acceptsURL("jdbc:postgres:test")); assertFalse(drv.acceptsURL("jdbc:postgres:test"));
assertTrue(!drv.acceptsURL("postgresql:test")); assertFalse(drv.acceptsURL("postgresql:test"));
assertTrue(!drv.acceptsURL("db")); assertFalse(drv.acceptsURL("db"));
assertTrue(!drv.acceptsURL("jdbc:postgresql://localhost:5432a/test")); assertFalse(drv.acceptsURL("jdbc:postgresql://localhost:5432a/test"));
assertFalse(drv.acceptsURL("jdbc:postgresql://localhost:500000/test"));
assertFalse(drv.acceptsURL("jdbc:postgresql://localhost:0/test"));
assertFalse(drv.acceptsURL("jdbc:postgresql://localhost:-2/test"));


// failover urls // failover urls
verifyUrl(drv, "jdbc:postgresql://localhost,127.0.0.1:5432/test", "localhost,127.0.0.1", verifyUrl(drv, "jdbc:postgresql://localhost,127.0.0.1:5432/test", "localhost,127.0.0.1",
Expand Down

0 comments on commit 30f06e1

Please sign in to comment.