RFR: 8305906: HttpClient may use incorrect key when finding pooled HTTP/2 connection for IPv6 address

Jaikiran Pai jpai at openjdk.org
Thu Apr 20 11:06:51 UTC 2023


On Thu, 13 Apr 2023 11:13:07 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

> Can I please get a review of this change which proposes to fix the issue reported in https://bugs.openjdk.org/browse/JDK-8305906?
> 
> As noted in that issue, when IPv6 hosts are involved, the HttpClient on certain occasions can end up caching the connection with a key which doesn't match with the key which is then used in a subsequent request against the same target host. 
> 
> The commit in this PR now wraps the IPv6 address in a square bracket consistently so that the correct key is used both during storing the connection in the pool and when looking up.
> 
> A new jtreg test has been added which reproduces this issue without the fix and verifies the fix.

Thank you for catching that, Daniel. You are indeed right, the fix missed this part. I updated the new `ConnectionReuseTest` to run a test which matches what you described and that too fails, with the fix I had in this PR.

I have now updated the PR to use `HttpRequestImpl.getAddress()` (which returns a `InetSocketAddress`) instead of `HttpRequestImpl.getURI()` while looking up the cache keys. What this does is that it gets rid of the usage of the `URI.getHost()` API completely, during connection cache lookups. Instead, this now relies solely on the `InetAddress` API which always returns a long form address string. This is similar to what we do in the HTTP1 connection caching internally.

With this change the `ConnectionReuseTest` passes, even with the newly added additional test. I will run tier tests to make sure this doesn't impact anything else.

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

PR Comment: https://git.openjdk.org/jdk/pull/13456#issuecomment-1516134613


More information about the net-dev mailing list