On Wed, 16 Aug 2023 23:11:11 GMT, Weibing Xiao <d...@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:
> 
>   updated the code according to the review

together with a refactor extact methods on the createSocket method, the 
existing logic could be simplified something like as follows:


            InetSocketAddress endpoint =
               createInetSocketAddress(ldapServerHost, ldapServerPort);

            if (socketFactory != null) {
                 // further refine with refactor extract method 
createConnectionSocket(InetSocketAddress serverEndpoint, String 
factoryClassName, int timeout)

                // create the factory

                @SuppressWarnings("unchecked")
                Class<? extends SocketFactory> socketFactoryClass =
                        (Class<? extends SocketFactory>) 
Obj.helper.loadClass(socketFactory);
                Method getDefault =
                        socketFactoryClass.getMethod("getDefault", new 
Class<?>[]{});
                SocketFactory factory = (SocketFactory) getDefault.invoke(null, 
new Object[]{});
 
                // create unconnected socket
                socket = factory.createSocket();

               if (connectTimeout > 0) {                 
                  if (debug) {
                        System.err.println("Connection.createSocket: creating 
socket with " +
                                "a timeout using supplied socket factory");
                    }

                    // connect socket
                    socket.connect(endpoint, connectTimeout);
               } else {
                   if (debug) {
                        System.err.println("Connection.createSocket: creating 
socket using " +
                                "supplied socket factory");
                    }
                    // connect socket
                    socket.connect(endpoint);
               }
             } else { // NO SocketFactory
                // further refine with refactor extract method 
createConnectionSocket(InetSocketAddress serverEndpoint,  int timeout)
                socket = new Socket();
                if (connectTimeout > 0) {
                    if (debug) {
                        System.err.println("Connection.createSocket: creating 
socket with " +
                                "a timeout");
                    }
                    socket.connect(endpoint, connectTimeout);
                } else {
                   if (debug) {
                        System.err.println("Connection.createSocket: creating 
socket");
                   }
                   socket.connect(endpoint);
              }

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

PR Comment: https://git.openjdk.org/jdk/pull/15294#issuecomment-1681403755

Reply via email to