Code review request, 7106773: 512 bits RSA key cannot work with SHA384 and SHA512

[Xuelei Fan]

webrev: http://cr.openjdk.java.net/~xuelei/7106773/webrev.03/

On 11/21/2011 4:34 PM, Weijun Wang wrote:
> I removed most of the thread and keep some sections below:
> 
> On 11/21/2011 03:30 PM, Xuelei Fan wrote:
>> new webrev: http://cr.openjdk.java.net/~xuelei/7106773/webrev.02/
> 
> What is synchronized for? Why only on the for block?  This won't prevent
> map iteration and insertion at the same time.
> 
You're right. The SynchronizedMap is not necessary. Changed to use
normal hash map, and synchronizing the map in both iteration and
insertion block.

>>
>>>
>>> Another solution is to define a new interface:
>>>      package sun.security.util;
>>>      public interface Measurable {
>>>          public int getLength();
>>>      }
>>>
>>> And updated P11Key and MSCPI Key accordingly to implement the
>>> Measureable interface:
>>>    package sun.security.pkcs11;
>>>    ...
>>> - abstract class P11Key implements Key, Measurable {
>>> + abstract class P11Key implements Key {
>>>    ...
>>> -     int keyLength() {
>>> +     public int getLength() {
>>>    ...
>>>    }
>>>
>>> It is similar for MSCAPI Key.
> 
> How about this approach? This looks very safe.
> 
I also prefer this approach, although it need more updates in PKCS11 and
MSCPI source code. If you vote for this approach, I will try to
implement it.

Thanks,
Xuelei

>>>
>>>>>>
>>>>>> Calling getDeclaredMethod each time is a waste. How about just
>>>>>>
>>>>>>           while (clazz != null) {
>>>>>>               if (clazz.getName().equals(baseClassName)) {
>>>>>>                   // do sth
>>>>>>                   break;
>>>>>>               }
>>>>>>               clazz = clazz.getSuperclass();
>>>>>>           }
>>>>>>
>>>>>> Also, you're using methods in reflection. Is it faster to use fields?
>>>>>>
>>>>> I also concerns about the override of the target method, although
>>>>> there
>>>>> is no such override at present. From some reports, it seems that
>>>>> fields
>>>>> reflection cost more cycles than method reflection.
> 
> You mentioned "concerns about the override of the target method".
> 
> Thanks
> Max