[Bug crypto/28204] PBEKeySpec incorrectly deletes the originally passed password array

[raif at swiftdsl dot com dot au]

------- Comment #8 from raif at swiftdsl dot com dot au  2006-07-05 21:37 
-------
(In reply to comment #7)
> After looking into the issue more...
> An upated patch can be found here:
> http://developer.classpath.org/pipermail/classpath-patches/attachments/20060705/1f52b6af/Crypto-PBEKeySpec.bin
> ...
> I also have a mauve testlet ... can be found here:
> http://developer.classpath.org/pipermail/classpath-patches/attachments/20060705/1f52b6af/TestOfPBEKeySpec.java
> ...
> Comments?

on the PBEKeySpec:

* i don't think cloning the password and salt at construction is enough.  the
code for getPassword() and getSalt() should still return a clone of the arrays.
 if this is the case, my patch/diff comparator did not show it.  the cloning is
necessary to guard againt either of these entities being changed between
consecutive invocations of those methods.

* minor nit: you probably can reduce code duplication by confining all the
correctness checking code in one method; e.g. checkParams() or the like, which
can be called in each constructor.


on the Test:

* the "Tags" line, near the beginning of the file, for the crypto classes is
usually:

// Tags: GNU-CRYPTO JDK1.4


* because the test's methods are run sequentally, it's difficult to see if and
how previous test methods may have affected the class fields; e.g. salt, etc. 
i suggest you include the field initialisation in each methods.

* usually, we use harness.check(true, "message") instead of comments in the
code to reflect an expected behaviour; i.e. catch NPE in testConstructorPSI(),
etc.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=28204