RFR: 8324209: Check implementation of Expect: 100-continue in the java.net.http.HttpClient

[Daniel Fuchs]

On Fri, 9 Aug 2024 13:46:30 GMT, Darragh Clarke <dclarke at openjdk.org> wrote:

> Currently `HttpClient` will timeout if a server doesn't respond to a request which includes `Expect: 100-Continue`
> 
> Section 10.1.1 of [rfc 9110](https://datatracker.ietf.org/doc/html/rfc9110#name-expect) states that
> 
> a client SHOULD NOT wait for an indefinite period before sending the content.
> 
> 
> 
> This PR changes `HttpClient` to wait for a maximum of 5 seconds for a server response, this will be shorter if a timeout is set. If no response is received, the message will be sent regardless. 
> This should bring `HttpClient` in line with how [HttpUrlConnection](https://github.com/DarraghClarke/jdk/blob/61386c199a3b29457c002ad31a23990b7f6f88fd/src/java.base/share/classes/sun/net/www/protocol/http/HttpURLConnection.java#L1305) treats expect continue timeouts.
> 
> This is done using `orTimeout` in the `expectContinue` method , though there is some changes in `streams.java` where it was possible for race conditions to cause timeouts where `CompleteableFuture`s were removed from `response_cfs` prematurely or in some cases not removed at all.
> 
> I've tested this against tiers 1-3 and it appears to be stable.

Look good in general. Some minor comments.

src/java.net.http/share/classes/jdk/internal/net/http/ExchangeImpl.java line 61:

> 59:     final Exchange<T> exchange;
> 60: 
> 61:     volatile boolean expectTimeoutRaised;

Could you make this field private and add a method to get it instead? For instance 
`final boolean expectTimeoutRaised() { return expectTimeoutRaised;}`

test/jdk/java/net/httpclient/ExpectContinueTest.java line 102:

> 100:                 { hangUri, 417, false, HTTP_1_1},
> 101:                 { h2postUri, 200, false, HTTP_2 },
> 102:                 { h2forcePostUri, 200, false, HTTP_2 },

It would be good to test 'forcePost' with HTTP/1.1 too.

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

PR Review: https://git.openjdk.org/jdk/pull/20525#pullrequestreview-2230630358
PR Review Comment: https://git.openjdk.org/jdk/pull/20525#discussion_r1711777928
PR Review Comment: https://git.openjdk.org/jdk/pull/20525#discussion_r1711782168