RFR: 8325579: Inconsistent behavior in com.sun.jndi.ldap.Connection::createSocket [v6]
Christoph Langer
clanger at openjdk.org
Tue Feb 20 09:45:22 UTC 2024
> 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 eight additional commits since the last revision:
- Review feedback
- Rename back to LdapSSLHandshakeFailureTest to ease reviewing
- Merge branch 'master' into JDK-8325579
- Typo
- Merge branch 'master' into JDK-8325579
- Rename test and refine comment
- Enhance test
- JDK-8325579
-------------
Changes:
- all: https://git.openjdk.org/jdk/pull/17797/files
- new: https://git.openjdk.org/jdk/pull/17797/files/d8d1d6db..ec6579d2
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=17797&range=05
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=17797&range=04-05
Stats: 5175 lines in 157 files changed: 1842 ins; 1842 del; 1491 mod
Patch: https://git.openjdk.org/jdk/pull/17797.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/17797/head:pull/17797
PR: https://git.openjdk.org/jdk/pull/17797
More information about the core-libs-dev
mailing list