7194075: Various classes of sunec.jar are duplicated in rt.jar (original) (raw)

Stephen Flores stephen.flores at oracle.com
Wed Jan 16 04:46:01 UTC 2013

Sean,

Here are the changes for the final version (as outlined in my comments below):

http://cr.openjdk.java.net/~sflores/7194075/webrev-2/

Steve.

On 12/19/2012 07:23 PM, Stephen Flores wrote:

Sorry, for the delayed response, I have been working in another area.

Comments below: On 11/29/2012 12:49 PM, Sean Mullan wrote: 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? When I figure out the new bug system, will do this. * 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. I will revert the change. * src/share/classes/sun/security/ec/ECDSASignature.java [278, 305] The comment should be aligned to the left. OK * 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: OK, I will change the code to use the InvalidKeySpecException from the key factory to create a InvalidKeyException. Sorry, I keep forgetting about the cause constructor on exceptions, because for 8 years while working on J2ME MIDP I did not have cause constructor on exceptions or the getCause method. What exception message did the old code throw? It might be best to preserve that behavior. [151-2] same comment as above OK * src/share/classes/sun/security/util/ECUtil.java [230-60] Can you add a comment as to why this is commented out? I will remove it. Steve. --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.

More information about the security-dev mailing list