[15] RFR JDK-8246077: Cloneable test in HmacCore seems questionable

[Valerie Peng]

I am merely following the spec's recommendation of trying the clone() 
for cloneability check.

If you both are ok with it and prefer the instanceof check, I can sure 
reverting back the changes in HmacCore and HandshakeHash classes.

Valerie

On 6/5/2020 2:04 AM, Seán Coffey wrote:
> I share the same concern. clone() is a heavy weight operation in 
> constructors that can be called alot during intensive crypto operations.
>
> Now that you've done work on Delegate class - why not also keep the 
> (instanceof Cloneable) test ? That can serve as your fastpath for the 
> default JDK configuration AFAIK.
>
> regards,
> Sean.
>
> On 05/06/2020 00:16, Weijun Wang wrote:
>>
>>> 在 2020年6月5日，03:19，Valerie Peng <valerie.peng at oracle.com> 写道：
>>>
>>>> Can you give an example when these 2 rules have different results? 
>>>> Is this only true for those implementation that directly subclass 
>>>> MessageDigest?
>>> Before this fix, even the Spi impl implements Cloneable the 
>>> instanceof check will always fail because the wrapper class, i.e. 
>>> MessageDigest.Delegate does not. However, if you call the clone() 
>>> (made public by the MessageDigest class), it will succeed because 
>>> Delegate.clone() checks to see if the spi object implements the 
>>> Cloneable interface, if yes, it will proceed to call the spi 
>>> clone(). So, for this scenario, the results are different, e.g. 
>>> instanceof returns false, but clone() succeeds. This is just one 
>>> example. Is this what you are asking?
>> No.
>>
>> I understand this case, but this has already been fixed. Is there any 
>> other example? Or are you only follow the words in the spec? i.e. try 
>> clone() to see if it’s cloneable.
>>
>> I am worried that try clone() is much heavier than just check 
>> instanof Cloneable.
>>
>> Thanks,
>> Max