RFR: 8371721: Refactor checkTrusted methods in X509TrustManagerImpl [v4]

Sean Coffey coffeys at openjdk.org
Wed Dec 10 20:46:40 UTC 2025


On Fri, 5 Dec 2025 17:00:18 GMT, Artur Barashev <abarashev at openjdk.org> wrote:

>> The 3 checkTrusted methods in X509TrustManagerImpl have a lot of repeating code that can be moved into a helper method.
>
> Artur Barashev has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Only the last few sentences of javadoc are outdated

src/java.base/share/classes/sun/security/ssl/X509TrustManagerImpl.java line 210:

> 208: 
> 209:         if (socket instanceof SSLSocket sslSocket && sslSocket.isConnected()) {
> 210:             session = sslSocket.getHandshakeSession();

subtle change in the refactoring now that the session non-null check is delayed until the new `findTrustedCertificate` call. 
The `SSLAlgorithmConstraints.forEngine/forSocket/forQUIC` methods also reference the session before the `findTrustedCertificate` call . Have you ensured that a null session can't cause issue there ?

src/java.base/share/classes/sun/security/ssl/X509TrustManagerImpl.java line 261:

> 259:     }
> 260: 
> 261:     private void findTrustedCertificate(X509Certificate[] chain,

good to see the refactoring take place. However the `findTrustedCertificate` method name suggests a return value (for me at least)

wondering is something like `ensureTrustedCertificate` or similar might be better for a method returning void ?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/28275#discussion_r2608145437
PR Review Comment: https://git.openjdk.org/jdk/pull/28275#discussion_r2608151422


More information about the security-dev mailing list