Hi Steve,
Most of this looks good. Here are my comments:
* General
You haven't added any new regression tests for this. Since this is
essentially a lot of code refactoring and covered by existing tests, can
you add the noreg-cleanup keyword to the bug?
* src/share/classes/java/security/AlgorithmParameters.java
[394] You can't make this change, since it would violate the spec which
says it returns null. You will need to workaround this some other way.
* src/share/classes/sun/security/ec/ECDSASignature.java
[278, 305] The comment should be aligned to the left.
* src/share/classes/sun/security/pkcs11/P11ECKeyFactory.java
[121-2] The exception message here seems misleading, since it doesn't
have to be an instance of ECPublicKey, it could just be a PublicKey as
long as the encoding is correct. Why not just set the cause to the
InvalidKeySpecException, ex:
What exception message did the old code throw? It might be best to
preserve that behavior.
[151-2] same comment as above
* src/share/classes/sun/security/util/ECUtil.java
[230-60] Can you add a comment as to why this is commented out?
--Sean
On 11/26/2012 10:21 PM, Stephen Flores wrote:
Vincent, Sean,
Please review the fix for:
CR 7194075: Various classes of sunec.jar are duplicated in rt.jar
http://cr.openjdk.java.net/~sflores/7194075/webrev-1/
Changes:
*Changed/renamed any of methods that did not support the public API to
package private.
*Moved the decode and encode point methods out of ECParameters to a new
class sun.security.util.ECUtil.
*Changed any "new byte[], System.arraycopy" blocks in ECUtil point
methods to Arrays.copyOfRange.
*Added a new AlgorithmParameterSpec in sun.security.util to get curves
by key size, for PKCS11 to use.
*Moved all of static lookup methods in ECParameters, NamedCurve and the
curve repository to separate class (CurveDB). This made ECParameters and
NamedCurve cleaner and easier work on (there was some ECParameters cleanup.
*In JSSE and PKCS11 and changed the references to ECParmeters and
NamedCurve to the ECUtil which has utility methods that use the public
APIs.
*Changed to the EC unit test to use the list of supported curves in the
property that the SunEC provider has already.
*Changed SunECEntries to build the list of supported curves property
from the collection in CurveDB.
*Changed the JDK makefiles to not duplicate EC classes in rt.jar.
Thanks,
Steve.
