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