Updates Http2VisitorInterface::OnHeaderForStream() to return a boolean. This is necessary in order to communicate when header names or values are invalid.
This change adds a test that exercises nghttp2 header validation. (Specifically, the case of an invalid trailing header name.)
This change also adds the following behavior for the OG stack, for client endpoints:
* Closes a stream that receives END_STREAM and is half-closed local.
PiperOrigin-RevId: 380816879
diff --git a/http2/adapter/callback_visitor.cc b/http2/adapter/callback_visitor.cc
index 70fd899..042d32b 100644
--- a/http2/adapter/callback_visitor.cc
+++ b/http2/adapter/callback_visitor.cc
@@ -113,12 +113,16 @@
it->second->received_headers = true;
}
-void CallbackVisitor::OnHeaderForStream(Http2StreamId stream_id,
+bool CallbackVisitor::OnHeaderForStream(Http2StreamId stream_id,
absl::string_view name,
absl::string_view value) {
- callbacks_->on_header_callback(
- nullptr, ¤t_frame_, ToUint8Ptr(name.data()), name.size(),
- ToUint8Ptr(value.data()), value.size(), NGHTTP2_NV_FLAG_NONE, user_data_);
+ if (callbacks_->on_header_callback) {
+ return 0 == callbacks_->on_header_callback(
+ nullptr, ¤t_frame_, ToUint8Ptr(name.data()),
+ name.size(), ToUint8Ptr(value.data()), value.size(),
+ NGHTTP2_NV_FLAG_NONE, user_data_);
+ }
+ return true;
}
void CallbackVisitor::OnEndHeadersForStream(Http2StreamId stream_id) {
diff --git a/http2/adapter/callback_visitor.h b/http2/adapter/callback_visitor.h
index 0147d8b..5070797 100644
--- a/http2/adapter/callback_visitor.h
+++ b/http2/adapter/callback_visitor.h
@@ -31,8 +31,7 @@
void OnSettingsEnd() override;
void OnSettingsAck() override;
void OnBeginHeadersForStream(Http2StreamId stream_id) override;
- void OnHeaderForStream(Http2StreamId stream_id,
- absl::string_view name,
+ bool OnHeaderForStream(Http2StreamId stream_id, absl::string_view name,
absl::string_view value) override;
void OnEndHeadersForStream(Http2StreamId stream_id) override;
void OnBeginDataForStream(Http2StreamId stream_id,
diff --git a/http2/adapter/http2_visitor_interface.h b/http2/adapter/http2_visitor_interface.h
index a7e34e9..8e621e4 100644
--- a/http2/adapter/http2_visitor_interface.h
+++ b/http2/adapter/http2_visitor_interface.h
@@ -83,8 +83,10 @@
// Called when the connection receives the header |key| and |value| for a
// stream. The HTTP/2 pseudo-headers defined in RFC 7540 Sections 8.1.2.3 and
// 8.1.2.4 are also conveyed in this callback. This method is called after
- // OnBeginHeadersForStream().
- virtual void OnHeaderForStream(Http2StreamId stream_id, absl::string_view key,
+ // OnBeginHeadersForStream(). Should return "false" to indicate that the
+ // header name or value should be rejected. This will cause the HTTP
+ // transaction to fail.
+ virtual bool OnHeaderForStream(Http2StreamId stream_id, absl::string_view key,
absl::string_view value) = 0;
// Called when the connection has received the complete header block for a
diff --git a/http2/adapter/mock_http2_visitor.h b/http2/adapter/mock_http2_visitor.h
index 3c4ee2f..09a9077 100644
--- a/http2/adapter/mock_http2_visitor.h
+++ b/http2/adapter/mock_http2_visitor.h
@@ -11,7 +11,9 @@
// A mock visitor class, for use in tests.
class MockHttp2Visitor : public Http2VisitorInterface {
public:
- MockHttp2Visitor() = default;
+ MockHttp2Visitor() {
+ ON_CALL(*this, OnHeaderForStream).WillByDefault(testing::Return(true));
+ }
MOCK_METHOD(ssize_t,
OnReadyToSend,
@@ -32,10 +34,8 @@
(Http2StreamId stream_id),
(override));
- MOCK_METHOD(void,
- OnHeaderForStream,
- (Http2StreamId stream_id,
- absl::string_view key,
+ MOCK_METHOD(bool, OnHeaderForStream,
+ (Http2StreamId stream_id, absl::string_view key,
absl::string_view value),
(override));
diff --git a/http2/adapter/nghttp2_adapter_test.cc b/http2/adapter/nghttp2_adapter_test.cc
index b0e7127..d2875eb 100644
--- a/http2/adapter/nghttp2_adapter_test.cc
+++ b/http2/adapter/nghttp2_adapter_test.cc
@@ -98,39 +98,32 @@
{":scheme", "http"},
{":authority", "example.com"},
{":path", "/this/is/request/one"}});
- const auto nvs1 = GetNghttp2Nvs(headers1);
const std::vector<const Header> headers2 =
ToHeaders({{":method", "GET"},
{":scheme", "http"},
{":authority", "example.com"},
{":path", "/this/is/request/two"}});
- const auto nvs2 = GetNghttp2Nvs(headers2);
const std::vector<const Header> headers3 =
ToHeaders({{":method", "GET"},
{":scheme", "http"},
{":authority", "example.com"},
{":path", "/this/is/request/three"}});
- const auto nvs3 = GetNghttp2Nvs(headers3);
const char* kSentinel1 = "arbitrary pointer 1";
const char* kSentinel3 = "arbitrary pointer 3";
- const int32_t stream_id1 = nghttp2_submit_request(
- adapter->session().raw_ptr(), nullptr, nvs1.data(), nvs1.size(), nullptr,
- const_cast<char*>(kSentinel1));
+ const int32_t stream_id1 =
+ adapter->SubmitRequest(headers1, nullptr, const_cast<char*>(kSentinel1));
ASSERT_GT(stream_id1, 0);
QUICHE_LOG(INFO) << "Created stream: " << stream_id1;
- const int32_t stream_id2 =
- nghttp2_submit_request(adapter->session().raw_ptr(), nullptr, nvs2.data(),
- nvs2.size(), nullptr, nullptr);
+ const int32_t stream_id2 = adapter->SubmitRequest(headers2, nullptr, nullptr);
ASSERT_GT(stream_id2, 0);
QUICHE_LOG(INFO) << "Created stream: " << stream_id2;
- const int32_t stream_id3 = nghttp2_submit_request(
- adapter->session().raw_ptr(), nullptr, nvs3.data(), nvs3.size(), nullptr,
- const_cast<char*>(kSentinel3));
+ const int32_t stream_id3 =
+ adapter->SubmitRequest(headers3, nullptr, const_cast<char*>(kSentinel3));
ASSERT_GT(stream_id3, 0);
QUICHE_LOG(INFO) << "Created stream: " << stream_id3;
@@ -252,6 +245,147 @@
EXPECT_THAT(visitor.data(), testing::IsEmpty());
}
+TEST(NgHttp2AdapterTest, ClientHandlesTrailers) {
+ DataSavingVisitor visitor;
+ auto adapter = NgHttp2Adapter::CreateClientAdapter(visitor);
+
+ testing::InSequence s;
+
+ const std::vector<const Header> headers1 =
+ ToHeaders({{":method", "GET"},
+ {":scheme", "http"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/one"}});
+
+ const char* kSentinel1 = "arbitrary pointer 1";
+ const int32_t stream_id1 =
+ adapter->SubmitRequest(headers1, nullptr, const_cast<char*>(kSentinel1));
+ ASSERT_GT(stream_id1, 0);
+ QUICHE_LOG(INFO) << "Created stream: " << stream_id1;
+
+ 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::HEADERS}));
+ visitor.Clear();
+
+ const std::string stream_frames =
+ TestFrameSequence()
+ .ServerPreface()
+ .Headers(1,
+ {{":status", "200"},
+ {"server", "my-fake-server"},
+ {"date", "Tue, 6 Apr 2021 12:54:01 GMT"}},
+ /*fin=*/false)
+ .Data(1, "This is the response body.")
+ .Headers(1, {{"final-status", "A-OK"}},
+ /*fin=*/true)
+ .Serialize();
+
+ // Server preface (empty SETTINGS)
+ EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0));
+ EXPECT_CALL(visitor, OnSettingsStart());
+ EXPECT_CALL(visitor, OnSettingsEnd());
+
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 4));
+ EXPECT_CALL(visitor, OnBeginHeadersForStream(1));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, ":status", "200"));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, "server", "my-fake-server"));
+ EXPECT_CALL(visitor,
+ OnHeaderForStream(1, "date", "Tue, 6 Apr 2021 12:54:01 GMT"));
+ EXPECT_CALL(visitor, OnEndHeadersForStream(1));
+ EXPECT_CALL(visitor, OnFrameHeader(1, 26, DATA, 0));
+ EXPECT_CALL(visitor, OnBeginDataForStream(1, 26));
+ EXPECT_CALL(visitor, OnDataForStream(1, "This is the response body."));
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 5));
+ EXPECT_CALL(visitor, OnBeginHeadersForStream(1));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, "final-status", "A-OK"));
+ EXPECT_CALL(visitor, OnEndHeadersForStream(1));
+ EXPECT_CALL(visitor, OnEndStream(1));
+ EXPECT_CALL(visitor, OnCloseStream(1, Http2ErrorCode::NO_ERROR));
+
+ const ssize_t stream_result = adapter->ProcessBytes(stream_frames);
+ EXPECT_EQ(stream_frames.size(), stream_result);
+
+ EXPECT_TRUE(adapter->session().want_write());
+ result = adapter->Send();
+ EXPECT_EQ(0, result);
+ EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS}));
+}
+
+TEST(NgHttp2AdapterTest, ClientHandlesInvalidTrailers) {
+ DataSavingVisitor visitor;
+ auto adapter = NgHttp2Adapter::CreateClientAdapter(visitor);
+
+ testing::InSequence s;
+
+ const std::vector<const Header> headers1 =
+ ToHeaders({{":method", "GET"},
+ {":scheme", "http"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/one"}});
+
+ const char* kSentinel1 = "arbitrary pointer 1";
+ const int32_t stream_id1 =
+ adapter->SubmitRequest(headers1, nullptr, const_cast<char*>(kSentinel1));
+ ASSERT_GT(stream_id1, 0);
+ QUICHE_LOG(INFO) << "Created stream: " << stream_id1;
+
+ 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::HEADERS}));
+ visitor.Clear();
+
+ const std::string stream_frames =
+ TestFrameSequence()
+ .ServerPreface()
+ .Headers(1,
+ {{":status", "200"},
+ {"server", "my-fake-server"},
+ {"date", "Tue, 6 Apr 2021 12:54:01 GMT"}},
+ /*fin=*/false)
+ .Data(1, "This is the response body.")
+ .Headers(1, {{":bad-status", "9000"}},
+ /*fin=*/true)
+ .Serialize();
+
+ // Server preface (empty SETTINGS)
+ EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0));
+ EXPECT_CALL(visitor, OnSettingsStart());
+ EXPECT_CALL(visitor, OnSettingsEnd());
+
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 4));
+ EXPECT_CALL(visitor, OnBeginHeadersForStream(1));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, ":status", "200"));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, "server", "my-fake-server"));
+ EXPECT_CALL(visitor,
+ OnHeaderForStream(1, "date", "Tue, 6 Apr 2021 12:54:01 GMT"));
+ EXPECT_CALL(visitor, OnEndHeadersForStream(1));
+ EXPECT_CALL(visitor, OnFrameHeader(1, 26, DATA, 0));
+ EXPECT_CALL(visitor, OnBeginDataForStream(1, 26));
+ EXPECT_CALL(visitor, OnDataForStream(1, "This is the response body."));
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 5));
+ EXPECT_CALL(visitor, OnBeginHeadersForStream(1));
+
+ // Bad status trailer will cause a PROTOCOL_ERROR. The header is never
+ // delivered in an OnHeaderForStream callback.
+ EXPECT_CALL(visitor, OnCloseStream(1, Http2ErrorCode::PROTOCOL_ERROR));
+
+ const ssize_t stream_result = adapter->ProcessBytes(stream_frames);
+ EXPECT_EQ(stream_frames.size(), stream_result);
+
+ EXPECT_TRUE(adapter->session().want_write());
+ result = adapter->Send();
+ EXPECT_EQ(0, result);
+ EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS,
+ spdy::SpdyFrameType::RST_STREAM}));
+}
+
TEST(NgHttp2AdapterTest, ClientSubmitRequest) {
DataSavingVisitor visitor;
auto adapter = NgHttp2Adapter::CreateClientAdapter(visitor);
@@ -812,6 +946,71 @@
EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::HEADERS}));
}
+TEST(NgHttp2AdapterTest, ServerSendsInvalidTrailers) {
+ DataSavingVisitor visitor;
+ auto adapter = NgHttp2Adapter::CreateServerAdapter(visitor);
+ EXPECT_FALSE(adapter->session().want_write());
+
+ const std::string frames = TestFrameSequence()
+ .ClientPreface()
+ .Headers(1,
+ {{":method", "GET"},
+ {":scheme", "https"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/one"}},
+ /*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, 5));
+ EXPECT_CALL(visitor, OnBeginHeadersForStream(1));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, ":method", "GET"));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, ":scheme", "https"));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, ":authority", "example.com"));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, ":path", "/this/is/request/one"));
+ EXPECT_CALL(visitor, OnEndHeadersForStream(1));
+ EXPECT_CALL(visitor, OnEndStream(1));
+
+ const ssize_t result = adapter->ProcessBytes(frames);
+ EXPECT_EQ(frames.size(), result);
+
+ const absl::string_view kBody = "This is an example response body.";
+
+ // The body source must indicate that the end of the body is not the end of
+ // the stream.
+ auto body1 =
+ absl::make_unique<TestDataFrameSource>(visitor, kBody, /*has_fin=*/false);
+ int submit_result = adapter->SubmitResponse(
+ 1, ToHeaders({{":status", "200"}, {"x-comment", "Sure, sounds good."}}),
+ std::move(body1));
+ EXPECT_EQ(submit_result, 0);
+ EXPECT_TRUE(adapter->session().want_write());
+ 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,
+ spdy::SpdyFrameType::DATA}));
+ EXPECT_THAT(visitor.data(), testing::HasSubstr(kBody));
+ visitor.Clear();
+ EXPECT_FALSE(adapter->session().want_write());
+
+ // The body source has been exhausted by the call to Send() above.
+ int trailer_result =
+ adapter->SubmitTrailer(1, ToHeaders({{":final-status", "a-ok"}}));
+ ASSERT_EQ(trailer_result, 0);
+ EXPECT_TRUE(adapter->session().want_write());
+
+ send_result = adapter->Send();
+ EXPECT_EQ(0, send_result);
+ EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::HEADERS}));
+}
+
} // namespace
} // namespace test
} // namespace adapter
diff --git a/http2/adapter/nghttp2_callbacks.cc b/http2/adapter/nghttp2_callbacks.cc
index 6bf5455..d0895c1 100644
--- a/http2/adapter/nghttp2_callbacks.cc
+++ b/http2/adapter/nghttp2_callbacks.cc
@@ -161,9 +161,9 @@
void* user_data) {
QUICHE_CHECK_NE(user_data, nullptr);
auto* visitor = static_cast<Http2VisitorInterface*>(user_data);
- visitor->OnHeaderForStream(frame->hd.stream_id, ToStringView(name),
- ToStringView(value));
- return 0;
+ const bool success = visitor->OnHeaderForStream(
+ frame->hd.stream_id, ToStringView(name), ToStringView(value));
+ return success ? 0 : NGHTTP2_ERR_HTTP_HEADER;
}
int OnDataChunk(nghttp2_session* /* session */,
diff --git a/http2/adapter/oghttp2_adapter_test.cc b/http2/adapter/oghttp2_adapter_test.cc
index 9f4da15..543fab4 100644
--- a/http2/adapter/oghttp2_adapter_test.cc
+++ b/http2/adapter/oghttp2_adapter_test.cc
@@ -11,6 +11,20 @@
namespace test {
namespace {
+using testing::_;
+
+enum FrameType {
+ DATA,
+ HEADERS,
+ PRIORITY,
+ RST_STREAM,
+ SETTINGS,
+ PUSH_PROMISE,
+ PING,
+ GOAWAY,
+ WINDOW_UPDATE,
+};
+
using spdy::SpdyFrameType;
class OgHttp2AdapterTest : public testing::Test {
@@ -39,6 +53,152 @@
TestFrameSequence().ClientPreface().Ping(17).Serialize());
}
+TEST(OgHttp2AdapterClientTest, ClientHandlesTrailers) {
+ DataSavingVisitor visitor;
+ OgHttp2Adapter::Options options{.perspective = Perspective::kClient};
+ auto adapter = OgHttp2Adapter::Create(visitor, options);
+
+ testing::InSequence s;
+
+ const std::vector<const Header> headers1 =
+ ToHeaders({{":method", "GET"},
+ {":scheme", "http"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/one"}});
+
+ const char* kSentinel1 = "arbitrary pointer 1";
+ const int32_t stream_id1 =
+ adapter->SubmitRequest(headers1, nullptr, const_cast<char*>(kSentinel1));
+ ASSERT_GT(stream_id1, 0);
+ QUICHE_LOG(INFO) << "Created stream: " << stream_id1;
+
+ 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,
+ spdy::SpdyFrameType::HEADERS}));
+ visitor.Clear();
+
+ const std::string stream_frames =
+ TestFrameSequence()
+ .ServerPreface()
+ .Headers(1,
+ {{":status", "200"},
+ {"server", "my-fake-server"},
+ {"date", "Tue, 6 Apr 2021 12:54:01 GMT"}},
+ /*fin=*/false)
+ .Data(1, "This is the response body.")
+ .Headers(1, {{"final-status", "A-OK"}},
+ /*fin=*/true)
+ .Serialize();
+
+ // Server preface (empty SETTINGS)
+ EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0));
+ EXPECT_CALL(visitor, OnSettingsStart());
+ EXPECT_CALL(visitor, OnSettingsEnd());
+
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 4));
+ EXPECT_CALL(visitor, OnBeginHeadersForStream(1));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, ":status", "200"));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, "server", "my-fake-server"));
+ EXPECT_CALL(visitor,
+ OnHeaderForStream(1, "date", "Tue, 6 Apr 2021 12:54:01 GMT"));
+ EXPECT_CALL(visitor, OnEndHeadersForStream(1));
+ EXPECT_CALL(visitor, OnFrameHeader(1, 26, DATA, 0));
+ EXPECT_CALL(visitor, OnBeginDataForStream(1, 26));
+ EXPECT_CALL(visitor, OnDataForStream(1, "This is the response body."));
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 5));
+ EXPECT_CALL(visitor, OnBeginHeadersForStream(1));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, "final-status", "A-OK"));
+ EXPECT_CALL(visitor, OnEndHeadersForStream(1));
+ EXPECT_CALL(visitor, OnEndStream(1));
+ EXPECT_CALL(visitor, OnCloseStream(1, Http2ErrorCode::NO_ERROR));
+
+ const ssize_t stream_result = adapter->ProcessBytes(stream_frames);
+ EXPECT_EQ(stream_frames.size(), stream_result);
+
+ EXPECT_TRUE(adapter->session().want_write());
+ result = adapter->Send();
+ EXPECT_EQ(0, result);
+ EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS}));
+}
+
+// TODO(birenroy): Validate headers and re-enable this test.
+TEST(OgHttp2AdapterClientTest, DISABLED_ClientHandlesInvalidTrailers) {
+ DataSavingVisitor visitor;
+ OgHttp2Adapter::Options options{.perspective = Perspective::kClient};
+ auto adapter = OgHttp2Adapter::Create(visitor, options);
+
+ testing::InSequence s;
+
+ const std::vector<const Header> headers1 =
+ ToHeaders({{":method", "GET"},
+ {":scheme", "http"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/one"}});
+
+ const char* kSentinel1 = "arbitrary pointer 1";
+ const int32_t stream_id1 =
+ adapter->SubmitRequest(headers1, nullptr, const_cast<char*>(kSentinel1));
+ ASSERT_GT(stream_id1, 0);
+ QUICHE_LOG(INFO) << "Created stream: " << stream_id1;
+
+ 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,
+ spdy::SpdyFrameType::HEADERS}));
+ visitor.Clear();
+
+ const std::string stream_frames =
+ TestFrameSequence()
+ .ServerPreface()
+ .Headers(1,
+ {{":status", "200"},
+ {"server", "my-fake-server"},
+ {"date", "Tue, 6 Apr 2021 12:54:01 GMT"}},
+ /*fin=*/false)
+ .Data(1, "This is the response body.")
+ .Headers(1, {{":bad-status", "9000"}},
+ /*fin=*/true)
+ .Serialize();
+
+ // Server preface (empty SETTINGS)
+ EXPECT_CALL(visitor, OnFrameHeader(0, 0, SETTINGS, 0));
+ EXPECT_CALL(visitor, OnSettingsStart());
+ EXPECT_CALL(visitor, OnSettingsEnd());
+
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 4));
+ EXPECT_CALL(visitor, OnBeginHeadersForStream(1));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, ":status", "200"));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, "server", "my-fake-server"));
+ EXPECT_CALL(visitor,
+ OnHeaderForStream(1, "date", "Tue, 6 Apr 2021 12:54:01 GMT"));
+ EXPECT_CALL(visitor, OnEndHeadersForStream(1));
+ EXPECT_CALL(visitor, OnFrameHeader(1, 26, DATA, 0));
+ EXPECT_CALL(visitor, OnBeginDataForStream(1, 26));
+ EXPECT_CALL(visitor, OnDataForStream(1, "This is the response body."));
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, HEADERS, 5));
+ EXPECT_CALL(visitor, OnBeginHeadersForStream(1));
+
+ // Bad status trailer will cause a PROTOCOL_ERROR. The header is never
+ // delivered in an OnHeaderForStream callback.
+ EXPECT_CALL(visitor, OnCloseStream(1, Http2ErrorCode::PROTOCOL_ERROR));
+
+ const ssize_t stream_result = adapter->ProcessBytes(stream_frames);
+ EXPECT_EQ(stream_frames.size(), stream_result);
+
+ EXPECT_TRUE(adapter->session().want_write());
+ result = adapter->Send();
+ EXPECT_EQ(0, result);
+ EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS,
+ spdy::SpdyFrameType::RST_STREAM}));
+}
+
TEST_F(OgHttp2AdapterTest, SubmitMetadata) {
EXPECT_QUICHE_BUG(adapter_->SubmitMetadata(3, true), "Not implemented");
}
@@ -102,6 +262,73 @@
SpdyFrameType::PING}));
}
+TEST(OgHttp2AdapterServerTest, ServerSendsInvalidTrailers) {
+ DataSavingVisitor visitor;
+ OgHttp2Adapter::Options options{.perspective = Perspective::kServer};
+ auto adapter = OgHttp2Adapter::Create(visitor, options);
+ EXPECT_FALSE(adapter->session().want_write());
+
+ const std::string frames = TestFrameSequence()
+ .ClientPreface()
+ .Headers(1,
+ {{":method", "GET"},
+ {":scheme", "https"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/one"}},
+ /*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, 5));
+ EXPECT_CALL(visitor, OnBeginHeadersForStream(1));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, ":method", "GET"));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, ":scheme", "https"));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, ":authority", "example.com"));
+ EXPECT_CALL(visitor, OnHeaderForStream(1, ":path", "/this/is/request/one"));
+ EXPECT_CALL(visitor, OnEndHeadersForStream(1));
+ EXPECT_CALL(visitor, OnEndStream(1));
+
+ const ssize_t result = adapter->ProcessBytes(frames);
+ EXPECT_EQ(frames.size(), result);
+
+ const absl::string_view kBody = "This is an example response body.";
+
+ // The body source must indicate that the end of the body is not the end of
+ // the stream.
+ auto body1 =
+ absl::make_unique<TestDataFrameSource>(visitor, kBody, /*has_fin=*/false);
+ int submit_result = adapter->SubmitResponse(
+ 1, ToHeaders({{":status", "200"}, {"x-comment", "Sure, sounds good."}}),
+ std::move(body1));
+ EXPECT_EQ(submit_result, 0);
+ EXPECT_TRUE(adapter->session().want_write());
+ 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::SETTINGS,
+ spdy::SpdyFrameType::HEADERS, spdy::SpdyFrameType::DATA}));
+ EXPECT_THAT(visitor.data(), testing::HasSubstr(kBody));
+ visitor.Clear();
+ EXPECT_FALSE(adapter->session().want_write());
+
+ // The body source has been exhausted by the call to Send() above.
+ int trailer_result =
+ adapter->SubmitTrailer(1, ToHeaders({{":final-status", "a-ok"}}));
+ ASSERT_EQ(trailer_result, 0);
+ EXPECT_TRUE(adapter->session().want_write());
+
+ send_result = adapter->Send();
+ EXPECT_EQ(0, send_result);
+ EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::HEADERS}));
+}
+
} // namespace
} // namespace test
} // namespace adapter
diff --git a/http2/adapter/oghttp2_session.cc b/http2/adapter/oghttp2_session.cc
index 6d252e2..b9962a3 100644
--- a/http2/adapter/oghttp2_session.cc
+++ b/http2/adapter/oghttp2_session.cc
@@ -428,6 +428,12 @@
iter->second.half_closed_remote = true;
}
visitor_.OnEndStream(stream_id);
+ if (iter != stream_map_.end() && iter->second.half_closed_local &&
+ options_.perspective == Perspective::kClient) {
+ // From the client's perspective, the stream can be closed if it's already
+ // half_closed_local.
+ visitor_.OnCloseStream(stream_id, Http2ErrorCode::NO_ERROR);
+ }
}
void OgHttp2Session::OnStreamPadLength(spdy::SpdyStreamId stream_id,
diff --git a/http2/adapter/recording_http2_visitor.cc b/http2/adapter/recording_http2_visitor.cc
index 4cae3c0..bc07fe4 100644
--- a/http2/adapter/recording_http2_visitor.cc
+++ b/http2/adapter/recording_http2_visitor.cc
@@ -45,11 +45,12 @@
events_.push_back(absl::StrFormat("OnBeginHeadersForStream %d", stream_id));
}
-void RecordingHttp2Visitor::OnHeaderForStream(Http2StreamId stream_id,
+bool RecordingHttp2Visitor::OnHeaderForStream(Http2StreamId stream_id,
absl::string_view name,
absl::string_view value) {
events_.push_back(
absl::StrFormat("OnHeaderForStream %d %s %s", stream_id, name, value));
+ return true;
}
void RecordingHttp2Visitor::OnEndHeadersForStream(Http2StreamId stream_id) {
diff --git a/http2/adapter/recording_http2_visitor.h b/http2/adapter/recording_http2_visitor.h
index feac2de..74498cc 100644
--- a/http2/adapter/recording_http2_visitor.h
+++ b/http2/adapter/recording_http2_visitor.h
@@ -29,8 +29,7 @@
void OnSettingsEnd() override;
void OnSettingsAck() override;
void OnBeginHeadersForStream(Http2StreamId stream_id) override;
- void OnHeaderForStream(Http2StreamId stream_id,
- absl::string_view name,
+ bool OnHeaderForStream(Http2StreamId stream_id, absl::string_view name,
absl::string_view value) override;
void OnEndHeadersForStream(Http2StreamId stream_id) override;
void OnBeginDataForStream(Http2StreamId stream_id,
