Add oghttp2 and nghttp2 adapter tests for receiving a GOAWAY with active streams.
This CL introduces adapter tests for the following scenario, inspired directly
by the Envoy test added in [1]:
1. Client opens two streams, stream IDs 1 and 3.
2. Server sends RST_STREAM for stream 1.
3. Server sends GOAWAY with last_stream_id 1.
4. Server sends WINDOW_UPDATE.
5. Client should close stream 3.
When receiving a GOAWAY, both nghttp2 [2] and GFE2 [3] close the streams above
the last_stream_id of the GOAWAY. This is a feature gap that oghttp2 needs to
fill. The added tests also show that oghttp2 should avoid invoking the visitor
OnWindowUpdate() for a closed stream (though delivering the common header is
okay/consistent with (o|n)http2 behavior for other frames on closed streams).
[1] http://go/envoy-upstream/pull/20576/files#diff-671fe996ff6537911528a7f54c931b68b93aa199411740ff407b1116ac96a43b
[2] http://google3/third_party/nghttp2/src/lib/nghttp2_session.c;l=4750;rcl=433094660
[3] http://google3/gfe/gfe2/http2/http2_backend_dispatcher.cc;l=642-659;rcl=438051287
PiperOrigin-RevId: 439628963
diff --git a/http2/adapter/nghttp2_adapter_test.cc b/http2/adapter/nghttp2_adapter_test.cc
index 47d085c..4341a24 100644
--- a/http2/adapter/nghttp2_adapter_test.cc
+++ b/http2/adapter/nghttp2_adapter_test.cc
@@ -1547,6 +1547,78 @@
EXPECT_EQ(visitor.data(), spdy::kHttp2ConnectionHeaderPrefix);
}
+TEST(NgHttp2AdapterTest, ClientReceivesGoAway) {
+ DataSavingVisitor visitor;
+ auto adapter = NgHttp2Adapter::CreateClientAdapter(visitor);
+
+ testing::InSequence s;
+
+ const std::vector<Header> headers1 =
+ ToHeaders({{":method", "GET"},
+ {":scheme", "http"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/one"}});
+
+ const int32_t stream_id1 = adapter->SubmitRequest(headers1, nullptr, nullptr);
+ ASSERT_GT(stream_id1, 0);
+
+ const std::vector<Header> headers2 =
+ ToHeaders({{":method", "GET"},
+ {":scheme", "http"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/two"}});
+
+ const int32_t stream_id2 = adapter->SubmitRequest(headers2, nullptr, nullptr);
+ ASSERT_GT(stream_id2, stream_id1);
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(HEADERS, stream_id1, _, 0x5));
+ EXPECT_CALL(visitor, OnFrameSent(HEADERS, stream_id1, _, 0x5, 0));
+ EXPECT_CALL(visitor, OnBeforeFrameSent(HEADERS, stream_id2, _, 0x5));
+ EXPECT_CALL(visitor, OnFrameSent(HEADERS, stream_id2, _, 0x5, 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({SpdyFrameType::HEADERS, SpdyFrameType::HEADERS}));
+ visitor.Clear();
+
+ const std::string stream_frames =
+ TestFrameSequence()
+ .ServerPreface()
+ .RstStream(1, Http2ErrorCode::ENHANCE_YOUR_CALM)
+ .GoAway(1, Http2ErrorCode::INTERNAL_ERROR, "indigestion")
+ .WindowUpdate(0, 42)
+ .WindowUpdate(1, 42)
+ .Serialize();
+
+ EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0));
+ EXPECT_CALL(visitor, OnSettingsStart());
+ EXPECT_CALL(visitor, OnSettingsEnd());
+ EXPECT_CALL(visitor, OnFrameHeader(1, 4, RST_STREAM, 0));
+ EXPECT_CALL(visitor, OnRstStream(1, Http2ErrorCode::ENHANCE_YOUR_CALM));
+ EXPECT_CALL(visitor, OnCloseStream(1, Http2ErrorCode::ENHANCE_YOUR_CALM));
+ EXPECT_CALL(visitor, OnFrameHeader(0, _, GOAWAY, 0));
+ EXPECT_CALL(visitor,
+ OnGoAway(1, Http2ErrorCode::INTERNAL_ERROR, "indigestion"));
+ EXPECT_CALL(visitor, OnCloseStream(3, Http2ErrorCode::REFUSED_STREAM));
+ EXPECT_CALL(visitor, OnFrameHeader(0, 4, WINDOW_UPDATE, 0));
+ EXPECT_CALL(visitor, OnWindowUpdate(0, 42));
+ EXPECT_CALL(visitor, OnFrameHeader(1, 4, WINDOW_UPDATE, 0));
+
+ const int64_t stream_result = adapter->ProcessBytes(stream_frames);
+ EXPECT_EQ(stream_frames.size(), stream_result);
+
+ // Intriguingly, despite want_write() being true, the sent data is empty; the
+ // receipt of the GOAWAY prevents the SETTINGS ack from being written.
+ EXPECT_TRUE(adapter->want_write());
+ result = adapter->Send();
+ EXPECT_EQ(0, result);
+ EXPECT_THAT(visitor.data(), testing::IsEmpty());
+}
+
TEST(NgHttp2AdapterTest, ClientFailsOnGoAway) {
DataSavingVisitor visitor;
auto adapter = NgHttp2Adapter::CreateClientAdapter(visitor);
diff --git a/http2/adapter/oghttp2_adapter_test.cc b/http2/adapter/oghttp2_adapter_test.cc
index eed1071..9c8523c 100644
--- a/http2/adapter/oghttp2_adapter_test.cc
+++ b/http2/adapter/oghttp2_adapter_test.cc
@@ -1913,6 +1913,85 @@
EXPECT_THAT(serialized, EqualsFrames({SpdyFrameType::SETTINGS}));
}
+TEST(OgHttp2AdapterTest, ClientReceivesGoAway) {
+ DataSavingVisitor visitor;
+ OgHttp2Adapter::Options options{.perspective = Perspective::kClient};
+ auto adapter = OgHttp2Adapter::Create(visitor, options);
+
+ testing::InSequence s;
+
+ const std::vector<Header> headers1 =
+ ToHeaders({{":method", "GET"},
+ {":scheme", "http"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/one"}});
+
+ const int32_t stream_id1 = adapter->SubmitRequest(headers1, nullptr, nullptr);
+ ASSERT_GT(stream_id1, 0);
+
+ const std::vector<Header> headers2 =
+ ToHeaders({{":method", "GET"},
+ {":scheme", "http"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/two"}});
+
+ const int32_t stream_id2 = adapter->SubmitRequest(headers2, nullptr, nullptr);
+ ASSERT_GT(stream_id2, stream_id1);
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x0));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x0, 0));
+ EXPECT_CALL(visitor, OnBeforeFrameSent(HEADERS, stream_id1, _, 0x5));
+ EXPECT_CALL(visitor, OnFrameSent(HEADERS, stream_id1, _, 0x5, 0));
+ EXPECT_CALL(visitor, OnBeforeFrameSent(HEADERS, stream_id2, _, 0x5));
+ EXPECT_CALL(visitor, OnFrameSent(HEADERS, stream_id2, _, 0x5, 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({SpdyFrameType::SETTINGS, SpdyFrameType::HEADERS,
+ SpdyFrameType::HEADERS}));
+ visitor.Clear();
+
+ const std::string stream_frames =
+ TestFrameSequence()
+ .ServerPreface()
+ .RstStream(1, Http2ErrorCode::ENHANCE_YOUR_CALM)
+ .GoAway(1, Http2ErrorCode::INTERNAL_ERROR, "indigestion")
+ .WindowUpdate(0, 42)
+ .WindowUpdate(1, 42)
+ .Serialize();
+
+ EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0));
+ EXPECT_CALL(visitor, OnSettingsStart());
+ EXPECT_CALL(visitor, OnSettingsEnd());
+ EXPECT_CALL(visitor, OnFrameHeader(1, 4, RST_STREAM, 0));
+ EXPECT_CALL(visitor, OnRstStream(1, Http2ErrorCode::ENHANCE_YOUR_CALM));
+ EXPECT_CALL(visitor, OnCloseStream(1, Http2ErrorCode::ENHANCE_YOUR_CALM));
+ EXPECT_CALL(visitor, OnFrameHeader(0, _, GOAWAY, 0));
+ // Currently, oghttp2 does not pass the opaque data to the visitor.
+ EXPECT_CALL(visitor, OnGoAway(1, Http2ErrorCode::INTERNAL_ERROR, ""));
+ // TODO(b/228093860): Close stream IDs above the GOAWAY's last_stream_id.
+ EXPECT_CALL(visitor, OnFrameHeader(0, 4, WINDOW_UPDATE, 0));
+ EXPECT_CALL(visitor, OnWindowUpdate(0, 42));
+ EXPECT_CALL(visitor, OnFrameHeader(1, 4, WINDOW_UPDATE, 0));
+ // TODO(b/228093860): Do not pass WINDOW_UPDATE events on closed streams.
+ EXPECT_CALL(visitor, OnWindowUpdate(1, 42));
+
+ const int64_t stream_result = adapter->ProcessBytes(stream_frames);
+ EXPECT_EQ(stream_frames.size(), static_cast<size_t>(stream_result));
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, 0, 0x1));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, 0, 0x1, 0));
+
+ EXPECT_TRUE(adapter->want_write());
+ result = adapter->Send();
+ EXPECT_EQ(0, result);
+ EXPECT_THAT(visitor.data(), EqualsFrames({SpdyFrameType::SETTINGS}));
+}
+
TEST(OgHttp2AdapterTest, ClientFailsOnGoAway) {
DataSavingVisitor visitor;
OgHttp2Adapter::Options options{.perspective = Perspective::kClient};