diff --git a/pkgs/http2/CHANGELOG.md b/pkgs/http2/CHANGELOG.md index b98deeeefa..3beb80636d 100644 --- a/pkgs/http2/CHANGELOG.md +++ b/pkgs/http2/CHANGELOG.md @@ -1,6 +1,7 @@ ## 3.0.1-wip - Gracefully handle receiving headers on a stream that the client has canceled. (#1799) +- Treat incoming server push streams as connection protocol error when pushes are disabled (SETTINGS_ENABLE_PUSH=0). ## 3.0.0 diff --git a/pkgs/http2/lib/src/streams/stream_handler.dart b/pkgs/http2/lib/src/streams/stream_handler.dart index 7db1310658..0826674593 100644 --- a/pkgs/http2/lib/src/streams/stream_handler.dart +++ b/pkgs/http2/lib/src/streams/stream_handler.dart @@ -745,6 +745,15 @@ class StreamHandler extends Object with TerminatableMixin, ClosableMixin { throw ProtocolException('Expected open state (was: ${stream.state}).'); } + // RFC 7540 6.5.2/8.2: An endpoint that has both sent and received + // acknowledgement of SETTINGS_ENABLE_PUSH=0 MUST treat receipt of a + // PUSH_PROMISE frame as a connection error of type PROTOCOL_ERROR. + if (!_localSettings.enablePush) { + throw ProtocolException( + 'Received PUSH_PROMISE although SETTINGS_ENABLE_PUSH is 0.', + ); + } + var pushedStream = newRemoteStream(frame.promisedStreamId); _changeState(pushedStream, StreamState.ReservedRemote); diff --git a/pkgs/http2/test/client_test.dart b/pkgs/http2/test/client_test.dart index 9f1a98a50d..41b6a4faf8 100644 --- a/pkgs/http2/test/client_test.dart +++ b/pkgs/http2/test/client_test.dart @@ -883,6 +883,100 @@ void main() { await Future.wait([serverFun(), clientFun()]); }); + clientTest( + 'client-reports-connection-error-on-push-when-push-disabled', + ( + ClientTransportConnection client, + FrameWriter serverWriter, + StreamIterator serverReader, + Future Function() nextFrame, + ) async { + var handshakeCompleter = Completer(); + var gotPushPromise = Completer(); + + Future serverFun() async { + // Server accepts the settings with SETTINGS_ENABLE_PUSH = 0. + serverWriter.writeSettingsFrame([]); + expect(await nextFrame(), isA()); + serverWriter.writeSettingsAckFrame(); + expect(await nextFrame(), isA()); + + handshakeCompleter.complete(); + + var headers = await nextFrame() as HeadersFrame; + var streamId = headers.header.streamId; + + // Push stream on the open stream. + var pushStreamId = 2; + serverWriter.writePushPromiseFrame(streamId, pushStreamId, [ + Header.ascii('a', 'b'), + ]); + + // Since the client sent SETTINGS_ENABLE_PUSH=0, receiving a + // PUSH_PROMISE **must** result in a connection error of type + // PROTOCOL_ERROR. + try { + var frame = await nextFrame().timeout( + const Duration(milliseconds: 500), + ); + if (frame is GoawayFrame) { + expect(frame.errorCode, ErrorCode.PROTOCOL_ERROR); + expect( + ascii.decode(frame.debugData), + contains( + 'Received PUSH_PROMISE although SETTINGS_ENABLE_PUSH is 0', + ), + ); + } else { + fail('Expected GoawayFrame, but got $frame'); + } + } finally { + expect(await serverReader.moveNext(), false); + await serverWriter.close(); + } + } + + Future clientFun() async { + await handshakeCompleter.future; + + var stream = client.makeRequest([Header.ascii('a', 'b')]); + + // Listen for push stream, which should not happen if pushes are + // disabled. + stream.peerPushes.listen((push) { + gotPushPromise.complete(); + }); + + // Wait for the stream to throw an error. + var done = Completer(); + stream.incomingMessages.listen( + (_) {}, + onError: (Object e) { + expect( + e, + isA().having( + (p0) => p0.message, + 'Forcefully terminated message', + contains('Connection is being forcefully terminated'), + ), + ); + if (!done.isCompleted) done.complete(); + }, + onDone: () { + if (!done.isCompleted) done.complete(); + }, + ); + + await Future.any([gotPushPromise.future, done.future]); + + await client.terminate(); + } + + await Future.wait([serverFun(), clientFun()]); + }, + settings: const ClientSettings(allowServerPushes: false), + ); + clientTest('client-reports-flowcontrol-error-on-negative-window', ( ClientTransportConnection client, FrameWriter serverWriter, @@ -1095,10 +1189,11 @@ void clientTest( StreamIterator frameReader, Future Function() readNext, ) - func, -) { + func, { + ClientSettings? settings, +}) { return test(name, () { - var streams = ClientStreams(); + var streams = ClientStreams(settings: settings); var serverReader = streams.serverConnectionFrameReader; Future readNext() async { @@ -1116,8 +1211,12 @@ void clientTest( } class ClientStreams { + final ClientSettings? settings; final StreamController> writeA = StreamController(); final StreamController> writeB = StreamController(); + + ClientStreams({this.settings}); + Stream> get readA => writeA.stream; Stream> get readB => writeB.stream; @@ -1136,5 +1235,5 @@ class ClientStreams { } ClientTransportConnection get clientConnection => - ClientTransportConnection.viaStreams(readB, writeA); + ClientTransportConnection.viaStreams(readB, writeA, settings: settings); }