RFR: 8369995: Implement extra logging and/or propagate errors in X509KeyManagerImpl and fix StringIndexOutOfBoundsException [v4]

Artur Barashev abarashev at openjdk.org
Fri Oct 17 16:23:09 UTC 2025


On Fri, 17 Oct 2025 15:57:07 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.
>> 
>> Additionally, dot checking logic has been changed, so no cases similar to `.A` will not trigger StringOutOfBounds exceptions. 
>> 
>> Thank you @djelinski for finding the issue and analysis.
>
> Mikhail Yankelevich has updated the pull request incrementally with one additional commit since the last revision:
> 
>   preventing double run of tests that don't need it; Changed messages and made the error message a warning

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

> 224:         int firstDot = alias.indexOf('.');
> 225:         int secondDot = alias.indexOf('.', firstDot + 1);
> 226:         if ((firstDot == -1) || (secondDot == -1)) {

The following code looks correct to me to check for a proper alias format and to avoid Out of Bounds exception. Please add a test case for that:

        // parse the alias
        int firstDot = alias.indexOf('.');
        int secondDot = -1;

        if (firstDot > 0 && alias.length() - firstDot > 3) {
            secondDot = alias.indexOf('.', firstDot + 1);
        }

        if (secondDot - firstDot < 2 || alias.length() - secondDot < 2) {
            if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) {
                SSLLogger.warning("Invalid alias format: " + alias);
            }
            return null;
        }

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

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


More information about the security-dev mailing list