On Mon, 19 Oct 2026 13:42:05 GMT, Daniel Jeliński <[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 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? We reach here when processing the data from Exchange E coming from Stream N has caused an error - in which case - we don't want to continue processing it. Typical scenario, we received connection close. The quic connection gets close, but for stream N, we have received all the data. So we continue processing stream N. Later while processing buffered data for Stream N, we run into a malformed frame, which causes this method to be called. In that case we want to cancel the exchange E associated with Stream N, because although all the data was received, it is not correct. This is what we're doing here. > 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? No ;-) > 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? As long as we haven't processed all the buffered data? This is HTTP/2 so it is impossible to know whether you have received all the data before processing it. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/33385#discussion_r4316759718 PR Review Comment: https://git.openjdk.org/jdk/pull/33385#discussion_r4316770922 PR Review Comment: https://git.openjdk.org/jdk/pull/33385#discussion_r4316784275
