On Sat, 7 Sep 2024 13:12:23 GMT, Jaikiran Pai <j...@openjdk.org> wrote:

>> Aleksei Efimov has updated the pull request incrementally with two 
>> additional commits since the last revision:
>> 
>>  - guard against possible integer value overflows
>>  - make startTime a local variable
>
> src/jdk.naming.dns/share/classes/com/sun/jndi/dns/DnsClient.java line 478:
> 
>> 476:                     long elapsedMillis = Math.max(1,
>> 477:                             TimeUnit.NANOSECONDS.toMillis(end - start));
>> 478:                     timeoutLeft = timeoutLeft - (int) elapsedMillis;
> 
> Hello Aleksei, should this `int` cast take into account potential integer 
> value overflow? In theory (and probably even in practice depending on what 
> the initial timeout value was configured to), the `elapsedMillis`, I think 
> can be a value greater than `Integer.MAX_VALUE`, in which case this cast to 
> `int` can cause unexpected computation of `timeoutLeft`.

Thanks for highlighting the overflow problem, Jaikiran. I agree that it could 
happen in practice when timeout and retries configuration properties specify 
timeout value close to `Integer.MAX_VALUE`. Addressed it in 
30f883b8cb31120907002191dbfd88d787c75ec8.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20892#discussion_r1750097522

Reply via email to