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

[Michael McMahon]

On Fri, 15 Oct 2021 14:47:18 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
>
> Evan Whelan has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Extracted both connectTimeouts to variables and added copyright header to accessor
>  - Removed reflection in favour of injected accessor for KeepAliveCache

Just wondering what actually happens if you encounter this bug? I guess that the post will fail somehow because it has been given a closed connection. If so, would it be possible to write the test to simulate that behavior?

So, say you initially do a HTTP request that succeeds (creating a cached connection). Then, somehow close the connection independent of the keep alive cache. Then attempt the streaming post which will either succeed or fail depending whether the bug is fixed or not.

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

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