RFR: 8296410: HttpClient throws java.io.IOException: no statuscode in response for HTTP2

[Conor Cleary]

On Wed, 18 Jan 2023 11:47:06 GMT, Conor Cleary <ccleary at openjdk.org> wrote:

> The changes in this PR look good to me. While looking at the source code, I noticed that the `Stream` class has a `PushedStream` nested class which too has a similar implementation in `handleResponse()`. I haven't yet checked the spec related to this; do you know if this code too will need a similar fix?

The original situation for a regular request-response in http2 was caused by a server responding to a client request with response headers that included only a code 200 status with no flags set and the related data. This was then followed by trailing headers with both the END_STREAM and END_HEADERS flags set with an empty header block (though the contents of the block aren't of concern here).

Now, the Push Promise version of this situation is a bit different. A Push Promise frame is sent to the client containing all expected details such as the promisedStreamId, request path etc. However if we try mirror the previous situation by _not_ including an END_HEADERS flag in the Push Promise frame and attempting to send a trailing HEADERS frame, the HttpClient correcly throws a protocol error specifying that the client `Expected a Continuation frame but received HEADERS`. This is behavior defined in [section 6.6. of RFC7540: HTTP/2](https://www.rfc-editor.org/rfc/rfc7540#section-6.6).


The PUSH_PROMISE frame defines the following flags:

END_HEADERS (0x4):  When set, bit 2 indicates that this
frame contains an entire header block (Section 4.3) and
is not followed by any CONTINUATION frames.

A PUSH_PROMISE frame without the END_HEADERS flag
set MUST be followed by a CONTINUATION frame for the
same stream.  A receiver MUST treat the receipt of any 
other type of frame or a frame on a different stream as a 
connection error (Section 5.4.1) of type PROTOCOL_ERROR.

      
With this in mind, I would argue that in the case of `PushedStream::handleResponse` no changes are required as the behavior is quite well defined and strict when it comes to testing what the HttpClient will allow. One interesting side note is  that END_STREAM is not strictly speaking a defined flag for PUSH_PROMISE frames.

Interested to see what reviewers have to say about that analysis or if there are any mistakes in my thinking, it was quite a tricky one to wrap my head around given the differences with how Push Requests are treated by the client.

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

PR: https://git.openjdk.org/jdk/pull/12028