On Thu, 13 Apr 2023 22:29:34 GMT, Weijun Wang <wei...@openjdk.org> wrote:
>> The KEM API and DHKEM impl. Note that this PR uses new methods in >> https://github.com/openjdk/jdk/pull/13250. > > Weijun Wang has updated the pull request incrementally with one additional > commit since the last revision: > > spec change, getAlgorithm src/java.base/share/classes/javax/crypto/KEM.java line 49: > 47: * If a provider is not specified in the {@code getInstance} method when > 48: * instantiating a {@code KEM} object, the {@code newEncapsulator} and > 49: * {@code newDecapsulator} method may return encapsulators or > decapsulators s/method/methods/ src/java.base/share/classes/javax/crypto/KEM.java line 51: > 49: * {@code newDecapsulator} method may return encapsulators or > decapsulators > 50: * from different providers. The provider selected is based on the > parameters > 51: * passed to the the {@code newEncapsulator} or {@code newDecapsulator} > methods: s/the the/the/ src/java.base/share/classes/javax/crypto/KEM.java line 54: > 52: * the private or public key and the optional {@code > AlgorithmParameterSpec}. > 53: * The {@link Encapsulator#provider} and {@link Decapsulator#provider} > methods > 54: * return the selected provider." remove "". src/java.base/share/classes/javax/crypto/KEM.java line 103: > 101: * @param key the shared secret as a key, must not be {@code > null}. > 102: * @param encapsulation the key encapsulation message, must not > be {@code null}. > 103: * @param params additional parameters, can be {@code null}. Nit: remove periods. src/java.base/share/classes/javax/crypto/KEM.java line 105: > 103: * @param params additional parameters, can be {@code null}. > 104: */ > 105: public Encapsulated { Should this also have an `@throws NullPointerException` clause? src/java.base/share/classes/javax/crypto/KEM.java line 128: > 126: * Returns the provider. > 127: * > 128: * @return thr provider s/thr/the/ src/java.base/share/classes/javax/crypto/KEM.java line 143: > 141: * @param encapsulation the key encapsulation message from the > sender > 142: * @return the shared secret as a {@code SecretKey} with > 143: * algorithm being "Generic", not {@code null} See my comment below, I don't think you need to specify that this does not return null, it should be implied. src/java.base/share/classes/javax/crypto/KEM.java line 160: > 158: * @param algorithm the algorithm for the secret key returned > 159: * @return a portion of the shared secret as a {@code SecretKey} > with > 160: * the specified algorithm, not {@code null} Same comment as above. src/java.base/share/classes/javax/crypto/KEM.java line 242: > 240: * shared secret as a key with algorithm being > "Generic", > 241: * the key encapsulation message, and optional > parameters. > 242: * The return value should not be {@code null}. "should" means it *could* still return null. I assume that is not what we want. Although I would be more inclined to only specify cases where null may be returned, and if it isn't mentioned, then it should be implied that null is not a legal return value. If in doubt, perhaps check with Joe/CCC for advice when you file the CSR. This general comment applies to the other return types in this API where you say "not null". I think you can omit those. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/13256#discussion_r1166801682 PR Review Comment: https://git.openjdk.org/jdk/pull/13256#discussion_r1166802062 PR Review Comment: https://git.openjdk.org/jdk/pull/13256#discussion_r1166802596 PR Review Comment: https://git.openjdk.org/jdk/pull/13256#discussion_r1166808093 PR Review Comment: https://git.openjdk.org/jdk/pull/13256#discussion_r1166811613 PR Review Comment: https://git.openjdk.org/jdk/pull/13256#discussion_r1166826429 PR Review Comment: https://git.openjdk.org/jdk/pull/13256#discussion_r1166829885 PR Review Comment: https://git.openjdk.org/jdk/pull/13256#discussion_r1166830271 PR Review Comment: https://git.openjdk.org/jdk/pull/13256#discussion_r1166841781
