RFR: 8314063 : The socket is not closed in Connection::createSocket when the handshake failed for LDAP connection [v7]
Aleksei Efimov
aefimov at openjdk.org
Mon Aug 21 13:23:34 UTC 2023
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 69:
> 67: public class LdapSSLHandshakeFailureTest {
> 68: private static String SOCKET_CLOSED_MSG = "The socket has been closed.";
> 69: private static String SOCKET_NOT_CLOSED_MSG = "The socket was not closed.";
`SOCKET_NOT_CLOSED_MSG` - not used and can be removed
test/jdk/com/sun/jndi/ldap/LdapSSLHandshakeFailureTest.java line 76:
> 74: setKeyStore();
> 75: // start the test server first.
> 76: boolean serverSlowDown = false;
`false` initializer is redundant here
test/jdk/com/sun/jndi/ldap/LdapSSLHandshakeFailureTest.java line 178:
> 176: }
> 177:
> 178: private static void setKeyStore() throws Exception {
`Exception` is not thrown by this method and can be removed
test/jdk/com/sun/jndi/ldap/LdapSSLHandshakeFailureTest.java line 211:
> 209: @Override
> 210: public void run() {
> 211: try (Socket socket = serverSocket.accept(); InputStream in = socket.getInputStream();
formatting: this can be made shorter by putting `InputStream in = socket.getInputStream();` on a separate line
test/jdk/com/sun/jndi/ldap/LdapSSLHandshakeFailureTest.java line 216:
> 214: Thread.sleep(5000);
> 215: }
> 216: byte[] bindResponse = {0x30, 0x0C, 0x02, 0x01, 0x01, 0x61, 0x07, 0x0A, 0x01, 0x00, 0x04, 0x00, 0x04, 0x00};
formatting: can be splitted in two lines to make it shorter
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15294#discussion_r1300093748
PR Review Comment: https://git.openjdk.org/jdk/pull/15294#discussion_r1300092781
PR Review Comment: https://git.openjdk.org/jdk/pull/15294#discussion_r1300095645
PR Review Comment: https://git.openjdk.org/jdk/pull/15294#discussion_r1300099068
PR Review Comment: https://git.openjdk.org/jdk/pull/15294#discussion_r1300100447
More information about the core-libs-dev
mailing list