Add testing to demonstrate nghttp2 and oghttp2 handling of data on a closed stream.
This CL adds an OgHttp2 and corresponding NgHttp2 unit test that exercises the
following scenario:
1. Client sends request to open a stream.
2. Client sends RST_STREAM on the stream it just opened.
3. Client receives response HEADERS + DATA on the stream.
This scenario can validly arise when the server sends the response between
the receipt of Steps 1 and 2.
NgHttp2 does not fully process/deliver callbacks for the closed stream, whereas
OgHttp2 does. It is hypothesized that this difference in behavior is causing a
test failure in
Http2FloodMitigationTest.UpstreamRstStreamStormOnDownstreamCloseRegressionTest
as explained in http://b/201799377#comment8.
PiperOrigin-RevId: 410124780
diff --git a/http2/adapter/nghttp2_adapter_test.cc b/http2/adapter/nghttp2_adapter_test.cc
index 8cb3c84..599a529 100644
--- a/http2/adapter/nghttp2_adapter_test.cc
+++ b/http2/adapter/nghttp2_adapter_test.cc
@@ -1261,6 +1261,92 @@
EXPECT_FALSE(adapter->want_write());
}
+TEST(NgHttp2AdapterTest, ClientReceivesDataOnClosedStream) {
+ DataSavingVisitor visitor;
+ auto adapter = NgHttp2Adapter::CreateClientAdapter(visitor);
+ int result = adapter->Send();
+ EXPECT_EQ(0, result);
+ // Client preface does not appear to include the mandatory SETTINGS frame.
+ EXPECT_THAT(visitor.data(),
+ testing::StrEq(spdy::kHttp2ConnectionHeaderPrefix));
+ visitor.Clear();
+
+ const std::string initial_frames =
+ TestFrameSequence().ServerPreface().Serialize();
+ testing::InSequence s;
+
+ // Server preface (empty SETTINGS)
+ EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0));
+ EXPECT_CALL(visitor, OnSettingsStart());
+ EXPECT_CALL(visitor, OnSettingsEnd());
+
+ const int64_t initial_result = adapter->ProcessBytes(initial_frames);
+ EXPECT_EQ(initial_frames.size(), initial_result);
+
+ // Client SETTINGS ack
+ EXPECT_TRUE(adapter->want_write());
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, 0, 0x1));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, 0, 0x1, 0));
+
+ result = adapter->Send();
+ EXPECT_EQ(0, result);
+ EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS}));
+ visitor.Clear();
+
+ // Let the client open a stream with a request.
+ int stream_id =
+ adapter->SubmitRequest(ToHeaders({{":method", "GET"},
+ {":scheme", "http"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/one"}}),
+ nullptr, nullptr);
+ EXPECT_GT(stream_id, 0);
+
+ EXPECT_TRUE(adapter->want_write());
+ EXPECT_CALL(visitor, OnBeforeFrameSent(HEADERS, stream_id, _, 0x5));
+ EXPECT_CALL(visitor, OnFrameSent(HEADERS, stream_id, _, 0x5, 0));
+
+ result = adapter->Send();
+ EXPECT_EQ(0, result);
+ EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::HEADERS}));
+ visitor.Clear();
+
+ // Let the client RST_STREAM the stream it opened.
+ adapter->SubmitRst(stream_id, Http2ErrorCode::CANCEL);
+ EXPECT_TRUE(adapter->want_write());
+ EXPECT_CALL(visitor, OnBeforeFrameSent(RST_STREAM, stream_id, _, 0x0));
+ EXPECT_CALL(visitor, OnFrameSent(RST_STREAM, stream_id, _, 0x0,
+ static_cast<int>(Http2ErrorCode::CANCEL)));
+ EXPECT_CALL(visitor, OnCloseStream(stream_id, Http2ErrorCode::CANCEL));
+
+ result = adapter->Send();
+ EXPECT_EQ(0, result);
+ EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::RST_STREAM}));
+ visitor.Clear();
+
+ // Let the server send a response on the stream. (It might not have received
+ // the RST_STREAM yet.)
+ const std::string response_frames =
+ TestFrameSequence()
+ .Headers(stream_id,
+ {{":status", "200"},
+ {"server", "my-fake-server"},
+ {"date", "Tue, 6 Apr 2021 12:54:01 GMT"}},
+ /*fin=*/false)
+ .Data(stream_id, "This is the response body.", /*fin=*/true)
+ .Serialize();
+
+ // The visitor gets notified about the HEADERS frame but not the DATA frame on
+ // the closed stream. No further processing for either frame occurs.
+ EXPECT_CALL(visitor, OnFrameHeader(stream_id, _, HEADERS, 0x4));
+ EXPECT_CALL(visitor, OnFrameHeader(stream_id, _, DATA, _)).Times(0);
+
+ const int64_t response_result = adapter->ProcessBytes(response_frames);
+ EXPECT_EQ(response_frames.size(), response_result);
+
+ EXPECT_FALSE(adapter->want_write());
+}
+
TEST(NgHttp2AdapterTest, SubmitMetadata) {
DataSavingVisitor visitor;
auto adapter = NgHttp2Adapter::CreateClientAdapter(visitor);
diff --git a/http2/adapter/oghttp2_adapter_test.cc b/http2/adapter/oghttp2_adapter_test.cc
index 78e3f94..88595fc 100644
--- a/http2/adapter/oghttp2_adapter_test.cc
+++ b/http2/adapter/oghttp2_adapter_test.cc
@@ -1170,6 +1170,107 @@
EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS}));
}
+TEST(OgHttp2AdapterClientTest, ClientReceivesDataOnClosedStream) {
+ DataSavingVisitor visitor;
+ OgHttp2Adapter::Options options{.perspective = Perspective::kClient};
+ auto adapter = OgHttp2Adapter::Create(visitor, options);
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x0));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x0, 0));
+ int result = adapter->Send();
+ EXPECT_EQ(0, result);
+ absl::string_view data = visitor.data();
+ EXPECT_THAT(data, testing::StartsWith(spdy::kHttp2ConnectionHeaderPrefix));
+ data.remove_prefix(strlen(spdy::kHttp2ConnectionHeaderPrefix));
+ EXPECT_THAT(data, EqualsFrames({spdy::SpdyFrameType::SETTINGS}));
+ visitor.Clear();
+
+ const std::string initial_frames =
+ TestFrameSequence().ServerPreface().Serialize();
+ testing::InSequence s;
+
+ // Server preface (empty SETTINGS)
+ EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0));
+ EXPECT_CALL(visitor, OnSettingsStart());
+ EXPECT_CALL(visitor, OnSettingsEnd());
+
+ const int64_t initial_result = adapter->ProcessBytes(initial_frames);
+ EXPECT_EQ(initial_frames.size(), static_cast<size_t>(initial_result));
+
+ // Client SETTINGS ack
+ EXPECT_TRUE(adapter->want_write());
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, 0, 0x1));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, 0, 0x1, 0));
+
+ result = adapter->Send();
+ EXPECT_EQ(0, result);
+ EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS}));
+ visitor.Clear();
+
+ // Let the client open a stream with a request.
+ int stream_id =
+ adapter->SubmitRequest(ToHeaders({{":method", "GET"},
+ {":scheme", "http"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/one"}}),
+ nullptr, nullptr);
+ EXPECT_GT(stream_id, 0);
+
+ EXPECT_TRUE(adapter->want_write());
+ EXPECT_CALL(visitor, OnBeforeFrameSent(HEADERS, stream_id, _, 0x5));
+ EXPECT_CALL(visitor, OnFrameSent(HEADERS, stream_id, _, 0x5, 0));
+
+ result = adapter->Send();
+ EXPECT_EQ(0, result);
+ EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::HEADERS}));
+ visitor.Clear();
+
+ // Let the client RST_STREAM the stream it opened.
+ adapter->SubmitRst(stream_id, Http2ErrorCode::CANCEL);
+ EXPECT_TRUE(adapter->want_write());
+ EXPECT_CALL(visitor, OnBeforeFrameSent(RST_STREAM, stream_id, _, 0x0));
+ EXPECT_CALL(visitor, OnFrameSent(RST_STREAM, stream_id, _, 0x0,
+ static_cast<int>(Http2ErrorCode::CANCEL)));
+ EXPECT_CALL(visitor, OnCloseStream(stream_id, Http2ErrorCode::NO_ERROR));
+
+ result = adapter->Send();
+ EXPECT_EQ(0, result);
+ EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::RST_STREAM}));
+ visitor.Clear();
+
+ // Let the server send a response on the stream. (It might not have received
+ // the RST_STREAM yet.)
+ const std::string response_frames =
+ TestFrameSequence()
+ .Headers(stream_id,
+ {{":status", "200"},
+ {"server", "my-fake-server"},
+ {"date", "Tue, 6 Apr 2021 12:54:01 GMT"}},
+ /*fin=*/false)
+ .Data(stream_id, "This is the response body.", /*fin=*/true)
+ .Serialize();
+
+ // The visitor gets notified about the HEADERS frame and DATA frame for the
+ // closed stream with further processing for both.
+ EXPECT_CALL(visitor, OnFrameHeader(stream_id, _, HEADERS, 0x4));
+ EXPECT_CALL(visitor, OnBeginHeadersForStream(stream_id));
+ EXPECT_CALL(visitor, OnHeaderForStream(stream_id, _, _)).Times(3);
+ EXPECT_CALL(visitor, OnEndHeadersForStream(stream_id));
+ EXPECT_CALL(visitor, OnFrameHeader(stream_id, _, DATA, 0x1));
+ EXPECT_CALL(visitor, OnBeginDataForStream(stream_id, _));
+ EXPECT_CALL(visitor,
+ OnDataForStream(stream_id, "This is the response body."));
+ EXPECT_CALL(visitor, OnEndStream(stream_id));
+ // This final OnCloseStream() is avoided because of a stream map check in
+ // OnEndStream().
+ EXPECT_CALL(visitor, OnCloseStream(stream_id, _)).Times(0);
+
+ const int64_t response_result = adapter->ProcessBytes(response_frames);
+ EXPECT_EQ(response_frames.size(), static_cast<size_t>(response_result));
+
+ EXPECT_FALSE(adapter->want_write());
+}
+
TEST_F(OgHttp2AdapterTest, SubmitMetadata) {
auto source = absl::make_unique<TestMetadataSource>(ToHeaderBlock(ToHeaders(
{{"query-cost", "is too darn high"}, {"secret-sauce", "hollandaise"}})));
