On Tue, 22 Aug 2023 18:21:17 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:
> 
>   format the code

Such a restriction appears to be a flaw in the design of SocketFactory and its 
use in jndi ldap.
Consider the existing updated code  and the inference of your assertion:
 ```
       // create the socket
        if (connectTimeout > 0) {
            InetSocketAddress endpoint =
                    createInetSocketAddress(host, port);
            // unconnected socket
            socket = factory.createSocket();
            // connected socket
            socket.connect(endpoint, connectTimeout);
            if (debug) {
                System.err.println("Connection: creating socket with " +
                        "a timeout using supplied socket factory");
            }
        } else {
            // continue (but ignore connectTimeout)
            if (socket == null) {
                // connected socket
                socket = factory.createSocket(host, port);
                if (debug) {
                    System.err.println("Connection: creating socket using " +
                            "supplied socket factory");
                }
            }
        }``

Your assertion is, "If the custom factory does not implement the method 
creatSocket (no method parameters), it will fail for creating the socket."
Thus, the first block of the, if (connectTimeout > 0) condition, is 
problematic. This assumes that if the timeout is > 0, then the factory will 
implement the no args factory method createSocket(). BUT that means that an 
implementor  of a customer factory would need to have knowledge of the internal 
implemntation of this class, and ensure no args factory method createSocket is 
implemented. If an implementor has knowledge to implement the no args factory 
method when timeout > 0, then they can also have knowldge to implement the no 
args factory method when the timeout is 0 or < 0, as suggested by the 
restructure.

Additionally, this would not prohibit a restructure of the non factory 
createConnectionSocket.

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

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

Reply via email to