Expands Http2VisitorInterface::OnHeaderForStream() to return an enum representing either OK, invalid or RST_STREAM.
This turns out to be necessary for tests related to invalid headers to pass.
This change fills in the implementation gap in the OG stack, with one minor difference: since the application is indicating the header error and potentially queuing the RST_STREAM, CloseStream() does not re-deliver the same error code back to the application.
PiperOrigin-RevId: 381958205
diff --git a/http2/adapter/callback_visitor.cc b/http2/adapter/callback_visitor.cc
index f0dcd0b..945bc0b 100644
--- a/http2/adapter/callback_visitor.cc
+++ b/http2/adapter/callback_visitor.cc
@@ -109,16 +109,23 @@
it->second->received_headers = true;
}
-bool CallbackVisitor::OnHeaderForStream(Http2StreamId stream_id,
- absl::string_view name,
- absl::string_view value) {
+Http2VisitorInterface::OnHeaderResult CallbackVisitor::OnHeaderForStream(
+ Http2StreamId stream_id, absl::string_view name, absl::string_view value) {
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_);
+ const int result = callbacks_->on_header_callback(
+ nullptr, ¤t_frame_, ToUint8Ptr(name.data()), name.size(),
+ ToUint8Ptr(value.data()), value.size(), NGHTTP2_NV_FLAG_NONE,
+ user_data_);
+ if (result == 0) {
+ return HEADER_OK;
+ } else if (result == NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE) {
+ return HEADER_RST_STREAM;
+ } else {
+ // Assume NGHTTP2_ERR_CALLBACK_FAILURE.
+ return HEADER_CONNECTION_ERROR;
+ }
}
- return true;
+ return HEADER_OK;
}
void CallbackVisitor::OnEndHeadersForStream(Http2StreamId stream_id) {
diff --git a/http2/adapter/callback_visitor.h b/http2/adapter/callback_visitor.h
index 49e3f33..38c8dda 100644
--- a/http2/adapter/callback_visitor.h
+++ b/http2/adapter/callback_visitor.h
@@ -31,8 +31,9 @@
void OnSettingsEnd() override;
void OnSettingsAck() override;
void OnBeginHeadersForStream(Http2StreamId stream_id) override;
- bool OnHeaderForStream(Http2StreamId stream_id, absl::string_view name,
- absl::string_view value) override;
+ OnHeaderResult OnHeaderForStream(Http2StreamId stream_id,
+ absl::string_view name,
+ absl::string_view value) override;
void OnEndHeadersForStream(Http2StreamId stream_id) override;
void OnBeginDataForStream(Http2StreamId stream_id,
size_t payload_length) override;
diff --git a/http2/adapter/http2_visitor_interface.h b/http2/adapter/http2_visitor_interface.h
index f71e21c..e3f7c53 100644
--- a/http2/adapter/http2_visitor_interface.h
+++ b/http2/adapter/http2_visitor_interface.h
@@ -83,11 +83,21 @@
// 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(). 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;
+ // OnBeginHeadersForStream(). May return HEADER_RST_STREAM to indicate the
+ // header block should be rejected. This will cause the library to queue a
+ // RST_STREAM frame, which will have a default error code of INTERNAL_ERROR.
+ // The visitor implementation may choose to queue a RST_STREAM with a
+ // different error code instead, which should be done before returning
+ // HEADER_RST_STREAM. Returning HEADER_CONNECTION_ERROR will lead to a
+ // non-recoverable error on the connection.
+ enum OnHeaderResult {
+ HEADER_OK,
+ HEADER_CONNECTION_ERROR,
+ HEADER_RST_STREAM,
+ };
+ virtual OnHeaderResult 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
// logical HEADERS frame on a stream (which may contain CONTINUATION frames,
diff --git a/http2/adapter/mock_http2_visitor.h b/http2/adapter/mock_http2_visitor.h
index 26c37b7..ca46e90 100644
--- a/http2/adapter/mock_http2_visitor.h
+++ b/http2/adapter/mock_http2_visitor.h
@@ -12,7 +12,7 @@
class MockHttp2Visitor : public Http2VisitorInterface {
public:
MockHttp2Visitor() {
- ON_CALL(*this, OnHeaderForStream).WillByDefault(testing::Return(true));
+ ON_CALL(*this, OnHeaderForStream).WillByDefault(testing::Return(HEADER_OK));
ON_CALL(*this, OnInvalidFrame).WillByDefault(testing::Return(true));
ON_CALL(*this, OnMetadataEndForStream).WillByDefault(testing::Return(true));
}
@@ -36,7 +36,7 @@
(Http2StreamId stream_id),
(override));
- MOCK_METHOD(bool, OnHeaderForStream,
+ MOCK_METHOD(OnHeaderResult, 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 22057c7..091f6e4 100644
--- a/http2/adapter/nghttp2_adapter_test.cc
+++ b/http2/adapter/nghttp2_adapter_test.cc
@@ -496,6 +496,148 @@
spdy::SpdyFrameType::RST_STREAM}));
}
+TEST(NgHttp2AdapterTest, ClientRstStreamWhileHandlingHeaders) {
+ 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;
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(HEADERS, stream_id1, _, 0x5));
+ EXPECT_CALL(visitor, OnFrameSent(HEADERS, stream_id1, _, 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({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.")
+ .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"))
+ .WillOnce(testing::DoAll(
+ testing::InvokeWithoutArgs([&adapter]() {
+ adapter->SubmitRst(1, Http2ErrorCode::REFUSED_STREAM);
+ }),
+ testing::Return(Http2VisitorInterface::HEADER_RST_STREAM)));
+
+ const ssize_t stream_result = adapter->ProcessBytes(stream_frames);
+ EXPECT_EQ(stream_frames.size(), stream_result);
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, 0, 0x1));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, 0, 0x1, 0));
+ EXPECT_CALL(visitor, OnBeforeFrameSent(RST_STREAM, stream_id1, 4, 0x0));
+ EXPECT_CALL(visitor,
+ OnFrameSent(RST_STREAM, stream_id1, 4, 0x0,
+ static_cast<int>(Http2ErrorCode::REFUSED_STREAM)));
+ EXPECT_CALL(visitor, OnCloseStream(1, Http2ErrorCode::REFUSED_STREAM));
+
+ 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, ClientConnectionErrorWhileHandlingHeaders) {
+ 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;
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(HEADERS, stream_id1, _, 0x5));
+ EXPECT_CALL(visitor, OnFrameSent(HEADERS, stream_id1, _, 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({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.")
+ .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"))
+ .WillOnce(
+ testing::Return(Http2VisitorInterface::HEADER_CONNECTION_ERROR));
+ EXPECT_CALL(visitor, OnConnectionError());
+
+ const ssize_t stream_result = adapter->ProcessBytes(stream_frames);
+ EXPECT_EQ(-902 /* NGHTTP2_ERR_CALLBACK_FAILURE */, stream_result);
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, 0, 0x1));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, 0, 0x1, 0));
+
+ EXPECT_TRUE(adapter->session().want_write());
+ result = adapter->Send();
+ EXPECT_EQ(0, result);
+ EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS}));
+}
+
TEST(NgHttp2AdapterTest, ClientSubmitRequest) {
DataSavingVisitor visitor;
auto adapter = NgHttp2Adapter::CreateClientAdapter(visitor);
@@ -895,6 +1037,65 @@
spdy::SpdyFrameType::PING}));
}
+TEST(NgHttp2AdapterTest, ServerErrorWhileHandlingHeaders) {
+ DataSavingVisitor visitor;
+ auto adapter = NgHttp2Adapter::CreateServerAdapter(visitor);
+
+ const std::string frames = TestFrameSequence()
+ .ClientPreface()
+ .Headers(1,
+ {{":method", "POST"},
+ {":scheme", "https"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/one"},
+ {"accept", "some bogus value!"}},
+ /*fin=*/false)
+ .WindowUpdate(1, 2000)
+ .Data(1, "This is the request body.")
+ .WindowUpdate(0, 2000)
+ .Serialize();
+ testing::InSequence s;
+
+ // Client 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, ":method", "POST"));
+ 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, OnHeaderForStream(1, "accept", "some bogus value!"))
+ .WillOnce(testing::Return(Http2VisitorInterface::HEADER_RST_STREAM));
+ EXPECT_CALL(visitor, OnFrameHeader(1, 4, WINDOW_UPDATE, 0));
+ EXPECT_CALL(visitor, OnWindowUpdate(1, 2000));
+ // DATA frame is not delivered to the visitor.
+ EXPECT_CALL(visitor, OnFrameHeader(0, 4, WINDOW_UPDATE, 0));
+ EXPECT_CALL(visitor, OnWindowUpdate(0, 2000));
+
+ const ssize_t result = adapter->ProcessBytes(frames);
+ EXPECT_EQ(frames.size(), result);
+
+ EXPECT_TRUE(adapter->session().want_write());
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, 0, 0x1));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, 0, 0x1, 0));
+ EXPECT_CALL(visitor, OnBeforeFrameSent(RST_STREAM, 1, 4, 0x0));
+ EXPECT_CALL(visitor,
+ OnFrameSent(RST_STREAM, 1, 4, 0x0,
+ static_cast<int>(Http2ErrorCode::INTERNAL_ERROR)));
+ EXPECT_CALL(visitor, OnCloseStream(1, Http2ErrorCode::INTERNAL_ERROR));
+
+ int send_result = adapter->Send();
+ // Some bytes should have been serialized.
+ EXPECT_EQ(0, send_result);
+ // SETTINGS ack
+ EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS,
+ spdy::SpdyFrameType::RST_STREAM}));
+}
+
TEST(NgHttp2AdapterTest, ServerSubmitResponse) {
DataSavingVisitor visitor;
auto adapter = NgHttp2Adapter::CreateServerAdapter(visitor);
diff --git a/http2/adapter/nghttp2_callbacks.cc b/http2/adapter/nghttp2_callbacks.cc
index 4ab163d..c2b64f4 100644
--- a/http2/adapter/nghttp2_callbacks.cc
+++ b/http2/adapter/nghttp2_callbacks.cc
@@ -161,9 +161,17 @@
void* user_data) {
QUICHE_CHECK_NE(user_data, nullptr);
auto* visitor = static_cast<Http2VisitorInterface*>(user_data);
- const bool success = visitor->OnHeaderForStream(
- frame->hd.stream_id, ToStringView(name), ToStringView(value));
- return success ? 0 : NGHTTP2_ERR_HTTP_HEADER;
+ const Http2VisitorInterface::OnHeaderResult result =
+ visitor->OnHeaderForStream(frame->hd.stream_id, ToStringView(name),
+ ToStringView(value));
+ switch (result) {
+ case Http2VisitorInterface::HEADER_OK:
+ return 0;
+ case Http2VisitorInterface::HEADER_CONNECTION_ERROR:
+ return NGHTTP2_ERR_CALLBACK_FAILURE;
+ case Http2VisitorInterface::HEADER_RST_STREAM:
+ return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
+ }
}
int OnBeforeFrameSent(nghttp2_session* /* session */,
diff --git a/http2/adapter/oghttp2_adapter_test.cc b/http2/adapter/oghttp2_adapter_test.cc
index 943bae7..4a4307b 100644
--- a/http2/adapter/oghttp2_adapter_test.cc
+++ b/http2/adapter/oghttp2_adapter_test.cc
@@ -215,6 +215,164 @@
EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS}));
}
+TEST(OgHttp2AdapterClientTest, ClientRstStreamWhileHandlingHeaders) {
+ 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;
+
+ 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));
+
+ 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.")
+ .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"))
+ .WillOnce(testing::DoAll(
+ testing::InvokeWithoutArgs([&adapter]() {
+ adapter->SubmitRst(1, Http2ErrorCode::REFUSED_STREAM);
+ }),
+ testing::Return(Http2VisitorInterface::HEADER_RST_STREAM)));
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, DATA, 0));
+ EXPECT_CALL(visitor, OnBeginDataForStream(1, _));
+ EXPECT_CALL(visitor, OnDataForStream(1, "This is the response body."));
+
+ const ssize_t stream_result = adapter->ProcessBytes(stream_frames);
+ EXPECT_EQ(stream_frames.size(), stream_result);
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, 0, 0x1));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, 0, 0x1, 0));
+ EXPECT_CALL(visitor, OnBeforeFrameSent(RST_STREAM, stream_id1, 4, 0x0));
+ EXPECT_CALL(visitor,
+ OnFrameSent(RST_STREAM, stream_id1, 4, 0x0,
+ static_cast<int>(Http2ErrorCode::REFUSED_STREAM)));
+ EXPECT_CALL(visitor, OnCloseStream(1, Http2ErrorCode::NO_ERROR));
+
+ 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, ClientConnectionErrorWhileHandlingHeaders) {
+ 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;
+
+ 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));
+
+ 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.")
+ .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"))
+ .WillOnce(
+ testing::Return(Http2VisitorInterface::HEADER_CONNECTION_ERROR));
+ EXPECT_CALL(visitor, OnConnectionError());
+ // Note: OgHttp2Adapter continues processing bytes until the input is
+ // complete.
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, DATA, 0));
+ EXPECT_CALL(visitor, OnBeginDataForStream(1, _));
+ EXPECT_CALL(visitor, OnDataForStream(1, "This is the response body."));
+
+ const ssize_t stream_result = adapter->ProcessBytes(stream_frames);
+ EXPECT_EQ(stream_frames.size(), stream_result);
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, 0, 0x1));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, 0, 0x1, 0));
+
+ 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. The library should
// invoke OnErrorDebug() with an error message for the invalid header. The
// library should also invoke OnInvalidFrame() for the invalid HEADERS frame.
@@ -334,6 +492,7 @@
EXPECT_CALL(http2_visitor_, OnFrameSent(PRIORITY, 3, _, 0x0, 0));
EXPECT_CALL(http2_visitor_, OnBeforeFrameSent(RST_STREAM, 3, _, 0x0));
EXPECT_CALL(http2_visitor_, OnFrameSent(RST_STREAM, 3, _, 0x0, 0x8));
+ EXPECT_CALL(http2_visitor_, OnCloseStream(3, Http2ErrorCode::NO_ERROR));
EXPECT_CALL(http2_visitor_, OnBeforeFrameSent(PING, 0, _, 0x0));
EXPECT_CALL(http2_visitor_, OnFrameSent(PING, 0, _, 0x0, 0));
EXPECT_CALL(http2_visitor_, OnBeforeFrameSent(GOAWAY, 0, _, 0x0));
@@ -460,6 +619,71 @@
EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::HEADERS}));
}
+TEST(OgHttp2AdapterServerTest, ServerErrorWhileHandlingHeaders) {
+ DataSavingVisitor visitor;
+ OgHttp2Adapter::Options options{.perspective = Perspective::kServer};
+ auto adapter = OgHttp2Adapter::Create(visitor, options);
+
+ const std::string frames = TestFrameSequence()
+ .ClientPreface()
+ .Headers(1,
+ {{":method", "POST"},
+ {":scheme", "https"},
+ {":authority", "example.com"},
+ {":path", "/this/is/request/one"},
+ {"accept", "some bogus value!"}},
+ /*fin=*/false)
+ .WindowUpdate(1, 2000)
+ .Data(1, "This is the request body.")
+ .WindowUpdate(0, 2000)
+ .Serialize();
+ testing::InSequence s;
+
+ // Client 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, ":method", "POST"));
+ 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, OnHeaderForStream(1, "accept", "some bogus value!"))
+ .WillOnce(testing::Return(Http2VisitorInterface::HEADER_RST_STREAM));
+ EXPECT_CALL(visitor, OnFrameHeader(1, 4, WINDOW_UPDATE, 0));
+ EXPECT_CALL(visitor, OnWindowUpdate(1, 2000));
+ EXPECT_CALL(visitor, OnFrameHeader(1, _, DATA, 0));
+ EXPECT_CALL(visitor, OnBeginDataForStream(1, _));
+ EXPECT_CALL(visitor, OnDataForStream(1, "This is the request body."));
+ EXPECT_CALL(visitor, OnFrameHeader(0, 4, WINDOW_UPDATE, 0));
+ EXPECT_CALL(visitor, OnWindowUpdate(0, 2000));
+
+ const ssize_t result = adapter->ProcessBytes(frames);
+ EXPECT_EQ(frames.size(), result);
+
+ EXPECT_TRUE(adapter->session().want_write());
+
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, _, 0x0));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, _, 0x0, 0));
+ EXPECT_CALL(visitor, OnBeforeFrameSent(SETTINGS, 0, 0, 0x1));
+ EXPECT_CALL(visitor, OnFrameSent(SETTINGS, 0, 0, 0x1, 0));
+ EXPECT_CALL(visitor, OnBeforeFrameSent(RST_STREAM, 1, 4, 0x0));
+ EXPECT_CALL(visitor,
+ OnFrameSent(RST_STREAM, 1, 4, 0x0,
+ static_cast<int>(Http2ErrorCode::INTERNAL_ERROR)));
+ EXPECT_CALL(visitor, OnCloseStream(1, Http2ErrorCode::NO_ERROR));
+
+ int send_result = adapter->Send();
+ // Some bytes should have been serialized.
+ EXPECT_EQ(0, send_result);
+ // SETTINGS ack
+ EXPECT_THAT(visitor.data(), EqualsFrames({spdy::SpdyFrameType::SETTINGS,
+ spdy::SpdyFrameType::SETTINGS,
+ spdy::SpdyFrameType::RST_STREAM}));
+}
+
} // namespace
} // namespace test
} // namespace adapter
diff --git a/http2/adapter/oghttp2_session.cc b/http2/adapter/oghttp2_session.cc
index 5532a37..eee664f 100644
--- a/http2/adapter/oghttp2_session.cc
+++ b/http2/adapter/oghttp2_session.cc
@@ -106,18 +106,24 @@
void OgHttp2Session::PassthroughHeadersHandler::OnHeader(
absl::string_view key,
absl::string_view value) {
- visitor_.OnHeaderForStream(stream_id_, key, value);
+ if (result_ == Http2VisitorInterface::HEADER_OK) {
+ result_ = visitor_.OnHeaderForStream(stream_id_, key, value);
+ }
}
void OgHttp2Session::PassthroughHeadersHandler::OnHeaderBlockEnd(
size_t /* uncompressed_header_bytes */,
size_t /* compressed_header_bytes */) {
- visitor_.OnEndHeadersForStream(stream_id_);
+ if (result_ == Http2VisitorInterface::HEADER_OK) {
+ visitor_.OnEndHeadersForStream(stream_id_);
+ } else {
+ session_.OnHeaderStatus(stream_id_, result_);
+ }
}
OgHttp2Session::OgHttp2Session(Http2VisitorInterface& visitor, Options options)
: visitor_(visitor),
- headers_handler_(visitor),
+ headers_handler_(*this, visitor),
connection_window_manager_(kInitialFlowControlWindowSize,
[this](size_t window_update_delta) {
SendWindowUpdate(kConnectionStreamId,
@@ -252,6 +258,12 @@
void OgHttp2Session::EnqueueFrame(std::unique_ptr<spdy::SpdyFrameIR> frame) {
if (frame->frame_type() == spdy::SpdyFrameType::GOAWAY) {
queued_goaway_ = true;
+ } else if (frame->frame_type() == spdy::SpdyFrameType::RST_STREAM) {
+ streams_reset_.insert(frame->stream_id());
+ auto iter = stream_map_.find(frame->stream_id());
+ if (iter != stream_map_.end()) {
+ iter->second.half_closed_local = true;
+ }
}
frames_.push_back(std::move(frame));
}
@@ -303,6 +315,13 @@
} else {
visitor_.OnFrameSent(c.frame_type(), c.stream_id(), c.length(), c.flags(),
c.error_code());
+ if (static_cast<FrameType>(c.frame_type()) == FrameType::RST_STREAM) {
+ // If this endpoint is resetting the stream, the stream should be
+ // closed. This endpoint is already aware of the outbound RST_STREAM and
+ // its error code, so close with NO_ERROR.
+ visitor_.OnCloseStream(c.stream_id(), Http2ErrorCode::NO_ERROR);
+ }
+
frames_.pop_front();
if (result < frame.size()) {
// The frame was partially written, so the rest must be buffered.
@@ -674,6 +693,20 @@
return true;
}
+void OgHttp2Session::OnHeaderStatus(
+ Http2StreamId stream_id, Http2VisitorInterface::OnHeaderResult result) {
+ QUICHE_DCHECK_NE(result, Http2VisitorInterface::HEADER_OK);
+ if (result == Http2VisitorInterface::HEADER_RST_STREAM) {
+ auto it = streams_reset_.find(stream_id);
+ if (it == streams_reset_.end()) {
+ EnqueueFrame(absl::make_unique<spdy::SpdyRstStreamIR>(
+ stream_id, spdy::ERROR_CODE_INTERNAL_ERROR));
+ }
+ } else if (result == Http2VisitorInterface::HEADER_CONNECTION_ERROR) {
+ visitor_.OnConnectionError();
+ }
+}
+
void OgHttp2Session::MaybeSetupPreface() {
if (!queued_preface_) {
if (options_.perspective == Perspective::kClient) {
@@ -709,13 +742,15 @@
StreamState& state) {
state.half_closed_local = true;
if (options_.perspective == Perspective::kServer) {
- if (!state.half_closed_remote) {
+ if (state.half_closed_remote) {
+ visitor_.OnCloseStream(stream_id, Http2ErrorCode::NO_ERROR);
+ } else {
// Since the peer has not yet ended the stream, this endpoint should
// send a RST_STREAM NO_ERROR. See RFC 7540 Section 8.1.
EnqueueFrame(absl::make_unique<spdy::SpdyRstStreamIR>(
stream_id, spdy::SpdyErrorCode::ERROR_CODE_NO_ERROR));
+ // Enqueuing the RST_STREAM also invokes OnCloseStream.
}
- visitor_.OnCloseStream(stream_id, Http2ErrorCode::NO_ERROR);
// TODO(birenroy): the server adapter should probably delete stream state
// when calling visitor_.OnCloseStream.
}
diff --git a/http2/adapter/oghttp2_session.h b/http2/adapter/oghttp2_session.h
index 30306fe..bd1a40a 100644
--- a/http2/adapter/oghttp2_session.h
+++ b/http2/adapter/oghttp2_session.h
@@ -144,6 +144,11 @@
bool OnUnknownFrame(spdy::SpdyStreamId stream_id,
uint8_t frame_type) override;
+ // Invoked when header processing encounters an invalid or otherwise
+ // problematic header.
+ void OnHeaderStatus(Http2StreamId stream_id,
+ Http2VisitorInterface::OnHeaderResult result);
+
private:
struct StreamState {
StreamState(int32_t stream_receive_window,
@@ -161,17 +166,24 @@
class PassthroughHeadersHandler : public spdy::SpdyHeadersHandlerInterface {
public:
- explicit PassthroughHeadersHandler(Http2VisitorInterface& visitor)
- : visitor_(visitor) {}
- void set_stream_id(Http2StreamId stream_id) { stream_id_ = stream_id; }
+ explicit PassthroughHeadersHandler(OgHttp2Session& session,
+ Http2VisitorInterface& visitor)
+ : session_(session), visitor_(visitor) {}
+ void set_stream_id(Http2StreamId stream_id) {
+ stream_id_ = stream_id;
+ result_ = Http2VisitorInterface::HEADER_OK;
+ }
void OnHeaderBlockStart() override;
void OnHeader(absl::string_view key, absl::string_view value) override;
void OnHeaderBlockEnd(size_t /* uncompressed_header_bytes */,
size_t /* compressed_header_bytes */) override;
private:
+ OgHttp2Session& session_;
Http2VisitorInterface& visitor_;
Http2StreamId stream_id_ = 0;
+ Http2VisitorInterface::OnHeaderResult result_ =
+ Http2VisitorInterface::HEADER_OK;
};
// Queues the connection preface, if not already done.
@@ -226,6 +238,8 @@
WindowManager connection_window_manager_;
+ absl::flat_hash_set<Http2StreamId> streams_reset_;
+
Http2StreamId next_stream_id_ = 1;
Http2StreamId highest_received_stream_id_ = 0;
int connection_send_window_ = kInitialFlowControlWindowSize;
diff --git a/http2/adapter/recording_http2_visitor.cc b/http2/adapter/recording_http2_visitor.cc
index 48afbe4..cf81ee1 100644
--- a/http2/adapter/recording_http2_visitor.cc
+++ b/http2/adapter/recording_http2_visitor.cc
@@ -45,12 +45,11 @@
events_.push_back(absl::StrFormat("OnBeginHeadersForStream %d", stream_id));
}
-bool RecordingHttp2Visitor::OnHeaderForStream(Http2StreamId stream_id,
- absl::string_view name,
- absl::string_view value) {
+Http2VisitorInterface::OnHeaderResult 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;
+ return HEADER_OK;
}
void RecordingHttp2Visitor::OnEndHeadersForStream(Http2StreamId stream_id) {
diff --git a/http2/adapter/recording_http2_visitor.h b/http2/adapter/recording_http2_visitor.h
index aea7f9d..62ce651 100644
--- a/http2/adapter/recording_http2_visitor.h
+++ b/http2/adapter/recording_http2_visitor.h
@@ -29,8 +29,9 @@
void OnSettingsEnd() override;
void OnSettingsAck() override;
void OnBeginHeadersForStream(Http2StreamId stream_id) override;
- bool OnHeaderForStream(Http2StreamId stream_id, absl::string_view name,
- absl::string_view value) override;
+ OnHeaderResult OnHeaderForStream(Http2StreamId stream_id,
+ absl::string_view name,
+ absl::string_view value) override;
void OnEndHeadersForStream(Http2StreamId stream_id) override;
void OnBeginDataForStream(Http2StreamId stream_id,
size_t payload_length) override;
