RFR: 8286171: HttpClient/2 : Expect:100-Continue blocks indefinitely when response is not 100 [v3]

[Conor Cleary]

On Thu, 9 Jun 2022 11:49:52 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> Conor Cleary has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - 8286171: Package-protected access for method
>>  - 8286171: Added checks for correct response codes
>
> test/jdk/java/net/httpclient/ExpectContinueTest.java line 102:
> 
>> 100:         // Due to limitations of the above Http1 Server, a manual approach is taken to test the hanging with the
>> 101:         // httpclient using Http1 so that the correct response header can be returned for the test case
>> 102:         http1HangServer = new Http1HangServer(saHang);
> 
> It took me a while to understand why we have special server implementation for this. The comment helped :) It reminded me about the `ServerImpl` used by the `HttpServer`, which unconditionally sends a `100` response code when it sees a `Expect: 100-continue` in the request header.

Yep, thats spot on. That is the exact case which made it necessary to include our own Server implementation for the HTTP/1 417 scenario.

> test/jdk/java/net/httpclient/ExpectContinueTest.java line 131:
> 
>> 129:         @Override
>> 130:         public void handle(HttpTestExchange exchange) throws IOException {
>> 131:             try (InputStream is = exchange.getRequestBody();
> 
> Should this also be doing something similar to what the `PostHandler` does for HTTP2 versioned request?

I don't think that is necessary here as in both HTTP/1 and HTTP/2 cases, the flow is exactly the same. The case you mentioned was due to the HTTP/1 server emitting a 100 response code in its implementation without it being explicitly specified in the handler.

> test/jdk/java/net/httpclient/ExpectContinueTest.java line 264:
> 
>> 262: 
>> 263:         HttpRequest getRequest = HttpRequest.newBuilder(getUri)
>> 264:                 .GET()
> 
> Is this missing an `.expectContinue(true)` here?

Its not needed for a GET request as the client is not sending a body in that case. In this test the GET serves only to complete the HTTP/2 upgrade before testing with a POST request.

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

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