On Thu, 2 Jul 2026 20:32:35 GMT, Valerie Peng <[email protected]> wrote:
>> Initial commit containing the public API changes and related regression >> tests. >> >> --------- >> - [x] I confirm that I make this contribution in accordance with the >> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai). > > Valerie Peng has updated the pull request incrementally with one additional > commit since the last revision: > > updated to address review comments. src/java.base/share/classes/com/sun/crypto/provider/Argon2DerivedKey.java line 48: > 46: * @since 27 > 47: */ > 48: public final class Argon2DerivedKey implements SecretKey, Destroyable { Don't need "implements Destroyable" since `SecretKey` already implements it. src/java.base/share/classes/com/sun/crypto/provider/Argon2DerivedKey.java line 61: > 59: > 60: /** > 61: * Create a Argon2 derived secret key using the supplied arguments. s/a/an/ src/java.base/share/classes/com/sun/crypto/provider/Argon2DerivedKey.java line 63: > 61: * Create a Argon2 derived secret key using the supplied arguments. > 62: * > 63: * @param type the Argon2 variant. Nit: no need for periods in `@param` and `@exception` descriptions. src/java.base/share/classes/com/sun/crypto/provider/Argon2DerivedKey.java line 68: > 66: * @param algo the algorithm for the derived key. > 67: * > 68: * @exception InvalidKeyException if less than 8 bytes are available > for `@throws` is more commonly used now than `@exception`. src/java.base/share/classes/com/sun/crypto/provider/Argon2DerivedKey.java line 68: > 66: * @param algo the algorithm for the derived key. > 67: * > 68: * @exception InvalidKeyException if less than 8 bytes are available > for Remove? This ctor does not throw IKE. src/java.base/share/classes/com/sun/crypto/provider/Argon2DerivedKey.java line 71: > 69: * the key. > 70: */ > 71: public Argon2DerivedKey(String type, Argon2ParameterSpec spec, Can be package-private I think. src/java.base/share/classes/com/sun/crypto/provider/Argon2DerivedKey.java line 80: > 78: } > 79: > 80: public byte[] getEncoded() { Add `@Override`. src/java.base/share/classes/com/sun/crypto/provider/Argon2DerivedKey.java line 89: > 87: } > 88: > 89: public String getAlgorithm() { Add `@Override`. src/java.base/share/classes/com/sun/crypto/provider/Argon2DerivedKey.java line 93: > 91: } > 92: > 93: public String getFormat() { Add `@Override`. src/java.base/share/classes/com/sun/crypto/provider/Argon2DerivedKey.java line 144: > 142: public void destroy() { > 143: if (key != null) { > 144: Arrays.fill(key, (byte)0); Should you be using a cleaner instead? src/java.base/share/classes/com/sun/crypto/provider/Argon2DerivedKey.java line 174: > 172: * > 173: * @throws java.io.ObjectStreamException if a new object representing > 174: * this DES key could not be created s/DES/Argon2id/ src/java.base/share/classes/com/sun/crypto/provider/Argon2DerivedKey.java line 177: > 175: */ > 176: @java.io.Serial > 177: private Object writeReplace() throws java.io.ObjectStreamException { I question if we need to implement Serializable. Is there a need or use case for this? This will just create a dependency that would be difficult to remove later. Otherwise I think we should just throw an exception that serialization is not supported. src/java.base/share/classes/sun/security/provider/ByteArrayAccess.java line 41: > 39: * This class may also be helpful in improving the performance of the > 40: * crypto code in the SunJCE provider. However, for now it is only > accessible by > 41: * the message digest implementation in the SUN provider. This sentence starting with "However" seems outdated now. src/java.base/share/classes/sun/security/provider/ByteArrayAccess.java line 213: > 211: // > 212: //static long b2lLittle8(byte[] in, int inOfs) { > 213: // return (long) LE.LONG_ARRAY.get(in, inOfs); Remove? test/jdk/javax/crypto/spec/Argon2ParameterSpec/InvalidArgs.java line 28: > 26: * @library /test/lib > 27: * @bug 8253914 > 28: * @summary Ensure that Argon2ParameterSpec builder constructor throw the s/constructor/methods/ ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/29597#discussion_r3580954998 PR Review Comment: https://git.openjdk.org/jdk/pull/29597#discussion_r3580967901 PR Review Comment: https://git.openjdk.org/jdk/pull/29597#discussion_r3580965930 PR Review Comment: https://git.openjdk.org/jdk/pull/29597#discussion_r3580974567 PR Review Comment: https://git.openjdk.org/jdk/pull/29597#discussion_r3580983715 PR Review Comment: https://git.openjdk.org/jdk/pull/29597#discussion_r3580942367 PR Review Comment: https://git.openjdk.org/jdk/pull/29597#discussion_r3580936738 PR Review Comment: https://git.openjdk.org/jdk/pull/29597#discussion_r3580937951 PR Review Comment: https://git.openjdk.org/jdk/pull/29597#discussion_r3580940047 PR Review Comment: https://git.openjdk.org/jdk/pull/29597#discussion_r3581066010 PR Review Comment: https://git.openjdk.org/jdk/pull/29597#discussion_r3581073547 PR Review Comment: https://git.openjdk.org/jdk/pull/29597#discussion_r3581100221 PR Review Comment: https://git.openjdk.org/jdk/pull/29597#discussion_r3578901712 PR Review Comment: https://git.openjdk.org/jdk/pull/29597#discussion_r3578934373 PR Review Comment: https://git.openjdk.org/jdk/pull/29597#discussion_r3580273691
