On Mon, 11 Sep 2023 15:09:22 GMT, Conor Cleary <[email protected]> wrote:
> ### **Issue Description**
> There is missing test coverage for the implementation of
> partial/informational responses (status code 1xx in server response headers)
> for the `HttpClient`, specifically in the `HTTP/2` use case. RFC 9113 -
> HTTP/2, details the behavior of any client in these situations. Small changes
> and new test cases are needed to verify compliance with this specification.
> This issue primarily concerns how the client reacts to receiving a
> `RST_STREAM` frame at various stages of a partial/informational response from
> a server.
>
> ### **Solution Details**
> Minor changes were made in
> `src/java.net.http/share/classes/jdk/internal/net/http/Stream.java` to
> improve the handling client's handling of receiving a `RST_STREAM`.
> `incoming_reset()` (L574) will now cause the client to ignore a `RST_STREAM`
> frame if an `END_STREAM` flag has been received _and_ the request body has
> completed sending. Previously it would be ignored only if an `END_STREAM`
> flag was seen which caused cancelled partial responses to 'hang' indefinitely
> should a client be transmitting data in a POST/PUT request. Some changes
> where also made to the `schedule()` method in Stream (L190) to ensure both
> the sending of the Request Body and receipt of a RST_STREAM at various stages
> of an exchange do not cause unexpected behavior.
>
> Minor changes were made to the `Http2TestServer` implementation to improve
> the convinience of testing edge cases involving the sending of `HTTP/2`
> response headers.
>
> Concerning the new test cases, I have listed below the specifics of each case
> and the expected behavior of the client in the given scenario.
>
> **test/jdk/java/net/httpclient/ExpectContinueTest.java**
> - Client sends a POST request with the `Expect: 100-Continue` header included
> - Server responds with a `HEADERS` frame including a 100 status code and
> reads the request body. Server then sends `RST_STREAM` with code `NO_ERROR`
> or `PROTOCOL_ERROR` set.
> - Expected/Observed Client Behavior: Client completes exceptionally in
> both cases.
> - Client sends a POST request with the `Expect: 100-Continue` header included
> - Server responds with a `HEADERS` frame including a 100 status code and
> reads the request body. Server then sends Response Headers with status code
> 200 to complete the response. Server then sends RST_STREAM with code
> `NO_ERROR` or `PROTOCOL_ERROR` set.
> - Expected/Observed Client Behavior: Client completes exceptionally in
> both cases.
> - Client sends a POST request with the `Expect: 10...
test/jdk/java/net/httpclient/ExpectContinueTest.java line 103:
> 101: // URI, Expected Status Code, Will finish with
> Exception, Protocol Version
> 102: { postUri, 200, false, HTTP_1_1 },
> 103: { hangUri, 417, false, HTTP_1_1},
Suggestion:
{ hangUri, 417, false, HTTP_1_1},
test/jdk/java/net/httpclient/ExpectContinueTest.java line 104:
> 102: { postUri, 200, false, HTTP_1_1 },
> 103: { hangUri, 417, false, HTTP_1_1},
> 104: { h2postUri, 200, false, HTTP_2 },
Suggestion:
{ h2postUri, 200, false, HTTP_2 },
test/jdk/java/net/httpclient/http2/ExpectContinueResetTest.java line 228:
> 226: public void handle(Http2TestExchange exchange) throws
> IOException {
> 227: err.println("Sending 100");
> 228: exchange.sendResponseHeaders(100, 0);
Suggestion:
exchange.sendResponseHeaders(100, 0);
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15664#discussion_r1324961996
PR Review Comment: https://git.openjdk.org/jdk/pull/15664#discussion_r1324962219
PR Review Comment: https://git.openjdk.org/jdk/pull/15664#discussion_r1324961716
