RFR: JDK-8277795: ldap connection timeout not honoured under contention [v3]

[Daniel Fuchs]

On Thu, 13 Jan 2022 10:09:45 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:

>> src/java.naming/share/classes/com/sun/jndi/ldap/LdapClientFactory.java line 70:
>> 
>>> 68:     public PooledConnection createPooledConnection(PoolCallback pcb, long timeout)
>>> 69:         throws NamingException {
>>> 70:         return new LdapClient(host, port, socketFactory,
>> 
>> any need to perform sanity check against erroneous negative values on the timeout supplied here and in other parts of the solution
>
> Hmmm... Good point. I had looked into this yesterday when I reviewed - and AFAIU a value <= 0 would be interpreted as no timeout (that is, infinite timeout) - and that seems consistent throughout. It's non obvious - but I convinced myself that passing a negative value here would not necessarily be an error, and would work as expected. However the narrowing down of a negative long to an int doesn't necessarily preserve the sign.
> @robm-openjdk the conversion from long to int probably needs to also take care of values that are < Integer.MIN_VALUE. 
> 
> 
> jshell> long l = Integer.MIN_VALUE * 2L
> l ==> -4294967296
> 
> jshell> int x = (int)l
> x ==> 0
> 
> jshell> long l = Integer.MIN_VALUE * 2L + 1
> l ==> -4294967295
> 
> jshell> int x = (int)l
> x ==> 1

(Though I don't  think it can happen - but maybe I'm mistaken)
In any case it's safer to sanitize the input.

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

PR: https://git.openjdk.java.net/jdk/pull/6568