This is an automated email from the ASF dual-hosted git repository.

ggregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-dbcp.git


The following commit(s) were added to refs/heads/master by this push:
     new a4c5af0  Do not display credentials in toString() methods.
a4c5af0 is described below

commit a4c5af0da1de3a7f50c72fc7edaa1f653ca276dd
Author: Gary Gregory <gardgreg...@gmail.com>
AuthorDate: Mon Sep 21 12:10:35 2020 -0400

    Do not display credentials in toString() methods.
---
 src/changes/changes.xml                            |  16 ++-
 .../apache/commons/dbcp2/DelegatingConnection.java |   2 -
 .../commons/dbcp2/DriverConnectionFactory.java     |  13 +--
 src/main/java/org/apache/commons/dbcp2/Utils.java  |  50 ++++++----
 .../dbcp2/cpdsadapter/DriverAdapterCPDS.java       | 110 +++++++--------------
 .../apache/commons/dbcp2/datasources/PoolKey.java  |   2 +-
 .../commons/dbcp2/datasources/UserPassKey.java     |   8 --
 7 files changed, 86 insertions(+), 115 deletions(-)

diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index ff36926..4757547 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -81,13 +81,25 @@ The <action> type attribute can be add,update,fix,remove.
       <action dev="markt" type="fix" due-to="Sebastian Haas">
         Fix regression introduced by unreleased code clean-up #63.
       </action>
-      <!-- update -->
       <action dev="ggregory" type="update" due-to="DoiMasayuki, Alexander 
Norz, Gary Gregory">
         Update to PR#36 - PrepareStatement and prepareCall methods are 
extracted #37.
       </action>
       <action dev="ggregory" type="update" due-to="Gary Gregory">
-        Mask out user name and password from DriverAdapterCPDS.toString().
+        Do not display credentials in DriverAdapterCPDS.toString().
+      </action>
+      <action dev="ggregory" type="update" due-to="Gary Gregory">
+        Do not display credentials in DelegatingConnection.toString().
+      </action>
+      <action dev="ggregory" type="update" due-to="Gary Gregory">
+        Do not display credentials in DriverConnectionFactory.toString().
+      </action>
+      <action dev="ggregory" type="update" due-to="Gary Gregory">
+        Do not display credentials in PoolKey.toString().
+      </action>
+      <action dev="ggregory" type="update" due-to="Gary Gregory">
+        Do not display credentials in UserPassKey.toString().
       </action>
+      <!-- UPDATES -->
       <action dev="ggregory" type="update" issue="DBCP-650" due-to="Gary 
Gregory, Dependabot">
         Update Apache Commons Pool from 2.7.0 to 2.8.1, #48.
       </action>
