On Wed, 26 Apr 2023 20:36:02 GMT, Weijun Wang <wei...@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
