RFR: 8325579: Inconsistent behavior in com.sun.jndi.ldap.Connection::createSocket [v5]

[Aleksei Efimov]

On Fri, 16 Feb 2024 10:27:11 GMT, Christoph Langer <clanger at openjdk.org> wrote:

>> During analysing a customer case I figured out that we have an inconsistency between documentation and actual behavior in class com.sun.jndi.ldap.Connection. The [method documentation of com.sun.jndi.ldap.Connection::createSocket](https://github.com/openjdk/jdk/blob/3ebe6c192a5dd5cc46ae2d263713c9ff38cd46bb/src/java.naming/share/classes/com/sun/jndi/ldap/Connection.java#L281) states: "If a timeout is supplied but unconnected sockets are not supported then the timeout is ignored and a connected socket is created."
>> 
>> This, however does not happen. If a SocketFactory would not support unconnected sockets, it would likely throw a SocketException in [SocketFactory::createSocket()](https://github.com/openjdk/jdk/blob/6303c0e7136436a2d3cb6043b88edf788c0067cc/src/java.base/share/classes/javax/net/SocketFactory.java#L123). And since [the code](https://github.com/openjdk/jdk/blob/3ebe6c192a5dd5cc46ae2d263713c9ff38cd46bb/src/java.naming/share/classes/com/sun/jndi/ldap/Connection.java#L336) does not check for this behavior, a connection with timeout value through a SocketFactory that does not support unconnected sockets would simply fail with an IOException.
>> 
>> So we should either make the code adhere to what is documented or adapt the documentation to the actual behavior.
>> 
>> I hereby try to fix the connect coding. Alternatively, we could also adapt the description - I have no strong opinion. What do the experts suggest?
>
> Christoph Langer has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision:
> 
>  - Typo
>  - Merge branch 'master' into JDK-8325579
>  - Rename test and refine comment
>  - Enhance test
>  - JDK-8325579

Currently, it is hard to distinguish what part of the test responsible for [JDK-8314063](https://bugs.openjdk.org/browse/JDK-8314063) testing, and which part is for [JDK-8325579](https://bugs.openjdk.org/browse/JDK-8325579).
I would prefer to add a new test for the current fix instead: that could be done as additional test mode, OR even better to add a completely new test.
As Daniel stated before, it is hard to review the change made in this PR because of the test name change. Previous suggestion might help to address that too.

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

PR Comment: https://git.openjdk.org/jdk/pull/17797#issuecomment-1952861854