RFR: 8297878: KEM: Implementation [v10]

[Sean Mullan]

On Wed, 26 Apr 2023 20:36:02 GMT, Weijun Wang <weijun at openjdk.org> wrote:

>> src/java.base/share/classes/javax/crypto/KEM.java line 606:
>> 
>>> 604:      * @param publicKey the receiver's public key, must not be {@code null}
>>> 605:      * @return the encapsulator for this key
>>> 606:      * @throws InvalidKeyException if {@code publicKey} is invalid
>> 
>> What about throwing `NullPointerException` if `publicKey` is `null`? In this case a `RuntimeException` seems more appropriate, and throwing NPE seems more consistent with how you treat `null` parameters of other methods.
>
> Every other crypto engine class throws IKE when initing with a null key, including `Signature`, `Cipher`, `KeyAgreement`, 'Mac`. So I follow the same style.

Hmm, while I understand your thinking, this seems like the wrong pattern to follow though (ex: see Effective Java, Item 70), and it doesn't seem like something we should have to adhere to for API consistency. There are various bugs filed over the years on confusion over this style (search for "Cipher init NullPointerException"), where providers threw NPE or users thought the APIs should. Actually, there is still an open bug: https://bugs.openjdk.org/browse/JDK-4955097

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/13256#discussion_r1178390743