Skip to content

Commit

Permalink
Merge pull request from GHSA-673j-qm5f-xpv8
Browse files Browse the repository at this point in the history 
Removes loggerFile and loggerLevel properties and defers to java.util.logging to
configure all logging in the driver. Users that previously specifyied custom
logging via those connection properties should change their applications to
instead configure via a java.util.logging properties file or system properties.

The properties and enum values are now deprecated and may be removed in a future
driver version.

Co-authored-by: Dave Cramer <davecramer@gmail.com>
  • Loading branch information
@sehrope @davecramer
sehrope and davecramer committed Feb 15, 2022
1 parent c03664e commit f6d4703
Show file tree
Hide file tree
Showing 9 changed files with 28 additions and 263 deletions.
7 changes: 5 additions & 2 deletions README.md
View file
Expand Up @@ -83,6 +83,11 @@ where:
* **database** (Optional) is the database name. Defaults to the same name as the *user name* used in the connection.
* **propertyX** (Optional) is one or more option connection properties. For more information see *Connection properties*.

### Logging
PgJDBC uses java.util.logging for logging.
To configure log levels and control log output destination (e.g. file or console), configure your java.util.logging properties accordingly for the org.postgresql logger.
Note that the most detailed log levels, "`FINEST`", may include sensitive information such as connection details, query SQL, or command parameters.

#### Connection Properties
In addition to the standard connection parameters the driver supports a number of additional properties which can be used to specify additional driver behaviour specific to PostgreSQL™. These properties may be specified in either the connection URL or an additional Properties object parameter to DriverManager.getConnection.

Expand All @@ -104,8 +109,6 @@ In addition to the standard connection parameters the driver supports a number o
| sslpassword | String | null | The password for the client's ssl key (ignored if sslpasswordcallback is set) |
| sendBufferSize | Integer | -1 | Socket write buffer size |
| receiveBufferSize | Integer | -1 | Socket read buffer size |
| loggerLevel | String | null | Logger level of the driver using java.util.logging. Allowed values: OFF, DEBUG or TRACE. |
| loggerFile | String | null | File name output of the Logger, if set, the Logger will use a FileHandler to write to a specified file. If the parameter is not set or the file can't be created the ConsoleHandler will be used instead. |
| logServerErrorDetail | Boolean | true | Allows server error detail (such as sql statements and values) to be logged and passed on in exceptions. Setting to false will mask these errors so they won't be exposed to users, or logs. |
| allowEncodingChanges | Boolean | false | Allow for changes in client_encoding |
| logUnclosedConnections | Boolean | 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 |
Expand Down
12 changes: 4 additions & 8 deletions docs/documentation/head/connect.md
View file
Expand Up @@ -178,17 +178,13 @@ Connection conn = DriverManager.getConnection(url);

* **loggerLevel** = String

Logger level of the driver. Allowed values: <code>OFF</code>, <code>DEBUG</code> or <code>TRACE</code>.
This enable the <code>java.util.logging.Logger</code> Level of the driver based on the following mapping
of levels: DEBUG -&gt; FINE, TRACE -&gt; FINEST. This property is intended for debug the driver and
not for general SQL query debug.
This property is no longer used by the driver and will be ignored.
All logging configuration is handled by java.util.logging.

* **loggerFile** = String

File name output of the Logger. If set, the Logger will use a <code>java.util.logging.FileHandler</code>
to write to a specified file. If the parameter is not set or the file can’t be created the
<code>java.util.logging.ConsoleHandler</code> will be used instead. This parameter should be use
together with loggerLevel.
This property is no longer used by the driver and will be ignored.
All logging configuration is handled by java.util.logging.

* **allowEncodingChanges** = boolean

Expand Down
74 changes: 3 additions & 71 deletions pgjdbc/src/main/java/org/postgresql/Driver.java
View file
Expand Up @@ -9,10 +9,8 @@

