7194075: Various classes of sunec.jar are duplicated in rt.jar
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