RFR: 8369995: Implement extra logging and/or propagate errors in X509KeyManagerImpl

[Daniel Jeliński]

On Thu, 16 Oct 2025 16:34:50 GMT, Mikhail Yankelevich <myankelevich at openjdk.org> wrote:

> In [JDK-8309667](https://bugs.openjdk.org/browse/JDK-8309667), there were issues with debugging due to no logging or throwing of errors by X509KeyManagerImpl::getEntry. [Line](https://github.com/openjdk/jdk/blob/6a4c2676a6378f573bd58d1bc32b57765d756291/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java#L243-L245)
> 
> Extra logging and error propagating should be implemented for the X509KeyManagerImpl.
> 
> Thank you @djelinski for finding the issue and analysis.

src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java line 248:

> 246:                  KeyStoreException |
> 247:                  NumberFormatException |
> 248:                  NoSuchAlgorithmException e) {

Could you fix the (preexisting) IndexOutOfBounds issues?
- the check in line 226 looks wrong; `secondDot == firstDot` will always return `false`. It should be `secondDot == -1` instead; the current version will probably throw SIOOBE on aliases with a single dot, like `A.1`.
- `builders.get(builderIndex)` might throw IOOBE if builderIndex is out of range for the builders list, like `A.-1.B`.

The only exceptions that we want to propagate here are the `RuntimeException`s thrown by `builder` and `ks`, and we only propagate them so that they can be reported and fixed.

test/jdk/sun/security/ssl/X509KeyManager/X509KeyManagerNegativeTests.java line 58:

> 56:     @Test
> 57:     public void getCertificateChainIncompleteString() {
> 58:         Assert.assertThrows(StringIndexOutOfBoundsException.class,

Once you fix the KeyManager, you'll need a different set of tests here.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27851#discussion_r2437051091
PR Review Comment: https://git.openjdk.org/jdk/pull/27851#discussion_r2437095546