import org.postgresql.jdbc.PgConnection;
import org.postgresql.util.DriverInfo;
import org.postgresql.util.ExpressionProperties;
import org.postgresql.util.GT;
import org.postgresql.util.HostSpec;
import org.postgresql.util.LogWriterHandler;
import org.postgresql.util.PGPropertyPasswordParser;
import org.postgresql.util.PGPropertyServiceParser;
import org.postgresql.util.PGPropertyUtil;
Expand All @@ -39,12 +37,8 @@
import java.util.Properties;
import java.util.Set;
import java.util.concurrent.TimeUnit;
import java.util.logging.ConsoleHandler;
import java.util.logging.Formatter;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.logging.SimpleFormatter;
import java.util.logging.StreamHandler;

/**
* <p>The Java SQL framework allows for multiple database drivers. Each driver should supply a class
Expand Down Expand Up @@ -249,8 +243,6 @@ private Properties loadDefaultProperties() throws IOException {
return null;
}
try {
// Setup java.util.logging.Logger using connection properties.
setupLoggerFromProperties(props);

LOGGER.log(Level.FINE, "Connecting with URL: {0}", url);

Expand Down Expand Up @@ -291,73 +283,13 @@ private Properties loadDefaultProperties() throws IOException {
}
}

// Used to check if the handler file is the same
private static @Nullable String loggerHandlerFile;

/**
* <p>Setup java.util.logging.Logger using connection properties.</p>
*
* <p>See {@link PGProperty#LOGGER_FILE} and {@link PGProperty#LOGGER_FILE}</p>
*
* this is an empty method left here for graalvm
* we removed the ability to setup the logger from properties
* due to a security issue
* @param props Connection Properties
*/
private void setupLoggerFromProperties(final Properties props) {
final String driverLogLevel = PGProperty.LOGGER_LEVEL.get(props);
if (driverLogLevel == null) {
return; // Don't mess with Logger if not set
}
if ("OFF".equalsIgnoreCase(driverLogLevel)) {
PARENT_LOGGER.setLevel(Level.OFF);
return; // Don't mess with Logger if set to OFF
} else if ("DEBUG".equalsIgnoreCase(driverLogLevel)) {
PARENT_LOGGER.setLevel(Level.FINE);
} else if ("TRACE".equalsIgnoreCase(driverLogLevel)) {
PARENT_LOGGER.setLevel(Level.FINEST);
}

ExpressionProperties exprProps = new ExpressionProperties(props, System.getProperties());
final String driverLogFile = PGProperty.LOGGER_FILE.get(exprProps);
if (driverLogFile != null && driverLogFile.equals(loggerHandlerFile)) {
return; // Same file output, do nothing.
}

for (java.util.logging.Handler handlers : PARENT_LOGGER.getHandlers()) {
// Remove previously set Handlers
handlers.close();
PARENT_LOGGER.removeHandler(handlers);
loggerHandlerFile = null;
}

java.util.logging.Handler handler = null;
if (driverLogFile != null) {
try {
handler = new java.util.logging.FileHandler(driverLogFile);
loggerHandlerFile = driverLogFile;
} catch (Exception ex) {
System.err.println("Cannot enable FileHandler, fallback to ConsoleHandler.");
}
}

Formatter formatter = new SimpleFormatter();

if ( handler == null ) {
if (DriverManager.getLogWriter() != null) {
handler = new LogWriterHandler(DriverManager.getLogWriter());
} else if ( DriverManager.getLogStream() != null) {
handler = new StreamHandler(DriverManager.getLogStream(), formatter);
} else {
handler = new ConsoleHandler();
}
} else {
handler.setFormatter(formatter);
}

Level loggerLevel = PARENT_LOGGER.getLevel();
if (loggerLevel != null) {
handler.setLevel(loggerLevel);
}
PARENT_LOGGER.setUseParentHandlers(false);
PARENT_LOGGER.addHandler(handler);
}

