On Tue, 21 Feb 2023 17:31:29 GMT, Conor Cleary <ccle...@openjdk.org> wrote:
> ### Description > Submitter reported when sending data of a length greater than 64kb using the > `HttpClient` with HTTP/2, the HttpClient would hang indefinitely. Through > reproducing the issue and analysing logs it was also seen that the HttpClient > would not accept any incoming frames in this state. Note that 64kb is the > default window size of a HTTP/2 Stream with the `HttpClient`. > > The particular server used by the submitter did not emit window update frames > to increase the size of the client's send window. As it stands now, when the > client detects that sending more data will overflow the send window, it will > await a window update from the remote receiver before attempting to transmit > more data. If no window update comes from the server however, the client will > not process any incoming frames from the server including RST_STREAM frames. > It is important that if a remote indicates to the client that it can no > longer process data with a RST_STREAM with the NO_ERROR flag set that it > close the relevant connection instead of hanging indefinitely. See [section > 8.1 of RFC 7540](https://www.rfc-editor.org/rfc/rfc7540#section-8.1) for > description of this scenario. > > ### Summary of Changes & Justification > The hanging of the client observed when a RST_STREAM is received is caused by > the client not creating a responseSubscriber due to the response being seen > as incomplete. The reason this happens is because in this case where the > client cannot send more data due to the window being full, the request body > is only partially finished sending. A responseSubscriber for reading the > response from the server (i.e. any incoming frames) is only initialised when > the request body has completed sending. However, this will not occur as the > client is waiting for window updates. > > If a RST_STREAM with NO_ERROR is observed, the request body will be completed > without sending all of its data. This will initialise the responseSubcriber > for the request and enable the client to process all incoming data from the > client before subsequently processing the afforemntioned RST_STREAM frame. > > --------- > ### Progress > - [ ] Change must be properly reviewed (1 review required, with at least 1 > [Reviewer](https://openjdk.org/bylaws#reviewer)) > - [x] Change must not contain extraneous whitespace > - [x] Commit message must refer to an issue > > > > ### Reviewing > <details><summary>Using <code>git</code></summary> > > Checkout this PR locally: \ > `$ git fetch https://git.openjdk.org/jdk pull/12694/head:pull/12694` \ > `$ git checkout pull/12694` > > Update a local copy of the PR: \ > `$ git checkout pull/12694` \ > `$ git pull https://git.openjdk.org/jdk pull/12694/head` > > </details> > <details><summary>Using Skara CLI tools</summary> > > Checkout this PR locally: \ > `$ git pr checkout 12694` > > View PR using the GUI difftool: \ > `$ git pr show -t 12694` > > </details> > <details><summary>Using diff file</summary> > > Download this PR as a diff file: \ > <a > href="https://git.openjdk.org/jdk/pull/12694.diff">https://git.openjdk.org/jdk/pull/12694.diff</a> > > </details> test/jdk/java/net/httpclient/http2/PostPutTest.java line 109: > 107: } > 108: > 109: static class PostPutExchange extends Http2TestExchangeImpl { Can we make the HTTP2 server always reset the stream if the configured Http2Handler doesn't consume the entire input? There's even a TODO for that: https://github.com/openjdk/jdk/blob/master/test/jdk/java/net/httpclient/lib/jdk/httpclient/test/lib/http2/BodyInputStream.java#L133 ------------- PR: https://git.openjdk.org/jdk/pull/12694