diff --git a/pkgs/http2/CHANGELOG.md b/pkgs/http2/CHANGELOG.md index 2ac83f0378..4620a000ad 100644 --- a/pkgs/http2/CHANGELOG.md +++ b/pkgs/http2/CHANGELOG.md @@ -11,6 +11,9 @@ - Replenish the connection-level receive window and release active stream slots when a stream with buffered unconsumed `DATA` frames is terminated, reset, or its incoming subscription is cancelled. +- Ignore `PRIORITY` and unknown extension frames on open, idle, and closed + streams, and treat `SETTINGS`, `PING`, and `GOAWAY` frames with a non-zero + stream ID as a connection-level `PROTOCOL_ERROR`. ## 3.1.0 diff --git a/pkgs/http2/lib/src/connection.dart b/pkgs/http2/lib/src/connection.dart index 5a658ce608..5e07f7fb13 100644 --- a/pkgs/http2/lib/src/connection.dart +++ b/pkgs/http2/lib/src/connection.dart @@ -477,6 +477,16 @@ abstract class Connection { 'Cannot handle frame type ${frame.runtimeType} with stream-id 0.', ); } + } else if (frame is SettingsFrame || + frame is PingFrame || + frame is GoawayFrame) { + // RFC 9113 sections 6.5, 6.7, and 6.8: SETTINGS, PING, and GOAWAY frames + // always apply to the connection as a whole; a non-zero stream identifier + // must be treated as a connection error of type PROTOCOL_ERROR. + throw ProtocolException( + 'Cannot handle frame type ${frame.runtimeType} with non-zero ' + 'stream-id ${frame.header.streamId}.', + ); } else { _streams.processStreamFrame(_state, frame); } diff --git a/pkgs/http2/lib/src/streams/stream_handler.dart b/pkgs/http2/lib/src/streams/stream_handler.dart index 513c87da87..80e9f36615 100644 --- a/pkgs/http2/lib/src/streams/stream_handler.dart +++ b/pkgs/http2/lib/src/streams/stream_handler.dart @@ -745,7 +745,7 @@ class StreamHandler extends Object with TerminatableMixin, ClosableMixin { // unused or closed parent stream. // // As long as we do not handle stream priorities, we can safely ignore - // such frames on idle streams. + // such frames on idle or closed streams. // // NOTE: Firefox for example sends [PriorityFrame]s even without // opening any streams (e.g. streams 3,5,7,9,11 [PriorityFrame]s and @@ -753,6 +753,10 @@ class StreamHandler extends Object with TerminatableMixin, ClosableMixin { // // TODO: When implementing priorities for HTTP/2 streams, these frames // need to be taken into account. + } else if (frame is UnknownFrame) { + // RFC 9113 section 4.1: "Implementations MUST ignore and discard + // frames of unknown types." That includes frames an extension sends + // on streams this end considers idle or closed. } else if (frame is PushPromiseFrame) { throw ProtocolException( 'Cannot push on a non-existent stream ' @@ -794,6 +798,13 @@ class StreamHandler extends Object with TerminatableMixin, ClosableMixin { _handleWindowUpdate(stream, frame); } else if (frame is RstStreamFrame) { _handleRstFrame(stream, frame); + } else if (frame is PriorityFrame) { + // RFC 9113 section 6.3: PRIORITY frames are deprecated and carry + // nothing this end acts on; they can be sent on a stream in any + // state. + } else if (frame is UnknownFrame) { + // RFC 9113 section 4.1: "Implementations MUST ignore and discard + // frames of unknown types." } else { throw ProtocolException( 'Unsupported frame type ${frame.runtimeType}.', diff --git a/pkgs/http2/test/server_test.dart b/pkgs/http2/test/server_test.dart index 060333493f..d0c5a5c5fd 100644 --- a/pkgs/http2/test/server_test.dart +++ b/pkgs/http2/test/server_test.dart @@ -652,6 +652,159 @@ void main() { await Future.wait([serverFun(), clientFun()]); }); + + test( + 'ignores-priority-and-unknown-frames-on-open-and-idle-streams', + () async { + final streams = ClientErrorStreams(); + final server = streams.serverConnection; + final clientReader = streams.clientConnectionFrameReader; + final encoder = HPackEncoder(); + + Future nextFrame() async { + expect(await clientReader.moveNext(), isTrue); + return clientReader.current; + } + + Future serverFun() async { + final it = StreamIterator(server.incomingStreams); + expect(await it.moveNext(), isTrue); + final stream = it.current; + final messages = await stream.incomingMessages.toList(); + expect(messages, hasLength(2)); + expect(messages[0], isA()); + expect(messages[1], isA()); + stream.sendHeaders([ + Header.ascii(':status', '200'), + ], endStream: true); + expect(await it.moveNext(), isFalse); + await server.finish(); + } + + Future clientFun() async { + streams.writeConnectionPreface(); + expect(await nextFrame(), isA()); + streams.writeRawFrame( + type: FrameType.SETTINGS, + flags: SettingsFrame.FLAG_ACK, + streamId: 0, + payload: const [], + ); + streams.writeRawFrame( + type: FrameType.SETTINGS, + flags: 0, + streamId: 0, + payload: const [], + ); + expect(await nextFrame(), isA()); + + streams.writeRawFrame( + type: FrameType.HEADERS, + flags: HeadersFrame.FLAG_END_HEADERS, + streamId: 1, + payload: encoder.encode([Header.ascii('a', 'b')]), + ); + // Send PRIORITY on open stream 1 and unknown frame (0xfa) on open + // stream 1 and idle stream 5. + streams.writeRawFrame( + type: FrameType.PRIORITY, + flags: 0, + streamId: 1, + payload: const [0, 0, 0, 0, 16], + ); + streams.writeRawFrame( + type: 0xfa, + flags: 0, + streamId: 1, + payload: const [1, 2], + ); + streams.writeRawFrame( + type: 0xfa, + flags: 0, + streamId: 5, + payload: const [], + ); + streams.writeRawFrame( + type: FrameType.DATA, + flags: DataFrame.FLAG_END_STREAM, + streamId: 1, + payload: const [42], + ); + + expect(await nextFrame(), isA()); + expect(await nextFrame(), isA()); + expect(await nextFrame(), isA()); + + streams.writeRawFrame( + type: FrameType.GOAWAY, + flags: 0, + streamId: 0, + payload: const [0, 0, 0, 1, 0, 0, 0, 0], + ); + expect(await clientReader.moveNext(), isFalse); + } + + await Future.wait([serverFun(), clientFun()]); + }, + ); + + test('rejects-control-frames-with-nonzero-stream-id', () async { + for (final frameType in [ + FrameType.SETTINGS, + FrameType.PING, + FrameType.GOAWAY, + ]) { + final streams = ClientErrorStreams(); + final server = streams.serverConnection; + final incoming = server.incomingStreams.toList(); + final clientReader = streams.clientConnectionFrameReader; + + Future nextFrame() async { + expect(await clientReader.moveNext(), isTrue); + return clientReader.current; + } + + streams.writeConnectionPreface(); + expect(await nextFrame(), isA()); + streams.writeRawFrame( + type: FrameType.SETTINGS, + flags: SettingsFrame.FLAG_ACK, + streamId: 0, + payload: const [], + ); + streams.writeRawFrame( + type: FrameType.SETTINGS, + flags: 0, + streamId: 0, + payload: const [], + ); + expect(await nextFrame(), isA()); + + final payload = switch (frameType) { + FrameType.SETTINGS => const [], + FrameType.PING => const [0, 0, 0, 0, 0, 0, 0, 1], + FrameType.GOAWAY => const [0, 0, 0, 0, 0, 0, 0, 0], + _ => const [], + }; + streams.writeRawFrame( + type: frameType, + flags: 0, + streamId: 1, + payload: payload, + ); + + expect( + await nextFrame(), + isA().having( + (f) => f.errorCode, + 'errorCode', + ErrorCode.PROTOCOL_ERROR, + ), + ); + expect(await clientReader.moveNext(), isFalse); + expect(await incoming, isEmpty); + } + }); }); group('server-errors', () {