You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by gg...@apache.org on 2018/07/24 22:34:47 UTC

commons-dbcp git commit: [DBCP-517] Make defensive copies of char[] passwords.

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 <gg...@apache.org>
Authored: Tue Jul 24 16:34:43 2018 -0600
Committer: Gary Gregory <gg...@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
      */


Re: commons-dbcp git commit: [DBCP-517] Make defensive copies of char[] passwords.

Posted by Gary Gregory <ga...@gmail.com>.
On Thu, Jul 26, 2018 at 1:31 PM Oliver Heger <ol...@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 <ol...@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 ggregory@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 <gg...@apache.org>
> >>> Authored: Tue Jul 24 16:34:43 2018 -0600
> >>> Committer: Gary Gregory <gg...@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-unsubscribe@commons.apache.org
> >> For additional commands, e-mail: dev-help@commons.apache.org
> >>
> >>
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>

Re: commons-dbcp git commit: [DBCP-517] Make defensive copies of char[] passwords.

Posted by Oliver Heger <ol...@oliver-heger.de>.

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.

Oliver

> 
> Gary
> 
> On Wed, Jul 25, 2018, 14:00 Oliver Heger <ol...@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 ggregory@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 <gg...@apache.org>
>>> Authored: Tue Jul 24 16:34:43 2018 -0600
>>> Committer: Gary Gregory <gg...@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-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>>
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Re: commons-dbcp git commit: [DBCP-517] Make defensive copies of char[] passwords.

Posted by Gary Gregory <ga...@gmail.com>.
I'd we do not already have that, the name does not need "defensive".
Commons Collections would be where to put collections related code.

Gary

On Wed, Jul 25, 2018, 14:00 Oliver Heger <ol...@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 ggregory@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 <gg...@apache.org>
> > Authored: Tue Jul 24 16:34:43 2018 -0600
> > Committer: Gary Gregory <gg...@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-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>

Re: commons-dbcp git commit: [DBCP-517] Make defensive copies of char[] passwords.

Posted by Oliver Heger <ol...@oliver-heger.de>.
+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 ggregory@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 <gg...@apache.org>
> Authored: Tue Jul 24 16:34:43 2018 -0600
> Committer: Gary Gregory <gg...@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-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org