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/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/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 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,