On Fri, 13 Sep 2024 19:39:04 GMT, Kevin Driver <kdri...@openjdk.org> wrote:
>> Introduce an API for Key Derivation Functions (KDFs), which are >> cryptographic algorithms for deriving additional keys from a secret key and >> other data. See [JEP 478](https://openjdk.org/jeps/478). >> >> Work was begun in [another PR](https://github.com/openjdk/jdk/pull/18924). > > Kevin Driver has updated the pull request incrementally with one additional > commit since the last revision: > > refine wording on DPS getInstance with params exception Comments on the API spec in `KDF.java`. I'll need some more time to look at the implementation. src/java.base/share/classes/javax/crypto/KDF.java line 194: > 192: * otherwise {@code null} is returned. > 193: * > 194: * @see <a href="#DelayedProviderSelection">Delayed Provider > Selection</a> Why `@see DPS`? The paragraph above has nothing to do with it. If you meant the warning not to call it too early (which I think is not necessary), why not add the same to the `getProviderName` method? src/java.base/share/classes/javax/crypto/KDF.java line 207: > 205: * Returns a {@code KDF} object that implements the specified > algorithm. > 206: * > 207: * @see <a href="#DelayedProviderSelection">Delayed Provider > Selection</a> Put `@see` at the end. Put `@implNote` before `@param`. This is the order they are rendered in HTML javadoc. src/java.base/share/classes/javax/crypto/KDF.java line 266: > 264: * list > 265: * @throws NullPointerException > 266: * if the {@code algorithm} or {@code provider} is {@code null} Remove the `the` word. Same below. src/java.base/share/classes/javax/crypto/KDF.java line 345: > 343: * if at least one {@code Provider} supports a {@code KDFSpi} > 344: * implementation for the specified algorithm but none of them > 345: * support the specified parameters One of the two `@throws` above uses `supports a {@code KDF} implementation` and the other uses `supports a {@code KDFSpi} implementation`. We'd better choose the same class name. src/java.base/share/classes/javax/crypto/KDF.java line 426: > 424: * @throws InvalidAlgorithmParameterException > 425: * if the specified provider does not support a {@code KDFSpi} > 426: * implementation for the specified algorithm and parameters Do not mention `algorithm` in this `@throws`. It's already covered by `@throws NSAE`. Same below. src/java.base/share/classes/javax/crypto/KDF.java line 530: > 528: * results in something invalid > 529: * @throws NoSuchAlgorithmException > 530: * if {@code alg} is empty or invalid Is it easy to tell precisely what falls into "combination of alg and derivationSpec results in something invalid" and what falls into "alg is invalid"? src/java.base/share/classes/javax/crypto/KDF.java line 555: > 553: > 554: /** > 555: * Obtains raw data from a key derivation function. The first sentences of the two `derive` methods use different verbs: `Derives` and `Obtains`. Is it possible to use a same one? ------------- PR Review: https://git.openjdk.org/jdk/pull/20301#pullrequestreview-2305207448 PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1759845500 PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1759869223 PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1759877457 PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1759878466 PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1759878857 PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1759883966 PR Review Comment: https://git.openjdk.org/jdk/pull/20301#discussion_r1759882929