Skip to content

[http2] Gracefully receive headers on canceled streams #1800

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions pkgs/http2/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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`.
Expand Down
12 changes: 6 additions & 6 deletions pkgs/http2/lib/src/streams/stream_handler.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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()) {
Expand Down
2 changes: 1 addition & 1 deletion pkgs/http2/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -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

Expand Down
133 changes: 133 additions & 0 deletions pkgs/http2/test/client_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down