diff --git a/src/main/java/org/apache/commons/dbcp2/DelegatingConnection.java 
b/src/main/java/org/apache/commons/dbcp2/DelegatingConnection.java
index 96f65b9..6e3c3e0 100644
--- a/src/main/java/org/apache/commons/dbcp2/DelegatingConnection.java
+++ b/src/main/java/org/apache/commons/dbcp2/DelegatingConnection.java
@@ -105,8 +105,6 @@ public class DelegatingConnection<C extends Connection> 
extends AbandonedTrace i
                     if (meta != null) {
                         sb.append(", URL=");
                         sb.append(meta.getURL());
-                        sb.append(", UserName=");
-                        sb.append(meta.getUserName());
                         sb.append(", ");
                         sb.append(meta.getDriverName());
                         str = sb.toString();
diff --git 
a/src/main/java/org/apache/commons/dbcp2/DriverConnectionFactory.java 
b/src/main/java/org/apache/commons/dbcp2/DriverConnectionFactory.java
index 0434584..99b0ddf 100644
--- a/src/main/java/org/apache/commons/dbcp2/DriverConnectionFactory.java
+++ b/src/main/java/org/apache/commons/dbcp2/DriverConnectionFactory.java
@@ -37,12 +37,9 @@ public class DriverConnectionFactory implements 
ConnectionFactory {
     /**
      * Constructs a connection factory for a given Driver.
      *
-     * @param driver
-     *            The Driver.
-     * @param connectString
-     *            The connection string.
-     * @param properties
-     *            The connection properties.
+     * @param driver The Driver.
+     * @param connectString The connection string.
+     * @param properties The connection properties.
      */
     public DriverConnectionFactory(final Driver driver, final String 
connectString, final Properties properties) {
         this.driver = driver;
@@ -81,7 +78,7 @@ public class DriverConnectionFactory implements 
ConnectionFactory {
 
     @Override
     public String toString() {
-        return this.getClass().getName() + " [" + String.valueOf(driver) + ";" 
+ String.valueOf(connectionString) + ";"
-                + String.valueOf(properties) + "]";
+        return this.getClass().getName() + " [" + driver + ";" + 
connectionString + ";"
+            + Utils.cloneWithoutCredentials(properties) + "]";
     }
 }
diff --git a/src/main/java/org/apache/commons/dbcp2/Utils.java 
b/src/main/java/org/apache/commons/dbcp2/Utils.java
index 38cbfe6..61c4ded 100644
--- a/src/main/java/org/apache/commons/dbcp2/Utils.java
+++ b/src/main/java/org/apache/commons/dbcp2/Utils.java
@@ -23,6 +23,7 @@ import java.sql.ResultSet;
 import java.sql.Statement;
 import java.text.MessageFormat;
 import java.util.HashSet;
+import java.util.Properties;
 import java.util.ResourceBundle;
 import java.util.Set;
 
@@ -34,7 +35,7 @@ import java.util.Set;
 public final class Utils {
 
     private static final ResourceBundle messages = ResourceBundle
-            .getBundle(Utils.class.getPackage().getName() + ".LocalStrings");
+        .getBundle(Utils.class.getPackage().getName() + ".LocalStrings");
 
     /**
      * Whether the security manager is enabled.
@@ -70,8 +71,7 @@ public final class Utils {
     /**
      * Clones the given char[] if not null.
      *
-     * @param value
-     *            may be null.
+     * @param value may be null.
      * @return a cloned char[] or null.
      */
     public static char[] clone(final char[] value) {
@@ -79,10 +79,26 @@ public final class Utils {
     }
 
     /**
+     * Clones the given {@link Properties} without the standard "user" or 
"password" entries.
+     * 
+     * @param properties may be null
+     * @return a clone of the input without the standard "user" or "password" 
entries.
+     * @since 2.8.0
+     */
+    public static Properties cloneWithoutCredentials(final Properties 
properties) {
+        if (properties != null) {
+            Properties temp = (Properties) properties.clone();
+            temp.remove("user");
+            temp.remove("password");
+            return temp;
+        }
+        return properties;
+    }
+
+    /**
      * Closes the AutoCloseable (which may be null).
      *
-     * @param autoCloseable
-     *            an AutoCloseable, may be {@code null}
+     * @param autoCloseable an AutoCloseable, may be {@code null}
      * @since 2.6.0
      */
     public static void closeQuietly(final AutoCloseable autoCloseable) {
@@ -98,8 +114,7 @@ public final class Utils {
     /**
      * Closes the Connection (which may be null).
      *
-     * @param connection
-     *            a Connection, may be {@code null}
+     * @param connection a Connection, may be {@code null}
      * @deprecated Use {@link #closeQuietly(AutoCloseable)}.
      */
     @Deprecated
@@ -116,8 +131,7 @@ public final class Utils {
     /**
      * Closes the ResultSet (which may be null).
      *
-     * @param resultSet
-     *            a ResultSet, may be {@code null}
+     * @param resultSet a ResultSet, may be {@code null}
      * @deprecated Use {@link #closeQuietly(AutoCloseable)}.
      */
     @Deprecated
@@ -134,8 +148,7 @@ public final class Utils {
     /**
      * Closes the Statement (which may be null).
      *
-     * @param statement
-     *            a Statement, may be {@code null}.
+     * @param statement a Statement, may be {@code null}.
      * @deprecated Use {@link #closeQuietly(AutoCloseable)}.
      */
     @Deprecated
@@ -152,8 +165,7 @@ public final class Utils {
     /**
      * Gets the correct i18n message for the given key.
      *
-     * @param key
-     *            The key to look up an i18n message.
+     * @param key The key to look up an i18n message.
      * @return The i18n message.
      */
     public static String getMessage(final String key) {
@@ -163,10 +175,8 @@ public final class Utils {
     /**
      * Gets the correct i18n message for the given key with placeholders 
replaced by the supplied arguments.
      *
-     * @param key
-     *            A message key.
-     * @param args
-     *            The message arguments.
+     * @param key A message key.
+     * @param args The message arguments.
      * @return An i18n message.
      */
     public static String getMessage(final String key, final Object... args) {
@@ -181,8 +191,7 @@ public final class Utils {
     /**
      * Converts the given String to a char[].
      *
-     * @param value
-     *            may be null.
+     * @param value may be null.
      * @return a char[] or null.
      */
     public static char[] toCharArray(final String value) {
@@ -192,8 +201,7 @@ public final class Utils {
     /**
      * Converts the given char[] to a String.
      *
-     * @param value
-     *            may be null.
+     * @param value may be null.
      * @return a String or null.
      */
     public static String toString(final char[] value) {
diff --git 
a/src/main/java/org/apache/commons/dbcp2/cpdsadapter/DriverAdapterCPDS.java 
b/src/main/java/org/apache/commons/dbcp2/cpdsadapter/DriverAdapterCPDS.java
index fb98161..da10b3c 100644
--- a/src/main/java/org/apache/commons/dbcp2/cpdsadapter/DriverAdapterCPDS.java
+++ b/src/main/java/org/apache/commons/dbcp2/cpdsadapter/DriverAdapterCPDS.java
@@ -85,7 +85,7 @@ public class DriverAdapterCPDS implements 
ConnectionPoolDataSource, Referenceabl
     private static final long serialVersionUID = -4820523787212147844L;
 
     private static final String GET_CONNECTION_CALLED = "A PooledConnection 
was already requested from this source, "
-            + "further initialization is not allowed.";
+        + "further initialization is not allowed.";
 
     static {
         // Attempt to prevent deadlocks - see DBCP - 272
@@ -248,7 +248,7 @@ public class DriverAdapterCPDS implements 
ConnectionPoolDataSource, Referenceabl
      */
     @Override
     public Object getObjectInstance(final Object refObj, final Name name, 
final Context context,
-            final Hashtable<?, ?> env) throws Exception {
+        final Hashtable<?, ?> env) throws Exception {
         // The spec says to return null if we can't create an instance
         // of the reference
         DriverAdapterCPDS cpds = null;
@@ -351,14 +351,12 @@ public class DriverAdapterCPDS implements 
ConnectionPoolDataSource, Referenceabl
     /**
      * Attempts to establish a database connection.
      *
-     * @param pooledUserName
-     *            name to be used for the connection
-     * @param pooledUserPassword
-     *            password to be used fur the connection
+     * @param pooledUserName name to be used for the connection
+     * @param pooledUserPassword password to be used fur the connection
      */
     @Override
     public PooledConnection getPooledConnection(final String pooledUserName, 
final String pooledUserPassword)
-            throws SQLException {
+        throws SQLException {
         getConnectionCalled = true;
         PooledConnectionImpl pooledConnection = null;
         // Workaround for buggy WebLogic 5.1 classloader - ignore the 
exception upon first invocation.
@@ -367,19 +365,19 @@ public class DriverAdapterCPDS implements 
ConnectionPoolDataSource, Referenceabl
                 update(connectionProperties, KEY_USER, pooledUserName);
                 update(connectionProperties, KEY_PASSWORD, pooledUserPassword);
                 pooledConnection = new PooledConnectionImpl(
-                        DriverManager.getConnection(getUrl(), 
connectionProperties));
+                    DriverManager.getConnection(getUrl(), 
connectionProperties));
             } else {
                 pooledConnection = new PooledConnectionImpl(
-                        DriverManager.getConnection(getUrl(), pooledUserName, 
pooledUserPassword));
+                    DriverManager.getConnection(getUrl(), pooledUserName, 
pooledUserPassword));
             }
             
pooledConnection.setAccessToUnderlyingConnectionAllowed(isAccessToUnderlyingConnectionAllowed());
         } catch (final ClassCircularityError e) {
             if (connectionProperties != null) {
                 pooledConnection = new PooledConnectionImpl(
-                        DriverManager.getConnection(getUrl(), 
connectionProperties));
+                    DriverManager.getConnection(getUrl(), 
connectionProperties));
             } else {
                 pooledConnection = new PooledConnectionImpl(
-                        DriverManager.getConnection(getUrl(), pooledUserName, 
pooledUserPassword));
+                    DriverManager.getConnection(getUrl(), pooledUserName, 
pooledUserPassword));
             }
             
pooledConnection.setAccessToUnderlyingConnectionAllowed(isAccessToUnderlyingConnectionAllowed());
         }
@@ -497,8 +495,7 @@ public class DriverAdapterCPDS implements 
ConnectionPoolDataSource, Referenceabl
      * Sets the value of the accessToUnderlyingConnectionAllowed property. It 
controls if the PoolGuard allows access to
      * the underlying connection. (Default: false)
      *
-     * @param allow
-     *            Access to the underlying connection is granted when true.
+     * @param allow Access to the underlying connection is granted when true.
      */
     public synchronized void setAccessToUnderlyingConnectionAllowed(final 
boolean allow) {
         this.accessToUnderlyingConnectionAllowed = allow;
@@ -515,10 +512,8 @@ public class DriverAdapterCPDS implements 
ConnectionPoolDataSource, Referenceabl
      * null.
      * </p>
      *
-     * @param props
-     *            Connection properties to use when creating new connections.
-     * @throws IllegalStateException
-     *             if {@link #getPooledConnection()} has been called
+     * @param props Connection properties to use when creating new connections.
+     * @throws IllegalStateException if {@link #getPooledConnection()} has 
been called
      */
     public void setConnectionProperties(final Properties props) {
         assertInitializationAllowed();
@@ -537,8 +532,7 @@ public class DriverAdapterCPDS implements 
ConnectionPoolDataSource, Referenceabl
      * Sets the value of description. This property is here for use by the 
code which will deploy this datasource. It is
      * not used internally.
      *
-     * @param v
-     *            Value to assign to description.
+     * @param v Value to assign to description.
      */
     public void setDescription(final String v) {
         this.description = v;
@@ -548,12 +542,9 @@ public class DriverAdapterCPDS implements 
ConnectionPoolDataSource, Referenceabl
      * Sets the driver class name. Setting the driver class name cause the 
driver to be registered with the
      * DriverManager.
      *
-     * @param v
-     *            Value to assign to driver.
-     * @throws IllegalStateException
-     *             if {@link #getPooledConnection()} has been called
-     * @throws ClassNotFoundException
-     *             if the class cannot be located
+     * @param v Value to assign to driver.
+     * @throws IllegalStateException if {@link #getPooledConnection()} has 
been called
+     * @throws ClassNotFoundException if the class cannot be located
      */
     public void setDriver(final String v) throws ClassNotFoundException {
         assertInitializationAllowed();
@@ -583,10 +574,8 @@ public class DriverAdapterCPDS implements 
ConnectionPoolDataSource, Referenceabl
      * Gets the maximum number of statements that can remain idle in the pool, 
without extra ones being released, or
      * negative for no limit.
      *
-     * @param maxIdle
-     *            The maximum number of statements that can remain idle
-     * @throws IllegalStateException
-     *             if {@link #getPooledConnection()} has been called
+     * @param maxIdle The maximum number of statements that can remain idle
+     * @throws IllegalStateException if {@link #getPooledConnection()} has 
been called
      */
     public void setMaxIdle(final int maxIdle) {
         assertInitializationAllowed();
@@ -596,8 +585,7 @@ public class DriverAdapterCPDS implements 
ConnectionPoolDataSource, Referenceabl
     /**
      * Sets the maximum number of prepared statements.
      *
-     * @param maxPreparedStatements
-     *            the new maximum number of prepared statements
+     * @param maxPreparedStatements the new maximum number of prepared 
statements
      */
     public void setMaxPreparedStatements(final int maxPreparedStatements) {
         this.maxPreparedStatements = maxPreparedStatements;
@@ -607,12 +595,10 @@ public class DriverAdapterCPDS implements 
ConnectionPoolDataSource, Referenceabl
      * Sets the minimum amount of time a statement may sit idle in the pool 
before it is eligible for eviction by the
      * idle object evictor (if any). When non-positive, no objects will be 
evicted from the pool due to idle time alone.
      *
-     * @param minEvictableIdleTimeMillis
-     *            minimum time to set (in ms)
+     * @param minEvictableIdleTimeMillis minimum time to set (in ms)
      * @see #getMinEvictableIdleTimeMillis()
      * @see #setTimeBetweenEvictionRunsMillis(long)
-     * @throws IllegalStateException
-     *             if {@link #getPooledConnection()} has been called
+     * @throws IllegalStateException if {@link #getPooledConnection()} has 
been called
      */
     public void setMinEvictableIdleTimeMillis(final int 
minEvictableIdleTimeMillis) {
         assertInitializationAllowed();
@@ -640,10 +626,8 @@ public class DriverAdapterCPDS implements 
ConnectionPoolDataSource, Referenceabl
     /**
      * Sets the value of password for the default user.
      *
-     * @param userPassword
-     *            Value to assign to password.
-     * @throws IllegalStateException
-     *             if {@link #getPooledConnection()} has been called
+     * @param userPassword Value to assign to password.
+     * @throws IllegalStateException if {@link #getPooledConnection()} has 
been called
      */
     public void setPassword(final char[] userPassword) {
         assertInitializationAllowed();
@@ -654,10 +638,8 @@ public class DriverAdapterCPDS implements 
ConnectionPoolDataSource, Referenceabl
     /**
      * Sets the value of password for the default user.
      *
-     * @param userPassword
-     *            Value to assign to password.
-     * @throws IllegalStateException
-     *             if {@link #getPooledConnection()} has been called
+     * @param userPassword Value to assign to password.
+     * @throws IllegalStateException if {@link #getPooledConnection()} has 
been called
      */
     public void setPassword(final String userPassword) {
         assertInitializationAllowed();
@@ -668,10 +650,8 @@ public class DriverAdapterCPDS implements 
ConnectionPoolDataSource, Referenceabl
     /**
      * Whether to toggle the pooling of <code>PreparedStatement</code>s
      *
-     * @param poolPreparedStatements
-     *            true to pool statements.
-     * @throws IllegalStateException
-     *             if {@link #getPooledConnection()} has been called
+     * @param poolPreparedStatements true to pool statements.
+     * @throws IllegalStateException if {@link #getPooledConnection()} has 
been called
      */
     public void setPoolPreparedStatements(final boolean 
poolPreparedStatements) {
         assertInitializationAllowed();
@@ -682,12 +662,10 @@ public class DriverAdapterCPDS implements 
ConnectionPoolDataSource, Referenceabl
      * Sets the number of milliseconds to sleep between runs of the idle 
object evictor thread. When non-positive, no
      * idle object evictor thread will be run.
      *
-     * @param timeBetweenEvictionRunsMillis
-     *            The number of milliseconds to sleep between runs of the idle 
object evictor thread. When non-positive,
-     *            no idle object evictor thread will be run.
+     * @param timeBetweenEvictionRunsMillis The number of milliseconds to 
sleep between runs of the idle object evictor
+     *        thread. When non-positive, no idle object evictor thread will be 
run.
      * @see #getTimeBetweenEvictionRunsMillis()
-     * @throws IllegalStateException
-     *             if {@link #getPooledConnection()} has been called
+     * @throws IllegalStateException if {@link #getPooledConnection()} has 
been called
      */
     public void setTimeBetweenEvictionRunsMillis(final long 
timeBetweenEvictionRunsMillis) {
         assertInitializationAllowed();
@@ -697,10 +675,8 @@ public class DriverAdapterCPDS implements 
ConnectionPoolDataSource, Referenceabl
     /**
      * Sets the value of URL string used to locate the database for this 
datasource.
      *
-     * @param v
-     *            Value to assign to url.
-     * @throws IllegalStateException
-     *             if {@link #getPooledConnection()} has been called
+     * @param v Value to assign to url.
+     * @throws IllegalStateException if {@link #getPooledConnection()} has 
been called
      */
     public void setUrl(final String v) {
         assertInitializationAllowed();
@@ -710,10 +686,8 @@ public class DriverAdapterCPDS implements 
ConnectionPoolDataSource, Referenceabl
     /**
      * Sets the value of default user (login or user name).
      *
-     * @param v
-     *            Value to assign to user.
-     * @throws IllegalStateException
-     *             if {@link #getPooledConnection()} has been called
+     * @param v Value to assign to user.
+     * @throws IllegalStateException if {@link #getPooledConnection()} has 
been called
      */
     public void setUser(final String v) {
         assertInitializationAllowed();
@@ -732,7 +706,8 @@ public class DriverAdapterCPDS implements 
ConnectionPoolDataSource, Referenceabl
         builder.append("[description=");
         builder.append(description);
         builder.append(", url=");
-        // TODO What if the connection string contains a 'user' or 'password' 
query parameter but that connection string is not in a legal URL format?
+        // TODO What if the connection string contains a 'user' or 'password' 
query parameter but that connection string
+        // is not in a legal URL format?
         builder.append(url);
         builder.append(", driver=");
         builder.append(driver);
@@ -753,24 +728,13 @@ public class DriverAdapterCPDS implements 
ConnectionPoolDataSource, Referenceabl
         builder.append(", getConnectionCalled=");
         builder.append(getConnectionCalled);
         builder.append(", connectionProperties=");
-        Properties tmpProps = connectionProperties;
-        tmpProps = mask(tmpProps, "user");
-        tmpProps = mask(tmpProps, "password");
-        builder.append(tmpProps);
+        builder.append(Utils.cloneWithoutCredentials(connectionProperties));
         builder.append(", accessToUnderlyingConnectionAllowed=");
         builder.append(accessToUnderlyingConnectionAllowed);
         builder.append("]");
         return builder.toString();
     }
 
-    private Properties mask(Properties properties, final String 
maskValueAtKey) {
-        if (connectionProperties != null && 
connectionProperties.contains(maskValueAtKey)) {
-            properties = (Properties) connectionProperties.clone();
-            properties.put(maskValueAtKey, "[" + maskValueAtKey + "]");
-        }
-        return properties;
-    }
-
     private void update(final Properties properties, final String key, final 
String value) {
         if (properties != null && key != null) {
             if (value == null) {
diff --git a/src/main/java/org/apache/commons/dbcp2/datasources/PoolKey.java 
b/src/main/java/org/apache/commons/dbcp2/datasources/PoolKey.java
index 0aa70ea..2ace389 100644
--- a/src/main/java/org/apache/commons/dbcp2/datasources/PoolKey.java
+++ b/src/main/java/org/apache/commons/dbcp2/datasources/PoolKey.java
@@ -68,7 +68,7 @@ class PoolKey implements Serializable {
     public String toString() {
         final StringBuffer sb = new StringBuffer(50);
         sb.append("PoolKey(");
-        sb.append(userName).append(", ").append(dataSourceName);
+        sb.append("UserName").append(", ").append(dataSourceName);
         sb.append(')');
         return sb.toString();
     }
diff --git 
a/src/main/java/org/apache/commons/dbcp2/datasources/UserPassKey.java 
b/src/main/java/org/apache/commons/dbcp2/datasources/UserPassKey.java
index 789413c..9584db5 100644
--- a/src/main/java/org/apache/commons/dbcp2/datasources/UserPassKey.java
+++ b/src/main/java/org/apache/commons/dbcp2/datasources/UserPassKey.java
@@ -120,12 +120,4 @@ class UserPassKey implements Serializable {
         return result;
     }
 
-    @Override
-    public String toString() {
-        final StringBuffer sb = new StringBuffer(super.toString());
-        sb.append("[");
-        sb.append(userName);
-        sb.append(']');
-        return sb.toString();
-    }
 }

Reply via email to