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(); - } }