RFR: 8274779: HttpURLConnection: HttpClient and HttpsClient incorrectly check request method when set to POST

[Daniel Fuchs]

On Fri, 15 Oct 2021 08:49:20 GMT, Evan Whelan <ewhelan at openjdk.org> wrote:

> Hi,
> 
> Please review my fix for JDK-8274779 which changes how HttpClient and HttpsClient  checks for equality when comparing request methods. 
> 
> When `HttpURLConnection.setRequestMethod` is passed `new String("POST")` rather than the "POST" String literal, the old behaviour resulted in broken HttpClients being reused from the `KeepAliveCache`. 
> 
> This is because a call to `HttpClient.available()` was never reachable due to identity equality being used instead of logical equality.
> 
> The test case uses an injected KeepAliveCache, to which we put a HttpClient that is unavailable. By comparing the initial HttpClient's `connectTimeout` value to the "cached" client's connectTimeout (1234 vs 4321 respectively) we can assert that these values should never be equal as a new HttpClient should be created in cases where we can no longer use the cached one.
> 
> All CI testing is green for this fix.
> 
> Kind regards,
> Evan

Code changes look good. I am a bit more suspicious of the test.

test/jdk/sun/net/www/http/KeepAliveCache/RequestMethodEquality.java line 32:

> 30:  * @modules java.base/sun.net.www.http:+open
> 31:  *          java.base/sun.net.www.protocol.http
> 32:  * @run testng RequestMethodEquality

This test modifies some static fields in HttpClient so should run in /othervm

test/jdk/sun/net/www/http/KeepAliveCache/RequestMethodEquality.java line 95:

> 93:             // Injecting a mock KeepAliveCache that the HttpClient can use
> 94:             KeepAliveCache kac = new KeepAliveCache();
> 95:             kac.put(url, null, freshClient);

Instead of injecting a new KeepAliveCache you could get the cache already present in HttpClient. One way to do this is to use reflection - another is to inject an accessor class in the sun.net.www.http package. You may not even need to have to create a "fresh" HttpClient - there should be one there corresponding to the connection you already created?

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

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