Nice find Chris.
But I have to wonder why the NPE wasn't thrown at the
very beginning of `authenticate` namely one of these
two lines:
int readTimeout = conn.readTimeout;
conn.readTimeout = conn.connectTimeout;
which happen to be called even before ensureOpen() is
called a few lines later!
cheers,
-- daniel
On 10/09/2018 13:00, Chris Hegarty wrote:
Vyom,
The NPE is originating from the finally block of the LdapClient
`authenticate` method. In the finally block there is an attempt
to restore the "read" timeout, since it was changed earlier in
`authenticate`, to reflect the connect timeout value, since the
subsequent operation(s) are considered part of the connect phase.
An unsolicited message may close the connection during
authenticate, which is does and LdapClient.ldapBind throws
"javax.naming.CommunicationException: Request: 1 cancelled". The
finally block is then executed and the previous exception is
effectively throw away when the NPE is encountered.
If we agree that such behaviour is reasonable ( and I think it
most likely is ), then the finally block needs to be more
defensive - check that conn is not null. It makes no sense to
attempt to reset the read timeout if conn is null.
With the following change, then the CommunicationException will
be thrown from `authenticate`. I think this is the behaviour we
want, no?
--- a/src/java.naming/share/classes/com/sun/jndi/ldap/LdapClient.java
+++ b/src/java.naming/share/classes/com/sun/jndi/ldap/LdapClient.java
@@ -297,7 +297,8 @@
conn.setV3(isLdapv3);
return res;
} finally {
- conn.readTimeout = readTimeout;
+ if (conn != null)
+ conn.readTimeout = readTimeout;
}
}
-Chris.
