[http2] Gracefully receive headers on canceled streams (#1800)

* [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-headers-after-cancel: update changelog and pubspec]

wchargin-branch: http2-headers-after-cancel
wchargin-source: 8aed54b8d5bcc0450db8f46ab08a8012f4de1bd8

Author: wchargin

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`.

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()) {

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
 

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<Frame> serverReader,
+        Future<Frame> Function() nextFrame,
+      ) async {
+        var handshakeCompleter = Completer<void>();
+        var serverReceivedHeaders = Completer<void>();
+        var cancelDone = Completer<void>();
+
+        Future serverFun() async {
+          serverWriter.writeSettingsFrame([]);
+          expect(await nextFrame(), isA<SettingsFrame>());
+          serverWriter.writeSettingsAckFrame();
+          expect(await nextFrame(), isA<SettingsFrame>());
+
+          handshakeCompleter.complete();
+
+          var headers1 = await nextFrame() as HeadersFrame;
+          var streamId1 = headers1.header.streamId;
+          expect(
+            await nextFrame(),
+            isA<DataFrame>().having(
+              (p0) => p0.hasEndStreamFlag,
+              'Last data frame',
+              true,
+            ),
+          );
+
+          var headers2 = await nextFrame() as HeadersFrame;
+          var streamId2 = headers2.header.streamId;
+          expect(
+            await nextFrame(),
+            isA<DataFrame>().having(
+              (p0) => p0.hasEndStreamFlag,
+              'Last data frame',
+              true,
+            ),
+          );
+
+          serverReceivedHeaders.complete();
+          await cancelDone.future;
+          expect(
+            await nextFrame(),
+            isA<RstStreamFrame>().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<RstStreamFrame>().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<WindowUpdateFrame>().having(
+              (p0) => p0.header.streamId,
+              'Stream update',
+              streamId2,
+            ),
+          );
+          expect(
+            await nextFrame(),
+            isA<WindowUpdateFrame>().having(
+              (p0) => p0.header.streamId,
+              'Connection update',
+              0,
+            ),
+          );
+
+          expect(await nextFrame(), isA<GoawayFrame>());
+          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<DataStreamMessage>().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,