On Thu, 24 Mar 2022 12:41:26 GMT, Conor Cleary <[email protected]> wrote:
>> **Problem**
>> When a Continuation Frame is received by the httpclient using HTTP/2 after a
>> Push Promise frame (can happen if the amount of headers to be sent in a
>> single Push Promise frame exceeds the maximum frame size, so a Continuation
>> frame is required), the following exception occurs:
>>
>>
>> java.io.IOException: no statuscode in response
>> at
>> java.net.http/jdk.internal.net.http.HttpClientImpl.send(HttpClientImpl.java:565)
>> at
>> java.net.http/jdk.internal.net.http.HttpClientFacade.send(HttpClientFacade.java:119)
>> ...
>>
>> This exception occurs because there is no existing flow in
>> `jdk/internal/net/http/Http2Connection.java` which accounts for the case
>> where a PushPromiseFrame is received with the END_HEADERS flag set to 0x0.
>> When this occurs, the only acceptable frame/s (as multiple continuations are
>> also acceptable) that can be received by the client on the same stream is a
>> continuation frame.
>>
>> **Fix**
>> To ensure correct behavior, the following changes were made to
>> `jdk/internal/net/http/Http2Connection.java`.
>>
>> - The existing method `handlePushPromise()` was modified so that if the
>> END_HEADERS flag is _unset_ (flags equal to 0x0), then a record used to
>> track the state of the Push Promise containing a shared `HeaderDecoder` and
>> the received `PushPromiseFrame` is initialised.
>> - When the subsequent `ContinuationFrame` is received in `processFrame()`,
>> the method `handlePushContinuation()` is called instead of the default flow
>> resulting in `stream.incoming(frame)` being called (the source of the
>> incorrect behaviour originally).
>> - In `handlePushContinuation()`, the shared decoder is used to decode the
>> received `ContinuationFrame` headers and if the `END_HEADERS` flag is set
>> (flags equal to 0x4), the `HttpHeaders` object for the Push Promise as a
>> whole is constructed which serves to combine the headers from both the
>> `PushPromiseFrame` and the `ContinuationFrame`.
>>
>> A regression test was included which verifies that the exception is not
>> thrown and that the headers arrive correctly.
>
> Conor Cleary has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - 8263031: further verification of push promise and response
> - 8263031: Removed duplicate import
src/java.net.http/share/classes/jdk/internal/net/http/Http2Connection.java line
807:
> 805: } catch (UncheckedIOException e) {
> 806: debug.log("Error handling Push Promise with
> Continuation: " + e.getMessage(), e);
> 807: protocolError(ResetFrame.PROTOCOL_ERROR,
> e.getMessage());
I believe it would more clear to use `ErrorFrame.PROTOCOL_ERROR` or
`GoAwayFrame.PROTOCOL_ERRROR` here and in the other calls to protocolError
below, since we're not resetting the stream here but closing the whole
connection with a `GoAwayFrame`.
-------------
PR: https://git.openjdk.java.net/jdk/pull/7696
