On Thu, 23 Oct 2025 19:53:12 GMT, Bradford Wetmore <[email protected]> wrote:
>> Mikhail Yankelevich has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> line length changed
>
> src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java line 256:
>
>> 254: NumberFormatException |
>> 255: NoSuchAlgorithmException |
>> 256: IndexOutOfBoundsException e) {
>
> I always worry about enumerating all possible checked exceptions, as we might
> now start throwing new `RuntimeException`s that were previously
> caught/ignored. Was there a reason to drive the change to all checked?
>
> More importantly, will this change now cause new behavior due to RTEs?
Are you more interested in practical or dogmatic reasons?
For practical reasons, see
[JDK-8309667](https://bugs.openjdk.org/browse/JDK-8309667); there was a bug in
Keystore implementation that was hidden by this catch block, and impossible to
investigate without changing it.
For dogmatic, see our own secure coding guidelines, point 1-4 Implement robust
exception handling. KeyManager is not equipped to handle the runtime exceptions
thrown by KeyStores and Builders, and should propagate these exceptions to the
caller instead.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27851#discussion_r2459467367
