+1, Thank you. I had planned to create a patch for this issue, but did not find the time so far.
BTW, I have quite often the need to create defensive copies of arrays or collections in some variants (e.g. null safe, with wrapping to an unmodifiable collection, etc.). Could this be an addition to [lang]? A new utility class like DefensiveCopyUtils? Oliver Am 25.07.2018 um 00:34 schrieb ggreg...@apache.org: > Repository: commons-dbcp > Updated Branches: > refs/heads/master 70822f11d -> d7969ac93 > > > [DBCP-517] Make defensive copies of char[] passwords. > > Project: http://git-wip-us.apache.org/repos/asf/commons-dbcp/repo > Commit: http://git-wip-us.apache.org/repos/asf/commons-dbcp/commit/d7969ac9 > Tree: http://git-wip-us.apache.org/repos/asf/commons-dbcp/tree/d7969ac9 > Diff: http://git-wip-us.apache.org/repos/asf/commons-dbcp/diff/d7969ac9 > > Branch: refs/heads/master > Commit: d7969ac934e752e7a7b258fa5a5af9a563c40a13 > Parents: 70822f1 > Author: Gary Gregory <ggreg...@apache.org> > Authored: Tue Jul 24 16:34:43 2018 -0600 > Committer: Gary Gregory <ggreg...@apache.org> > Committed: Tue Jul 24 16:34:43 2018 -0600 > > ---------------------------------------------------------------------- > src/changes/changes.xml | 5 ++++- > src/main/java/org/apache/commons/dbcp2/Utils.java | 12 ++++++++++++ > .../commons/dbcp2/cpdsadapter/DriverAdapterCPDS.java | 4 ++-- > .../dbcp2/datasources/CPDSConnectionFactory.java | 11 ++++++++++- > .../dbcp2/cpdsadapter/TestDriverAdapterCPDS.java | 9 +++++++++ > .../dbcp2/datasources/TestCPDSConnectionFactory.java | 10 ++++++++++ > 6 files changed, 47 insertions(+), 4 deletions(-) > ---------------------------------------------------------------------- > > > http://git-wip-us.apache.org/repos/asf/commons-dbcp/blob/d7969ac9/src/changes/changes.xml > ---------------------------------------------------------------------- > diff --git a/src/changes/changes.xml b/src/changes/changes.xml > index c924411..8f1de55 100644 > --- a/src/changes/changes.xml > +++ b/src/changes/changes.xml > @@ -61,9 +61,12 @@ The <action> type attribute can be add,update,fix,remove. > > <body> > <release version="2.6.0" date="2018-MM-DD" description="This is a minor > release, including bug fixes and enhancements."> > - <action dev="ggregory" type="add" issue="DBCP-514" due-to="Gary > Gregory"> > + <action dev="ggregory" type="add" issue="DBCP-514" due-to="Tom > Jenkinson, Gary Gregory"> > Allow DBCP to register with a TransactionSynchronizationRegistry for > XA cases. > </action> > + <action dev="ggregory" type="update" issue="DBCP-517" due-to="Gary > Gregory"> > + Make defensive copies of char[] passwords. > + </action> > </release> > <release version="2.5.0" date="2018-07-15" description="This is a minor > release, including bug fixes and enhancements."> > <action dev="ggregory" type="update" issue="DBCP-505" due-to="Gary > Gregory"> > > http://git-wip-us.apache.org/repos/asf/commons-dbcp/blob/d7969ac9/src/main/java/org/apache/commons/dbcp2/Utils.java > ---------------------------------------------------------------------- > diff --git a/src/main/java/org/apache/commons/dbcp2/Utils.java > b/src/main/java/org/apache/commons/dbcp2/Utils.java > index 8e798c4..244b51b 100644 > --- a/src/main/java/org/apache/commons/dbcp2/Utils.java > +++ b/src/main/java/org/apache/commons/dbcp2/Utils.java > @@ -72,6 +72,17 @@ public final class Utils { > } > > /** > + * Clones the given char[] if not null. > + * > + * @param value > + * may be null. > + * @return a cloned char[] or null. > + */ > + public static char[] clone(final char[] value) { > + return value == null ? null : value.clone(); > + } > + > + /** > * Closes the ResultSet (which may be null). > * > * @param resultSet > @@ -169,4 +180,5 @@ public final class Utils { > public static String toString(final char[] value) { > return value == null ? null : String.valueOf(value); > } > + > } > > http://git-wip-us.apache.org/repos/asf/commons-dbcp/blob/d7969ac9/src/main/java/org/apache/commons/dbcp2/cpdsadapter/DriverAdapterCPDS.java > ---------------------------------------------------------------------- > 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 bbc8831..0844c9b 100644 > --- > a/src/main/java/org/apache/commons/dbcp2/cpdsadapter/DriverAdapterCPDS.java > +++ > b/src/main/java/org/apache/commons/dbcp2/cpdsadapter/DriverAdapterCPDS.java > @@ -423,8 +423,8 @@ public class DriverAdapterCPDS implements > ConnectionPoolDataSource, Referenceabl > */ > public void setPassword(final char[] userPassword) { > assertInitializationAllowed(); > - this.userPassword = userPassword; > - update(connectionProperties, KEY_PASSWORD, > Utils.toString(userPassword)); > + this.userPassword = Utils.clone(userPassword); > + update(connectionProperties, KEY_PASSWORD, > Utils.toString(this.userPassword)); > } > > /** > > http://git-wip-us.apache.org/repos/asf/commons-dbcp/blob/d7969ac9/src/main/java/org/apache/commons/dbcp2/datasources/CPDSConnectionFactory.java > ---------------------------------------------------------------------- > diff --git > a/src/main/java/org/apache/commons/dbcp2/datasources/CPDSConnectionFactory.java > > b/src/main/java/org/apache/commons/dbcp2/datasources/CPDSConnectionFactory.java > index f0ae74d..c0ee90b 100644 > --- > a/src/main/java/org/apache/commons/dbcp2/datasources/CPDSConnectionFactory.java > +++ > b/src/main/java/org/apache/commons/dbcp2/datasources/CPDSConnectionFactory.java > @@ -122,6 +122,15 @@ class CPDSConnectionFactory > } > > /** > + * (Testing API) Gets the value of password for the default user. > + * > + * @return value of password. > + */ > + char[] getPasswordCharArray() { > + return userPassword; > + } > + > + /** > * Returns the object pool used to pool connections created by this > factory. > * > * @return ObjectPool managing pooled connections > @@ -335,7 +344,7 @@ class CPDSConnectionFactory > * new password > */ > public synchronized void setPassword(final char[] userPassword) { > - this.userPassword = userPassword; > + this.userPassword = Utils.clone(userPassword); > } > > /** > > http://git-wip-us.apache.org/repos/asf/commons-dbcp/blob/d7969ac9/src/test/java/org/apache/commons/dbcp2/cpdsadapter/TestDriverAdapterCPDS.java > ---------------------------------------------------------------------- > diff --git > a/src/test/java/org/apache/commons/dbcp2/cpdsadapter/TestDriverAdapterCPDS.java > > b/src/test/java/org/apache/commons/dbcp2/cpdsadapter/TestDriverAdapterCPDS.java > index 0669b1f..7bae26e 100644 > --- > a/src/test/java/org/apache/commons/dbcp2/cpdsadapter/TestDriverAdapterCPDS.java > +++ > b/src/test/java/org/apache/commons/dbcp2/cpdsadapter/TestDriverAdapterCPDS.java > @@ -208,6 +208,15 @@ public class TestDriverAdapterCPDS { > } > > @Test > + public void testSetPasswordThenModCharArray() { > + char[] pwd = {'a' }; > + pcds.setPassword(pwd); > + assertEquals("a", pcds.getPassword()); > + pwd[0] = 'b'; > + assertEquals("a", pcds.getPassword()); > + } > + > + @Test > public void testSetPasswordNullWithConnectionProperties() throws > Exception { > pcds.setConnectionProperties(new Properties()); > pcds.setPassword("Secret"); > > http://git-wip-us.apache.org/repos/asf/commons-dbcp/blob/d7969ac9/src/test/java/org/apache/commons/dbcp2/datasources/TestCPDSConnectionFactory.java > ---------------------------------------------------------------------- > diff --git > a/src/test/java/org/apache/commons/dbcp2/datasources/TestCPDSConnectionFactory.java > > b/src/test/java/org/apache/commons/dbcp2/datasources/TestCPDSConnectionFactory.java > index 3f9c157..86c0dfe 100644 > --- > a/src/test/java/org/apache/commons/dbcp2/datasources/TestCPDSConnectionFactory.java > +++ > b/src/test/java/org/apache/commons/dbcp2/datasources/TestCPDSConnectionFactory.java > @@ -143,6 +143,16 @@ public class TestCPDSConnectionFactory { > assertEquals(0, pool.getNumIdle()); > } > > + @Test > + public void testSetPasswordThenModCharArray() { > + final CPDSConnectionFactory factory = new > CPDSConnectionFactory(cpds, null, -1, false, "userName", "password"); > + char[] pwd = {'a' }; > + factory.setPassword(pwd); > + assertEquals("a", String.valueOf(factory.getPasswordCharArray())); > + pwd[0] = 'b'; > + assertEquals("a", String.valueOf(factory.getPasswordCharArray())); > + } > + > /** > * JIRA: DBCP-442 > */ > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
