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

Stephen Flores stephen.flores at oracle.com
Thu Dec 20 00:23:28 UTC 2012


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?
>

This commented out code was in ECParameters that was not need to 
implement the public contract, but to be utility code like encodePoint 
and decodePoint, so I moved it *as is* to ECUtil.

I would like to remove it, as it the policy of other groups, but I don't 
know your groups policy.

Which would you like: remove the code or add the comment "Code that will 
have a possible use in the future"?

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