Adds unit tests demonstrating bugs in existing oghttp2 handling of stream close events.
PiperOrigin-RevId: 409513292
diff --git a/http2/adapter/nghttp2_adapter_test.cc b/http2/adapter/nghttp2_adapter_test.cc
index 56fcff5..8cb3c84 100644
--- a/http2/adapter/nghttp2_adapter_test.cc
+++ b/http2/adapter/nghttp2_adapter_test.cc
@@ -2564,6 +2564,106 @@
EXPECT_EQ(trailer_result, 0);
}
+TEST(NgHttp2AdapterTest, CompleteRequestWithServerResponse) {
+ DataSavingVisitor visitor;
+ auto adapter = NgHttp2Adapter::CreateServerAdapter(visitor);
+ EXPECT_FALSE(adapter->want_write());
+
+ const std::string frames =
+ TestFrameSequence()
+ .ClientPreface()
+ .Headers(1,
+ {{":method", "GET"},
+ {":scheme", "https"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/one"}},
+ /*fin=*/false)
+ .Data(1, "This is the response body.", /*fin=*/true)
+ .Serialize();
+ testing::InSequence s;
+
+ // Client preface (empty SETTINGS)
+ EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0));
+ EXPECT_CALL(visitor, OnSettingsStart());
+ EXPECT_CALL(visitor, OnSettingsEnd());
+ // Stream 1
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 4));
+ EXPECT_CALL(visitor, OnBeginHeadersForStream(1));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, _, _)).Times(4);
+ EXPECT_CALL(visitor, OnEndHeadersForStream(1));
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, DATA, 1));
+ EXPECT_CALL(visitor, OnBeginDataForStream(1, _));
+ EXPECT_CALL(visitor, OnDataForStream(1, _));
+ EXPECT_CALL(visitor, OnEndStream(1));
+
+ const int64_t result = adapter->ProcessBytes(frames);
+ EXPECT_EQ(frames.size(), static_cast<size_t>(result));
+
+ int submit_result =
+ adapter->SubmitResponse(1, ToHeaders({{":status", "200"}}), nullptr);
+ EXPECT_EQ(submit_result, 0);
+ EXPECT_TRUE(adapter->want_write());
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x1));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x1, 0));
+ EXPECT_CALL(visitor, OnBeforeFrameSent(HEADERS, 1, _, 0x5));
+ EXPECT_CALL(visitor, OnFrameSent(HEADERS, 1, _, 0x5, 0));
+ EXPECT_CALL(visitor, OnCloseStream(1, Http2ErrorCode::NO_ERROR));
+
+ int send_result = adapter->Send();
+ EXPECT_EQ(0, send_result);
+ EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS,
+ spdy::SpdyFrameType::HEADERS}));
+ EXPECT_FALSE(adapter->want_write());
+}
+
+TEST(NgHttp2AdapterTest, IncompleteRequestWithServerResponse) {
+ DataSavingVisitor visitor;
+ auto adapter = NgHttp2Adapter::CreateServerAdapter(visitor);
+ EXPECT_FALSE(adapter->want_write());
+
+ const std::string frames = TestFrameSequence()
+ .ClientPreface()
+ .Headers(1,
+ {{":method", "GET"},
+ {":scheme", "https"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/one"}},
+ /*fin=*/false)
+ .Serialize();
+ testing::InSequence s;
+
+ // Client preface (empty SETTINGS)
+ EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0));
+ EXPECT_CALL(visitor, OnSettingsStart());
+ EXPECT_CALL(visitor, OnSettingsEnd());
+ // Stream 1
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 4));
+ EXPECT_CALL(visitor, OnBeginHeadersForStream(1));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, _, _)).Times(4);
+ EXPECT_CALL(visitor, OnEndHeadersForStream(1));
+
+ const int64_t result = adapter->ProcessBytes(frames);
+ EXPECT_EQ(frames.size(), static_cast<size_t>(result));
+
+ int submit_result =
+ adapter->SubmitResponse(1, ToHeaders({{":status", "200"}}), nullptr);
+ EXPECT_EQ(submit_result, 0);
+ EXPECT_TRUE(adapter->want_write());
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x1));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x1, 0));
+ EXPECT_CALL(visitor, OnBeforeFrameSent(HEADERS, 1, _, 0x5));
+ EXPECT_CALL(visitor, OnFrameSent(HEADERS, 1, _, 0x5, 0));
+ // BUG: Should send RST_STREAM NO_ERROR as well, but nghttp2 does not.
+
+ int send_result = adapter->Send();
+ EXPECT_EQ(0, send_result);
+ EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS,
+ spdy::SpdyFrameType::HEADERS}));
+ EXPECT_FALSE(adapter->want_write());
+}
+
TEST(NgHttp2AdapterTest, ServerSendsInvalidTrailers) {
DataSavingVisitor visitor;
auto adapter = NgHttp2Adapter::CreateServerAdapter(visitor);
diff --git a/http2/adapter/oghttp2_adapter_test.cc b/http2/adapter/oghttp2_adapter_test.cc
index ec879f2..09f9d0c 100644
--- a/http2/adapter/oghttp2_adapter_test.cc
+++ b/http2/adapter/oghttp2_adapter_test.cc
@@ -1489,6 +1489,116 @@
EXPECT_FALSE(adapter->want_write());
}
+TEST(OgHttp2AdapterServerTest, CompleteRequestWithServerResponse) {
+ DataSavingVisitor visitor;
+ OgHttp2Adapter::Options options{.perspective = Perspective::kServer};
+ auto adapter = OgHttp2Adapter::Create(visitor, options);
+ EXPECT_FALSE(adapter->want_write());
+
+ const std::string frames =
+ TestFrameSequence()
+ .ClientPreface()
+ .Headers(1,
+ {{":method", "GET"},
+ {":scheme", "https"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/one"}},
+ /*fin=*/false)
+ .Data(1, "This is the response body.", /*fin=*/true)
+ .Serialize();
+ testing::InSequence s;
+
+ // Client preface (empty SETTINGS)
+ EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0));
+ EXPECT_CALL(visitor, OnSettingsStart());
+ EXPECT_CALL(visitor, OnSettingsEnd());
+ // Stream 1
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 4));
+ EXPECT_CALL(visitor, OnBeginHeadersForStream(1));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, _, _)).Times(4);
+ EXPECT_CALL(visitor, OnEndHeadersForStream(1));
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, DATA, 1));
+ EXPECT_CALL(visitor, OnBeginDataForStream(1, _));
+ EXPECT_CALL(visitor, OnDataForStream(1, _));
+ EXPECT_CALL(visitor, OnEndStream(1));
+
+ const int64_t result = adapter->ProcessBytes(frames);
+ EXPECT_EQ(frames.size(), static_cast<size_t>(result));
+
+ // BUG: OnCloseStream() should be invoked after OnFrameSent() for the response
+ // headers.
+ EXPECT_CALL(visitor, OnCloseStream(1, Http2ErrorCode::NO_ERROR));
+ int submit_result =
+ adapter->SubmitResponse(1, ToHeaders({{":status", "200"}}), nullptr);
+ EXPECT_EQ(submit_result, 0);
+ EXPECT_TRUE(adapter->want_write());
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x0));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x0, 0));
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x1));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x1, 0));
+ EXPECT_CALL(visitor, OnBeforeFrameSent(HEADERS, 1, _, 0x5));
+ EXPECT_CALL(visitor, OnFrameSent(HEADERS, 1, _, 0x5, 0));
+
+ int send_result = adapter->Send();
+ EXPECT_EQ(0, send_result);
+ EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS,
+ spdy::SpdyFrameType::SETTINGS,
+ spdy::SpdyFrameType::HEADERS}));
+ EXPECT_FALSE(adapter->want_write());
+}
+
+TEST(OgHttp2AdapterServerTest, IncompleteRequestWithServerResponse) {
+ DataSavingVisitor visitor;
+ OgHttp2Adapter::Options options{.perspective = Perspective::kServer};
+ auto adapter = OgHttp2Adapter::Create(visitor, options);
+ EXPECT_FALSE(adapter->want_write());
+
+ const std::string frames = TestFrameSequence()
+ .ClientPreface()
+ .Headers(1,
+ {{":method", "GET"},
+ {":scheme", "https"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/one"}},
+ /*fin=*/false)
+ .Serialize();
+ testing::InSequence s;
+
+ // Client preface (empty SETTINGS)
+ EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0));
+ EXPECT_CALL(visitor, OnSettingsStart());
+ EXPECT_CALL(visitor, OnSettingsEnd());
+ // Stream 1
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 4));
+ EXPECT_CALL(visitor, OnBeginHeadersForStream(1));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, _, _)).Times(4);
+ EXPECT_CALL(visitor, OnEndHeadersForStream(1));
+
+ const int64_t result = adapter->ProcessBytes(frames);
+ EXPECT_EQ(frames.size(), static_cast<size_t>(result));
+
+ int submit_result =
+ adapter->SubmitResponse(1, ToHeaders({{":status", "200"}}), nullptr);
+ EXPECT_EQ(submit_result, 0);
+ EXPECT_TRUE(adapter->want_write());
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x0));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x0, 0));
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x1));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x1, 0));
+ EXPECT_CALL(visitor, OnBeforeFrameSent(HEADERS, 1, _, 0x5));
+ EXPECT_CALL(visitor, OnFrameSent(HEADERS, 1, _, 0x5, 0));
+ // BUG: Should send RST_STREAM NO_ERROR as well.
+
+ int send_result = adapter->Send();
+ EXPECT_EQ(0, send_result);
+ EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS,
+ spdy::SpdyFrameType::SETTINGS,
+ spdy::SpdyFrameType::HEADERS}));
+ EXPECT_FALSE(adapter->want_write());
+}
+
TEST(OgHttp2AdapterServerTest, ServerSendsInvalidTrailers) {
DataSavingVisitor visitor;
OgHttp2Adapter::Options options{.perspective = Perspective::kServer};
