From bfecfc490557b05eb2a92061e585bdb0c0ab951d Mon Sep 17 00:00:00 2001 From: Willow Chargin Date: Sat, 26 Jul 2025 12:03:04 -0700 Subject: [PATCH 1/2] [http2] Gracefully receive headers on canceled streams If a client opens a stream and then cancels it, the server may have already sent response frames back. The HTTP/2 spec says that the client MUST be prepared to receive any such frames. We already handle this correctly for data frames, but prior to this patch, header frames on a canceled stream would cause the whole connection to be torn down. This patch fixes that. Fixes #1799. Test Plan: Unit tests included. Before this change, the new test would fail with "Connection is being forcefully terminated. (errorCode: 1)". wchargin-branch: http2-headers-after-cancel wchargin-source: 428af2c7cf2fc9f18884558e2b471d2d29a05998 --- .../http2/lib/src/streams/stream_handler.dart | 12 +- pkgs/http2/test/client_test.dart | 133 ++++++++++++++++++ 2 files changed, 139 insertions(+), 6 deletions(-) diff --git a/pkgs/http2/lib/src/streams/stream_handler.dart b/pkgs/http2/lib/src/streams/stream_handler.dart index 92d734498a..7db1310658 100644 --- a/pkgs/http2/lib/src/streams/stream_handler.dart +++ b/pkgs/http2/lib/src/streams/stream_handler.dart @@ -613,12 +613,12 @@ class StreamHandler extends Object with TerminatableMixin, ClosableMixin { _handleHeadersFrame(newStream, frame); _newStreamsC.add(newStream); } else { - // A server cannot open new streams to the client. The only way - // for a server to start a new stream is via a PUSH_PROMISE_FRAME. - throw ProtocolException( - 'HTTP/2 clients cannot receive HEADER_FRAMEs as a connection' - 'attempt.', - ); + // We must be able to receive header frames for streams that have + // already been closed. This can occur if we send RST_STREAM while + // the server already had header frames in flight. + // + // Still respond with an error, as the stream is closed. + throw _throwStreamClosedException(frame.header.streamId); } } else if (frame is WindowUpdateFrame) { if (frameBelongsToIdleStream()) { diff --git a/pkgs/http2/test/client_test.dart b/pkgs/http2/test/client_test.dart index 35a6392a77..9f1a98a50d 100644 --- a/pkgs/http2/test/client_test.dart +++ b/pkgs/http2/test/client_test.dart @@ -426,6 +426,139 @@ void main() { await Future.wait([serverFun(), clientFun()]); }); + clientTest('header-frame-received-after-stream-cancel', ( + ClientTransportConnection client, + FrameWriter serverWriter, + StreamIterator serverReader, + Future Function() nextFrame, + ) async { + var handshakeCompleter = Completer(); + var serverReceivedHeaders = Completer(); + var cancelDone = Completer(); + + Future serverFun() async { + serverWriter.writeSettingsFrame([]); + expect(await nextFrame(), isA()); + serverWriter.writeSettingsAckFrame(); + expect(await nextFrame(), isA()); + + handshakeCompleter.complete(); + + var headers1 = await nextFrame() as HeadersFrame; + var streamId1 = headers1.header.streamId; + expect( + await nextFrame(), + isA().having( + (p0) => p0.hasEndStreamFlag, + 'Last data frame', + true, + ), + ); + + var headers2 = await nextFrame() as HeadersFrame; + var streamId2 = headers2.header.streamId; + expect( + await nextFrame(), + isA().having( + (p0) => p0.hasEndStreamFlag, + 'Last data frame', + true, + ), + ); + + serverReceivedHeaders.complete(); + await cancelDone.future; + expect( + await nextFrame(), + isA().having( + (f) => f.header.streamId, + 'header.streamId', + streamId1, + ), + ); + + // Client has canceled, but we already had a response going out over + // the wire... + serverWriter.writeHeadersFrame(streamId1, [Header.ascii('e', 'f')]); + // Client will send an extra [RstStreamFrame] in response to this + // unexpected header. That's not required, but it is current + // behavior, so advance past it. + expect( + await nextFrame(), + isA().having( + (f) => f.header.streamId, + 'header.streamId', + streamId1, + ), + ); + + // Respond on the second stream. + var data2 = [43]; + serverWriter.writeDataFrame(streamId2, data2, endStream: true); + serverWriter.writeRstStreamFrame(streamId2, ErrorCode.STREAM_CLOSED); + + // The two WindowUpdateFrames for the data2 DataFrame. + expect( + await nextFrame(), + isA().having( + (p0) => p0.header.streamId, + 'Stream update', + streamId2, + ), + ); + expect( + await nextFrame(), + isA().having( + (p0) => p0.header.streamId, + 'Connection update', + 0, + ), + ); + + expect(await nextFrame(), isA()); + expect(await serverReader.moveNext(), false); + + await serverWriter.close(); + } + + Future clientFun() async { + await handshakeCompleter.future; + + // First stream: we'll send data and then cancel quickly, but the + // server will already have a response in flight. + var stream1 = client.makeRequest([Header.ascii('a', 'b')]); + await stream1.outgoingMessages.close(); + + // Second stream: server will respond only after we've canceled the + // first stream. + var stream2 = client.makeRequest([Header.ascii('c', 'd')]); + await stream2.outgoingMessages.close(); + + await serverReceivedHeaders.future; + stream1.terminate(); + cancelDone.complete(); + + var messages2 = await stream2.incomingMessages.toList(); + expect(messages2, hasLength(1)); + var message2 = messages2[0]; + expect( + message2, + isA().having( + (p0) => p0.bytes, + 'Same as `data2` above', + [43], + ), + ); + + expect(client.isOpen, true); + var future = client.finish(); + expect(client.isOpen, false); + await future; + } + + await Future.wait([serverFun(), clientFun()]); + }); + clientTest('data-frame-received-after-stream-cancel', ( ClientTransportConnection client, FrameWriter serverWriter, From 0bed60a9116826aa0883fa494df5cc1a446bd581 Mon Sep 17 00:00:00 2001 From: Willow Chargin Date: Sat, 26 Jul 2025 12:08:29 -0700 Subject: [PATCH 2/2] [http2-headers-after-cancel: update changelog and pubspec] wchargin-branch: http2-headers-after-cancel wchargin-source: 8aed54b8d5bcc0450db8f46ab08a8012f4de1bd8 --- pkgs/http2/CHANGELOG.md | 4 ++++ pkgs/http2/pubspec.yaml | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/pkgs/http2/CHANGELOG.md b/pkgs/http2/CHANGELOG.md index eb28862663..b98deeeefa 100644 --- a/pkgs/http2/CHANGELOG.md +++ b/pkgs/http2/CHANGELOG.md @@ -1,3 +1,7 @@ +## 3.0.1-wip + +- Gracefully handle receiving headers on a stream that the client has canceled. (#1799) + ## 3.0.0 - Require Dart SDK `3.7.0`. diff --git a/pkgs/http2/pubspec.yaml b/pkgs/http2/pubspec.yaml index 64ac728daa..e65f0a31a4 100644 --- a/pkgs/http2/pubspec.yaml +++ b/pkgs/http2/pubspec.yaml @@ -1,5 +1,5 @@ name: http2 -version: 3.0.0 +version: 3.0.1-wip description: A HTTP/2 implementation in Dart. repository: https://github.com/dart-lang/http/tree/master/pkgs/http2