RFR: 8309118: HttpClient: Add more tests for 100 ExpectContinue with HTTP/2
Conor Cleary
ccleary at openjdk.org
Tue Sep 12 09:37:12 UTC 2023
### **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: 100-Continue` header included
- Server responds with a `HEADERS` frame including a 100 status code, _does not read the request body_. Server then sends Response Headers with status code 200 to complete the response. After this, two different cases are checked, given below.
- Server sends an empty `DATA_FRAME` with the `END_STREAM` flag set, followed by a `RST_STREAM` with an arbitrary Error Code.
- Expected/Observed Client Behavior: Client completes normally as an `END_STREAM` from the server was seen after completeing the sending of the Request Body. This means the `RST_STREAM` frame can be ignored.
- Server sends a `RST_STREAM` with an arbitrary Error Code, followed by an empty `DATA_FRAME` with an arbitrary Error Code set.
- Expected/Observed Client Behavior: Client completes exceptionally as a `RST_STREAM` was seen before the stream was closed (for example with an empty `DATA_FRAME` with the `END_STREAM` flag set).
**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, an `END_HEADERS` flag set and an incorrectly set `END_STREAM` flag set.
- Expected/Observed Client Behavior: Client sends no request body, completes exceptionally with a `PROTOCOL_ERROR`.
-------------
Commit messages:
- 8309118: Removed unused try-with-resources
- 8309118: Improve comments and annotations
- 8309118: Remove local test timeout value
- 8309118: Add more tests for 100 ExpectContinue with HTTP/2
Changes: https://git.openjdk.org/jdk/pull/15664/files
Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=15664&range=00
Issue: https://bugs.openjdk.org/browse/JDK-8309118
Stats: 517 lines in 5 files changed: 414 ins; 59 del; 44 mod
Patch: https://git.openjdk.org/jdk/pull/15664.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/15664/head:pull/15664
PR: https://git.openjdk.org/jdk/pull/15664
More information about the net-dev
mailing list