As the the Delegate class takes care of the Cloneable SPI implementation, it should be sufficient to use "md instanceof Cloneable" only. It is not a expected behavior that a provider implements Cloneable but does not really support it.

Xuelei

On 6/5/2020 10:54 PM, Weijun Wang wrote:
Is it possible to try "md instanceof Cloneable || md.clone() returns"? 
Hopefully this is fast enough in most cases and also has the chance to recognize more 
actually-cloneable ones.

I'm also OK with simply using "md instanceof Cloneable".

--Max

On Jun 6, 2020, at 12:02 PM, Valerie Peng <valerie.p...@oracle.com> wrote:


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.p...@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

Reply via email to