On Thu, Jul 26, 2018 at 1:31 PM Oliver Heger <oliver.he...@oliver-heger.de> wrote:
> > > Am 25.07.2018 um 23:36 schrieb Gary Gregory: > > I'd we do not already have that, the name does not need "defensive". > > Commons Collections would be where to put collections related code. > > I would rather think that this copy functionality is pretty basic, so I > would put it in [lang]. I would not want to depend on [collections] just > to use one or two copy methods. And the utility class could support > non-collection types (mainly arrays) as well. > Hi Oliver, For a long time we've had ArrayUtils in [lang] CollectionUtils in [collections]. I do not think that it matters whether the code is "basic" or not; the domain of these operations is for Collection instances and [lang] does not concern itself with Collections, Commons Collections does. It so happens that Java makes the distinction between primitive and Objects which is why we have arrays like int[] and Collections like List<Integer>. These new methods belong in these two classes or in new classes but in their respective components, for example CopyArrays in [lang] and CopyCollection in [collections]. The theme for both components is well established so I am -1 to put Collection code in [lang]. Gary > > Oliver > > > > > Gary > > > > On Wed, Jul 25, 2018, 14:00 Oliver Heger <oliver.he...@oliver-heger.de> > > wrote: > > > >> +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 > >> > >> > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > >
