7194075: Various classes of sunec.jar are duplicated in rt.jar

[Sean Mullan]

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.