RFR: 8314063 : The socket is not closed in Connection::createSocket when the handshake failed for LDAP connection [v7]

[Andrey Turbanov]

On Sat, 19 Aug 2023 02:15:06 GMT, Weibing Xiao <duke at openjdk.org> wrote:

>> Please refer to JDK-8314063.
>> 
>> The failure scenario is due to the setting of connection timeout. It is either too small or not an optimal value for the system. When the client tries to connect to the server with LDAPs protocol. It requires the handshake after the socket is created and connected, but it fails due to connection timeout and leaves the socket open. It is not closed properly due to the exception handling in the JDK code.
>> 
>> The change is adding a try/catch block and closing the socket in the catch block,  and the format of the code got changed consequently.
>
> Weibing Xiao has updated the pull request incrementally with one additional commit since the last revision:
> 
>   refactor the code and test cases

test/jdk/com/sun/jndi/ldap/LdapSSLHandshakeFailureTest.java line 77:

> 75:         // start the test server first.
> 76:         boolean serverSlowDown = false;
> 77:         if(args.length ==2 ) {

Suggestion:

        if (args.length ==2 ) {

test/jdk/com/sun/jndi/ldap/LdapSSLHandshakeFailureTest.java line 80:

> 78:             serverSlowDown = Boolean.valueOf(args[1]);
> 79:         } else {
> 80:             if(args.length ==1 ) {

Suggestion:

            if (args.length ==1) {

test/jdk/com/sun/jndi/ldap/LdapSSLHandshakeFailureTest.java line 98:

> 96:         }
> 97:         env.put("java.naming.ldap.version", "3");
> 98:         if (args.length == 2 ) {

Suggestion:

        if (args.length == 2) {

test/jdk/com/sun/jndi/ldap/LdapSSLHandshakeFailureTest.java line 99:

> 97:         env.put("java.naming.ldap.version", "3");
> 98:         if (args.length == 2 ) {
> 99:             if( args[0].contains("LdapSSLHandshakeFailureTest")) {

Suggestion:

            if (args[0].contains("LdapSSLHandshakeFailureTest")) {

test/jdk/com/sun/jndi/ldap/LdapSSLHandshakeFailureTest.java line 122:

> 120:             }
> 121:         } finally {
> 122:             if(ctx != null)

Suggestion:

            if (ctx != null)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15294#discussion_r1299160346
PR Review Comment: https://git.openjdk.org/jdk/pull/15294#discussion_r1299160377
PR Review Comment: https://git.openjdk.org/jdk/pull/15294#discussion_r1299160495
PR Review Comment: https://git.openjdk.org/jdk/pull/15294#discussion_r1299160402
PR Review Comment: https://git.openjdk.org/jdk/pull/15294#discussion_r1299160439