/**
Expand Down
22 changes: 4 additions & 18 deletions pgjdbc/src/main/java/org/postgresql/PGProperty.java
View file
Expand Up @@ -293,31 +293,17 @@ public enum PGProperty {
"If disabled hosts are connected in the given order. If enabled hosts are chosen randomly from the set of suitable candidates"),

/**
* <p>File name output of the Logger, if set, the Logger will use a
* {@link java.util.logging.FileHandler} to write to a specified file. If the parameter is not set
* or the file can't be created the {@link java.util.logging.ConsoleHandler} will be used instead.</p>
*
* <p>Parameter should be use together with {@link PGProperty#LOGGER_LEVEL}</p>
* This property is no longer used by the driver and will be ignored.
* Logging is configured via java.util.logging.
*/
LOGGER_FILE(
"loggerFile",
null,
"File name output of the Logger"),

/**
* <p>Logger level of the driver. Allowed values: {@code OFF}, {@code DEBUG} or {@code TRACE}.</p>
*
* <p>This enable the {@link java.util.logging.Logger} of the driver based on the following mapping
* of levels:</p>
* <ul>
* <li>FINE -&gt; DEBUG</li>
* <li>FINEST -&gt; TRACE</li>
* </ul>
*
* <p><b>NOTE:</b> The recommended approach to enable java.util.logging is using a
* {@code logging.properties} configuration file with the property
* {@code -Djava.util.logging.config.file=myfile} or if your are using an application server
* you should use the appropriate logging subsystem.</p>
* This property is no longer used by the driver and will be ignored.
* Logging is configured via java.util.logging.
*/
LOGGER_LEVEL(
"loggerLevel",
Expand Down
20 changes: 12 additions & 8 deletions pgjdbc/src/main/java/org/postgresql/ds/common/BaseDataSource.java
View file
Expand Up @@ -1208,34 +1208,38 @@ public void setEscapeSyntaxCallMode(@Nullable String callMode) {
}

/**
* @return Logger Level of the JDBC Driver
* @see PGProperty#LOGGER_LEVEL
* This property is no longer used by the driver and will be ignored.
* @deprecated Configure via java.util.logging
*/
@Deprecated
public @Nullable String getLoggerLevel() {
return PGProperty.LOGGER_LEVEL.get(properties);
}

/**
* @param loggerLevel of the JDBC Driver
* @see PGProperty#LOGGER_LEVEL
* This property is no longer used by the driver and will be ignored.
* @deprecated Configure via java.util.logging
*/
@Deprecated
public void setLoggerLevel(@Nullable String loggerLevel) {
PGProperty.LOGGER_LEVEL.set(properties, loggerLevel);
}

/**
* @return File output of the Logger.
* @see PGProperty#LOGGER_FILE
* This property is no longer used by the driver and will be ignored.
* @deprecated Configure via java.util.logging
*/
@Deprecated
public @Nullable String getLoggerFile() {
ExpressionProperties exprProps = new ExpressionProperties(properties, System.getProperties());
return PGProperty.LOGGER_FILE.get(exprProps);
}

/**
* @param loggerFile File output of the Logger.
* @see PGProperty#LOGGER_LEVEL
* This property is no longer used by the driver and will be ignored.
* @deprecated Configure via java.util.logging
*/
@Deprecated
public void setLoggerFile(@Nullable String loggerFile) {
PGProperty.LOGGER_FILE.set(properties, loggerFile);
}
Expand Down
67 changes: 0 additions & 67 deletions pgjdbc/src/test/java/org/postgresql/test/jdbc2/DriverTest.java
View file
Expand Up @@ -16,8 +16,6 @@
import org.postgresql.PGEnvironment;
import org.postgresql.PGProperty;
import org.postgresql.test.TestUtil;
import org.postgresql.util.LogWriterHandler;
import org.postgresql.util.NullOutputStream;
import org.postgresql.util.URLCoder;

import org.junit.Test;
Expand All @@ -29,7 +27,6 @@

import java.io.ByteArrayOutputStream;
import java.io.PrintStream;
import java.io.PrintWriter;
import java.lang.reflect.Method;
import java.net.URL;
import java.nio.file.Files;
Expand All @@ -40,8 +37,6 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.Properties;
import java.util.logging.Handler;
import java.util.logging.Logger;

/*
* Tests the dynamically created class org.postgresql.Driver
Expand Down Expand Up @@ -497,68 +492,6 @@ public void testRegistration() throws Exception {
fail("Driver has not been found in DriverManager's list but it should be registered");
}

@Test
public void testSetLogWriter() throws Exception {

// this is a dummy to make sure TestUtil is initialized
Connection con = DriverManager.getConnection(TestUtil.getURL(), TestUtil.getUser(), TestUtil.getPassword());
con.close();
String loggerLevel = System.getProperty("loggerLevel");
String loggerFile = System.getProperty("loggerFile");

PrintWriter prevLog = DriverManager.getLogWriter();
try {
PrintWriter printWriter = new PrintWriter(new NullOutputStream(System.err));
DriverManager.setLogWriter(printWriter);
assertEquals(DriverManager.getLogWriter(), printWriter);
System.clearProperty("loggerFile");
System.clearProperty("loggerLevel");
Properties props = new Properties();
props.setProperty("user", TestUtil.getUser());
props.setProperty("password", TestUtil.getPassword());
props.setProperty("loggerLevel", "DEBUG");
con = DriverManager.getConnection(TestUtil.getURL(), props);

Logger logger = Logger.getLogger("org.postgresql");
Handler[] handlers = logger.getHandlers();
assertTrue(handlers[0] instanceof LogWriterHandler );
con.close();
} finally {
DriverManager.setLogWriter(prevLog);
setProperty("loggerLevel", loggerLevel);
setProperty("loggerFile", loggerFile);
}
}

@Test
public void testSetLogStream() throws Exception {
// this is a dummy to make sure TestUtil is initialized
Connection con = DriverManager.getConnection(TestUtil.getURL(), TestUtil.getUser(), TestUtil.getPassword());
con.close();
String loggerLevel = System.getProperty("loggerLevel");
String loggerFile = System.getProperty("loggerFile");

try {
DriverManager.setLogStream(new NullOutputStream(System.err));
System.clearProperty("loggerFile");
System.clearProperty("loggerLevel");
Properties props = new Properties();
props.setProperty("user", TestUtil.getUser());
props.setProperty("password", TestUtil.getPassword());
props.setProperty("loggerLevel", "DEBUG");
con = DriverManager.getConnection(TestUtil.getURL(), props);

Logger logger = Logger.getLogger("org.postgresql");
Handler []handlers = logger.getHandlers();
assertTrue( handlers[0] instanceof LogWriterHandler );
con.close();
} finally {
DriverManager.setLogStream(null);
setProperty("loggerLevel", loggerLevel);
setProperty("loggerFile", loggerFile);
}
}

@Test
public void testSystemErrIsNotClosedWhenCreatedMultipleConnections() throws Exception {
TestUtil.initDriver();
Expand Down
10 changes: 0 additions & 10 deletions pgjdbc/src/test/java/org/postgresql/test/jdbc2/PGPropertyTest.java
View file
Expand Up @@ -8,7 +8,6 @@
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertSame;
import static org.junit.Assert.assertTrue;

Expand Down Expand Up @@ -215,15 +214,6 @@ public void testPresenceCheck() {
assertFalse(PGProperty.READ_ONLY.isPresent(empty));
}

@Test
public void testNullValue() {
Properties empty = new Properties();
assertNull(PGProperty.LOGGER_LEVEL.getSetString(empty));
Properties withLogging = new Properties();
withLogging.setProperty(PGProperty.LOGGER_LEVEL.getName(), "OFF");
assertNotNull(PGProperty.LOGGER_LEVEL.getSetString(withLogging));
}

@Test
public void testEncodedUrlValues() {
String databaseName = "d&a%ta+base";
Expand Down
1 change: 0 additions & 1 deletion pgjdbc/src/test/java/org/postgresql/test/jdbc4/Jdbc4TestSuite.java
View file
Expand Up @@ -24,7 +24,6 @@
DatabaseMetaDataTest.class,
IsValidTest.class,
JsonbTest.class,
LogTest.class,
PGCopyInputStreamTest.class,
UUIDTest.class,
WrapperTest.class,
Expand Down

0 comments on commit f6d4703

Please sign in to comment.