HmacCore.java: 78 md = null; 79 String noCloneProv = md.getProvider().getName();
NPE on line 79. Should reverse. > On Jun 4, 2020, at 8:09 AM, Valerie Peng <valerie.p...@oracle.com> wrote: > > Hi, > > Anyone can help review this fix? I changed com.sun.crypto.provider.HmacCore > class and sun.security.ssl.HandshakeHash class to check for cloneability > based on the clone() call instead of the Cloneable interface. Can you give an example when these 2 rules have different results? Is this only true for those implementation that directly subclass MessageDigest? The test is not complete. There should be non-cloneable hash. Or even better, a hash which is not cloneable from the preferred provider but cloneable from another. Hopefully you can simply reuse an existing implementation with a different algorithm name and override its clone() to throw CNSE. Thanks, Max > Noticed a bug in sun.security.provider.DigestBase class which misses to reset > the temporary buffer when cloning and fixed it here as well. > > Lastly, I also made changes to java.security.MessageDigest and > java.security.Signature classes and attempt to match the Delegate wrapper > with the underlying spi object, i.e. if the spi implements Cloneable and then > Delegate wrapper also implements Cloneable. This part is mostly for non-JDK > callers which rely on the instanceof Cloneable check for cloneability. > However, for Signature class, if the object is requested using > Signature.getInstance(String), then we can't do matching here since the > underlying spi is not yet determined at this time. The 3 TestCloneable.java > tests are for testing this last part and is NOT for the HmacCore and > HandshakeHash changes. I am on the fence about this part and am open to leave > this out if minimum fix is preferred. > > Bug: https://bugs.openjdk.java.net/browse/JDK-8246077 > Webrev: http://cr.openjdk.java.net/~valeriep/8246077/webrev.00/ > > Thanks, > > Valerie >