On Thu, 9 Jun 2022 11:49:52 GMT, Jaikiran Pai <j...@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
