> 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 12 additional commits 
since the last revision:

 - Update module-info text
 - Merge branch 'master' into JDK-8325579
 - Indentation
 - Merge branch 'master' into JDK-8325579
 - 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
 - ... and 2 more: https://git.openjdk.org/jdk/compare/584e58bb...10271159

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

Changes:
  - all: https://git.openjdk.org/jdk/pull/17797/files
  - new: https://git.openjdk.org/jdk/pull/17797/files/97299970..10271159

Webrevs:
 - full: https://webrevs.openjdk.org/?repo=jdk&pr=17797&range=07
 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=17797&range=06-07

  Stats: 97262 lines in 1251 files changed: 15655 ins; 78153 del; 3454 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

Reply via email to