On Thu, 15 Oct 2026 12:36:07 GMT, Daniel Fuchs <[email protected]> wrote:
> This fixes a regression introduced by > [JDK-8317808](https://bugs.openjdk.org/browse/JDK-8317808). If the server > sends all the response bytes, sends a GOAWAY, and immediately closes the > connection since all streams are closed on its side, the client may fail to > complete the request properly although all bytes have been received. > Upon noticing that the connection is closed the client will interrupt all > exchanges, even those for which all bytes have been received (but not yet > pulled). > > This typically happens when the response body is delivered as an InputStream. > > The same issue also happens with HTTP/3. > > For HTTP/2 we will allow the BodySubscriber to continue pulling the bytes, > unless we notice that the bodyCF is completed exceptionally. > > For HTTP/3 the logic is a bit different: when the connection is closed it > forcefully terminates all exchanges. We will skip those exchange for which > the underlying QUIC receiving stream state is DATA_READ or DATA_RCVD (and > which are not blocked by QPack) > > Testing: > > A new scenario is added to > test/jdk/java/net/httpclient/http2/H2GoAwayPromptConnectionClose.java to test > the HTTP/2 changes. A new test is added for HTTP/3. > > Both tests fail without the fix and pass with it. Failure without the fix is > not 100% guaranteed, but my experiments saw the tests reliably failing. > > --------- > - [x] I confirm that I make this contribution in accordance with the [OpenJDK > Interim AI Policy](https://openjdk.org/legal/ai). src/java.net.http/share/classes/jdk/internal/net/http/Http3Connection.java line 680: > 678: // Figure out whether this is an orderly close. > 679: boolean orderlyClose = tc.isAppLayer() && tc.getCloseCode() == > H3_NO_ERROR.code(); > 680: if (orderlyClose) { IMO we shouldn't discard fully received responses even if the connection was closed because of an error. It may result in different requests getting different errors, but I think it's still better than throwing an error on an exchange that completed correctly otherwise. src/java.net.http/share/classes/jdk/internal/net/http/Http3Connection.java line 738: > 736: // makes sure it gets cancelled even if it has buffered > 737: // all data, as processing buffered data can still trigger > 738: // malformed frame errors processing buffered data can, but doesn't have to, trigger errors. Why would we cancel it without checking? src/java.net.http/share/classes/jdk/internal/net/http/Http3ExchangeImpl.java line 1755: > 1753: if (headerFrameReader.isBlocked()) return false; > 1754: > 1755: var state = stream.receivingState(); `receivingState` should have been internal to QUIC. Can I convince you to not use it in HTTP3 code? src/java.net.http/share/classes/jdk/internal/net/http/Stream.java line 219: > 217: // there is no point in pulling the data. > 218: // if bodyCf is null just stop here; this is not > expected. > 219: if (bodyCf == null || bodyCf.isCancelled() || > bodyCf.isCompletedExceptionally()) break; Can you add a comment stating when we actually want to continue the loop despite errorRef being non-null? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/33385#discussion_r4314453953 PR Review Comment: https://git.openjdk.org/jdk/pull/33385#discussion_r4314519070 PR Review Comment: https://git.openjdk.org/jdk/pull/33385#discussion_r4316402533 PR Review Comment: https://git.openjdk.org/jdk/pull/33385#discussion_r4316481967
